internal/service/frontend/api/v1/transformer_test.go (1)

119-134: LGTM — test covers happy-path propagation cleanly.

Optional follow-up: consider also asserting the negative path (e.g., core.DAG{} with no ParamSchema, or with malformed JSON) yields details.ParamSchema == nil, to lock in toJSONObject's nil-return contract against future regressions.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/service/frontend/api/v1/transformer_test.go` around lines 119 - 134,
Add a negative-path unit test alongside TestToDAGDetailsIncludesParamSchema that
calls toDAGDetails with a core.DAG that has no ParamSchema (e.g., core.DAG{})
and another case with an invalid ParamSchema (malformed JSON) and assert that
details.ParamSchema == nil for both; locate and update the test in
transformer_test.go and reference the existing toDAGDetails/toJSONObject
behavior so the new test locks in the nil-return contract against regressions.

ui/src/features/dags/components/dag-execution/__tests__/StartDAGModal.test.tsx (1)

50-52: Minor: prefer beforeEach reset or vi.clearAllMocks().

afterEach only clears renderedFormProps. If more vi.fn() instances accumulate state across tests later, they'd be missed. Consider beforeEach(() => vi.clearAllMocks()) for symmetry.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@ui/src/features/dags/components/dag-execution/__tests__/StartDAGModal.test.tsx`
around lines 50 - 52, The test teardown currently only calls
renderedFormProps.mockClear() in afterEach, which can miss other vi.fn() state;
replace or complement this with a beforeEach(() => vi.clearAllMocks()) to ensure
all mocks (including renderedFormProps and any future vi.fn() instances) are
reset before each test, or alternatively call vi.clearAllMocks() in afterEach
for symmetry; update the test file to add the beforeEach/afterEach call
referencing renderedFormProps and vi.clearAllMocks accordingly.

internal/core/spec/dag_param_schema.go (2)

45-47: Improve readability of the unsupported-construct rejection.

Long boolean conjunctions like this (and the analogous one at lines 75-77) are hard to scan and easy to miss when adding new JSON Schema keywords. Consider extracting a small helper.

♻️ Suggested refactor

func hasUnsupportedRootConstruct(s *jsonschema.Schema) bool {
    return len(s.PatternProperties) > 0 ||
        len(s.AllOf) > 0 || len(s.AnyOf) > 0 || len(s.OneOf) > 0 ||
        s.Not != nil || s.If != nil || s.Then != nil || s.Else != nil ||
        s.Items != nil || len(s.PrefixItems) > 0 || s.Contains != nil
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/core/spec/dag_param_schema.go` around lines 45 - 47, Extract the
long conjunctive check into a small helper like hasUnsupportedRootConstruct that
accepts *jsonschema.Schema (referencing the variable root used in the current
checks) and returns a bool; replace the current long boolean expression in the
early-return (the block using root.PatternProperties, root.AllOf, root.AnyOf,
root.OneOf, root.Not, root.If, root.Then, root.Else, root.Items,
root.PrefixItems, root.Contains) with a call to that helper, and do the same for
the analogous check later (the similar expression at the other return site) so
both places use the new function.

---

99-99: Type assertion is safe, but prefer explicit element cloning for clarity.

cloneAny(schema.Enum) safely returns []any since Enum is a slice and cloneAny explicitly handles the []any case. However, the type assertion can be avoided by directly cloning elements, which is clearer and follows the same defensive approach used elsewhere:

Suggested change

	if len(schema.Enum) > 0 {
-		result.Enum = cloneAny(schema.Enum).([]any)
+		enum := make([]any, len(schema.Enum))
+		for i, v := range schema.Enum {
+			enum[i] = cloneAny(v)
+		}
+		result.Enum = enum
	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/core/spec/dag_param_schema.go` at line 99, The assignment
result.Enum = cloneAny(schema.Enum).([]any) relies on a type assertion; instead,
iterate over schema.Enum, clone each element via cloneAny and append to a new
[]any slice, then assign that slice to result.Enum so element-level cloning is
explicit and matches the defensive pattern used elsewhere (update code handling
of schema.Enum in the logic around cloneAny, result.Enum, and schema.Enum).

ui/src/features/dags/components/dag-execution/paramSchemaForm.ts (1)

60-82: Edge case: empty string coerces to 0 for integer/number schemas.

Number('') is 0 and Number.isInteger(0) is true, so coerceParamSchemaValue('', integerSchema) returns 0 rather than the original empty string. Whether this is a problem depends on whether parseParams can produce empty-string values; if so, the user could see unexpected zeros pre-filled. Consider returning value when value.trim() === ''.

♻️ Suggested change

 function coerceParamSchemaValue(value: string, schema: JSONSchema): unknown {
+  if (value === '') {
+    return value;
+  }
   switch (inferScalarType(schema)) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/features/dags/components/dag-execution/paramSchemaForm.ts` around
lines 60 - 82, The function coerceParamSchemaValue incorrectly coerces an empty
string to 0 for 'integer'/'number' schemas; update coerceParamSchemaValue to
first check if the incoming value is an empty string (e.g., value.trim() === '')
and return the original value unchanged before calling inferScalarType /
attempting Number(value) conversion; keep this check at the top of
coerceParamSchemaValue so integer/number/boolean branches don't convert blank
input and retain existing behavior for non-empty strings.

ui/src/features/dags/components/dag-execution/StartDAGModal.tsx (1)

256-262: Defensive guard reads well; minor: also reject empty properties.

paramSchema will be set whenever dagDetails.paramSchema is a plain object, including {} or one without properties. The backend sanitizer rejects schemas without properties so this is unlikely in practice, but a UI-side guard would prevent rendering a useless empty form if a malformed schema ever leaks through.

♻️ Suggested change

-    const schema = dagDetails?.paramSchema;
-    if (!schema || Array.isArray(schema) || typeof schema !== 'object') {
-      return undefined;
-    }
-    return schema as JSONSchema;
+    const schema = dagDetails?.paramSchema as JSONSchema | undefined;
+    if (!schema || Array.isArray(schema) || typeof schema !== 'object') {
+      return undefined;
+    }
+    if (!schema.properties || Object.keys(schema.properties).length === 0) {
+      return undefined;
+    }
+    return schema;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/features/dags/components/dag-execution/StartDAGModal.tsx` around lines
256 - 262, The memoized paramSchema currently accepts any plain object from
dagDetails.paramSchema; update the guard in the React.useMemo so it also rejects
schemas that lack a non-empty properties object (e.g., ensure schema.properties
is an object and Object.keys(schema.properties).length > 0) before returning
schema as JSONSchema; this prevents returning {} or schemas without properties
and avoids rendering an empty form.

ui/src/features/dags/components/dag-execution/schemaFormWidgets.tsx (1)

142-163: Consider whether currentValue should fall back to the encoded raw value.

When value doesn't match any enumOptions entry (e.g., a stale or out-of-range default), enumOptionsIndexForValue returns nothing and currentValue becomes undefined, leaving the radio group visually unselected even though formData holds a value. RJSF's stock RadioWidget handles this similarly, so this is consistent — but worth a brief inline comment to flag the intent so future readers don't "fix" it.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/features/dags/components/dag-execution/schemaFormWidgets.tsx` around
lines 142 - 163, The currentValue computation returns undefined when value
doesn't match any enumOptions (via enumOptionsIndexForValue) which leaves the
radio group unselected; add a brief inline comment inside the React.useMemo
(near currentValue, enumOptionsIndexForValue, enumOptionValueEncoder,
enumOptions, and value references) stating this is intentional and that we
intentionally do not fall back to encoding the raw value so stale/out-of-range
defaults remain visually unselected, to prevent future maintainers from "fixing"
this behavior.

internal/service/frontend/api/v1/transformer.go (1)

443-453: Consider logging on unmarshal failure.

toJSONObject collapses both "empty" and "malformed JSON" into the same nil result. Since dag.ParamSchema is produced internally by buildRenderableParamSchema (which already json.Marshals a sanitized schema), an unmarshal error here indicates an internal inconsistency that would otherwise be invisible to operators. Consider at least logging the error.

♻️ Suggested change

 func toJSONObject(raw json.RawMessage) *map[string]any {
 	if len(raw) == 0 {
 		return nil
 	}

 	var value map[string]any
 	if err := json.Unmarshal(raw, &value); err != nil {
+		// Should not happen: paramSchema is produced via json.Marshal upstream.
+		logger.Warn("toJSONObject: failed to unmarshal param schema", "error", err)
 		return nil
 	}
 	return &value
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/service/frontend/api/v1/transformer.go` around lines 443 - 453,
toJSONObject currently hides JSON unmarshal failures by returning nil for both
empty and malformed input; update to log the unmarshal error (including the
error value and a brief context such as "failed to unmarshal dag.ParamSchema
produced by buildRenderableParamSchema") before returning nil. Locate the
toJSONObject function and add a logging statement that includes the raw input
length or a snippet and the error (so operators can see internal inconsistencies
between buildRenderableParamSchema and dag.ParamSchema) while preserving the
existing nil return behavior for len(raw)==0 and on error.

ui/src/features/dags/components/dag-execution/schemaFormTemplates.tsx (1)

1-1: Verify use of @rjsf/shadcn/lib/... deep import.

Importing from @rjsf/shadcn/lib/BaseInputTemplate/BaseInputTemplate.js reaches into the package's internal layout. If the package uses exports field gating in its package.json, this can break on a minor upgrade. Prefer a public re-export (often available via @rjsf/shadcn or its Templates/getDefaultRegistry) if exposed.

#!/bin/bash
# Inspect `@rjsf/shadcn`'s package.json to see whether deep '/lib/...js' subpaths are part of its public 'exports' map.
fd -i 'package.json' -t f node_modules/@rjsf/shadcn -d 2 2>/dev/null | head -5
# Fall back to repo-local lockfile to identify version & published exports surface
rg -nP '"@rjsf/shadcn"' -g 'package.json' -g 'pnpm-lock.yaml' | head -20

`@rjsf/shadcn` 6.5.1 BaseInputTemplate public export path

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/features/dags/components/dag-execution/schemaFormTemplates.tsx` at
line 1, The import is using a deep path into `@rjsf/shadcn`
("@rjsf/shadcn/lib/BaseInputTemplate/BaseInputTemplate.js") which can break on
upgrades; replace it with the package's public export instead (e.g., import
BaseInputTemplate from the top-level "@rjsf/shadcn" or pull it from the
package's exported Templates/getDefaultRegistry API) so that the code uses an
official public symbol (referencing BaseInputTemplate and any
Templates/getDefaultRegistry helpers) rather than a private lib subpath.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes