feat(ui): add custom step editor autocomplete#1989
Merged
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds editor hints support to expose inherited custom step type definitions from the base config to the DAG editor. It introduces new API types for editor metadata, backend logic to derive hints from the base config spec, and UI components that thread hints through the editor pipeline where they augment the YAML schema for improved validation and autocomplete. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser/UI
participant Service as Frontend Service
participant Spec as Spec Utils
participant Editor as DAG Editor
participant Monaco as Monaco YAML
Browser->>Service: GET /dags/{fileName}
Service->>Service: Read DAG file
Service->>Service: Get BaseConfig bytes
Service->>Spec: InheritedCustomStepTypeEditorHints(baseConfig)
Spec->>Spec: Decode & extract custom step types
Spec-->>Service: Return editor hints
Service-->>Browser: GetDAGDetails response + editorHints
Browser->>Editor: Render DAGSpec with editorHints
Editor->>Editor: Extract local custom step types from YAML
Editor->>Editor: mergeCustomStepTypeHints(inherited, local)
Editor->>Editor: buildAugmentedDAGSchema(baseSchema, merged)
Editor->>Monaco: Register dynamic schema (modelUri, augmentedSchema)
Monaco-->>Editor: Schema validation/completion configured
Browser->>Editor: User edits YAML content
Monaco->>Monaco: Validate against augmented schema
Monaco-->>Browser: Show custom type suggestions & validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/core/spec/editor_hints.go`:
- Around line 56-58: The current guard that does "if entry == nil ||
entry.InputSchema == nil || entry.InputSchema.Schema() == nil { continue }"
drops inherited custom step types when InputSchema is missing; instead preserve
the hint and treat a missing schema as an empty schema object: locate the block
that checks entry, InputSchema and InputSchema.Schema() (variables/methods:
entry, InputSchema, Schema()) and replace the unconditional continue with logic
that constructs or assigns an empty schema (e.g., zero-value schema/map/struct)
when InputSchema or Schema() is nil, then proceed to include the hint in editor
type suggestions so inherited step types are not removed.
In `@internal/service/frontend/api/v1/dags.go`:
- Around line 471-473: The loop is taking the address of the range variable
field (&hint.Description), which leads to all pointers referencing the same
reused stack slot; instead, create a new local variable per iteration and take
its address (e.g., desc := hint.Description; apiHint.Description = &desc) or
allocate a new string pointer and assign it, ensuring each apiHint.Description
points to a distinct copy; update the code around the hint -> apiHint mapping
where Description is assigned to use this per-iteration local copy.
In
`@ui/src/features/dags/components/dag-editor/__tests__/customStepSchema.test.ts`:
- Around line 1-12: This new test file is missing the repository GPLv3 license
header; run the repository license tool (make addlicense) or insert the standard
GPL v3 header at the top of
ui/src/features/dags/components/dag-editor/__tests__/customStepSchema.test.ts
(the new test file importing buildAugmentedDAGSchema,
extractLocalCustomStepTypeHints, mergeCustomStepTypeHints, and schema-utils
symbols) so the header precedes the imports; prefer running `make addlicense` to
apply the correct header consistently.
In `@ui/src/features/dags/components/dag-editor/customStepSchema.ts`:
- Around line 1-4: This new TypeScript file (imports: components, parseYaml,
dereferenceSchema, JSONSchema) is missing the repository GPL v3 header; run the
repository tool to add it by executing `make addlicense` (or the project’s
license tooling) so the standard GPL header is prepended to
ui/src/features/dags/components/dag-editor/customStepSchema.ts, then re-stage
the file and update the PR.
In `@ui/src/features/dags/components/dag-editor/DAGSpec.tsx`:
- Around line 153-179: The parsedLocalStepTypes value currently contains a new
array identity on every edit which invalidates the editorSchema memo; to fix
this, stabilize effectiveLocalStepTypes by comparing extracted step-type hints
by value and only replacing the previous array when the content actually changes
(e.g., compute parsedLocalStepTypes via extractLocalCustomStepTypeHints, then
deep-equal its stepTypes to lastGoodLocalStepTypes and reuse the previous array
identity when equal). Update the logic around parsedLocalStepTypes,
setLastGoodLocalStepTypes, and effectiveLocalStepTypes so
buildAugmentedDAGSchema( baseSchema,
mergeCustomStepTypeHints(inheritedCustomStepTypes, effectiveLocalStepTypes) )
only reruns when the hint set's content changes, preserving identity to avoid
re-registering the Monaco schema.
- Around line 181-183: The Monaco model URI generation in editorModelUri
currently only uses fileName, causing models to be shared across remote nodes;
update the memoized value in editorModelUri to include the remoteNode (e.g.,
`${encodeURIComponent(fileName)}:${remoteNode}` or similar) so the in-memory URI
is namespaced per node, ensuring model/schema registrations, undo history, and
transient edits are isolated per remoteNode; reference the editorModelUri
variable and the existing useContentEditor keying to `${fileName}:${remoteNode}`
when making this change.
In `@ui/src/lib/schema-utils.ts`:
- Around line 94-125: The dereferenceSchema/processNode flow currently only
stores a resolved ref in cache after recursively processing it, causing infinite
recursion for self-referential internal $ref; fix by inserting a placeholder
into cache immediately when encountering a local ref (use the ref string as key
and a temporary empty object or sentinel JSONSchema), then call
resolveInternalRef and processNode to fill the placeholder, and finally
replace/update the cache entry with the fully-processed schema before merging
with rest. Update the logic around the `$ref` handling in dereferenceSchema
(processNode, cache, resolveInternalRef) so the cache is populated prior to
recursive processing to break cycles.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9165226d-45f2-4a30-ab92-45add989e362
📒 Files selected for processing (19)
api/v1/api.gen.goapi/v1/api.yamlinternal/core/spec/editor_hints.gointernal/service/frontend/api/v1/dags.gointernal/service/frontend/api/v1/dags_internal_test.goui/src/api/v1/schema.tsui/src/contexts/SchemaContext.tsxui/src/features/dags/components/dag-details/DAGDetailsContent.tsxui/src/features/dags/components/dag-editor/DAGEditor.tsxui/src/features/dags/components/dag-editor/DAGEditorWithDocs.tsxui/src/features/dags/components/dag-editor/DAGSpec.tsxui/src/features/dags/components/dag-editor/DAGSpecReadOnly.tsxui/src/features/dags/components/dag-editor/SchemaDocSidebar/SchemaDocSidebar.tsxui/src/features/dags/components/dag-editor/__tests__/customStepSchema.test.tsui/src/features/dags/components/dag-editor/customStepSchema.tsui/src/hooks/useSchemaLookup.tsui/src/lib/schema-utils.tsui/src/pages/base-config/index.tsxui/src/pages/dags/dag/index.tsx
Comment on lines
+56
to
+58
| if entry == nil || entry.InputSchema == nil || entry.InputSchema.Schema() == nil { | ||
| continue | ||
| } |
There was a problem hiding this comment.
Do not drop inherited custom step types when InputSchema is absent.
At Line 56, the guard currently continues when schema is missing, which can remove otherwise valid inherited step types from editor type suggestions. Keep the hint and use an empty schema object instead.
Suggested fix
- if entry == nil || entry.InputSchema == nil || entry.InputSchema.Schema() == nil {
+ if entry == nil {
continue
}
- schemaData, err := json.Marshal(entry.InputSchema.Schema())
- if err != nil {
- return nil, fmt.Errorf("marshal input schema for %q: %w", entry.Name, err)
- }
-
- var schemaMap map[string]any
- if err := json.Unmarshal(schemaData, &schemaMap); err != nil {
- return nil, fmt.Errorf("unmarshal input schema for %q: %w", entry.Name, err)
- }
+ schemaMap := map[string]any{}
+ if entry.InputSchema != nil && entry.InputSchema.Schema() != nil {
+ schemaData, err := json.Marshal(entry.InputSchema.Schema())
+ if err != nil {
+ return nil, fmt.Errorf("marshal input schema for %q: %w", entry.Name, err)
+ }
+ if err := json.Unmarshal(schemaData, &schemaMap); err != nil {
+ return nil, fmt.Errorf("unmarshal input schema for %q: %w", entry.Name, err)
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/core/spec/editor_hints.go` around lines 56 - 58, The current guard
that does "if entry == nil || entry.InputSchema == nil ||
entry.InputSchema.Schema() == nil { continue }" drops inherited custom step
types when InputSchema is missing; instead preserve the hint and treat a missing
schema as an empty schema object: locate the block that checks entry,
InputSchema and InputSchema.Schema() (variables/methods: entry, InputSchema,
Schema()) and replace the unconditional continue with logic that constructs or
assigns an empty schema (e.g., zero-value schema/map/struct) when InputSchema or
Schema() is nil, then proceed to include the hint in editor type suggestions so
inherited step types are not removed.
Comment on lines
+471
to
+473
| if hint.Description != "" { | ||
| apiHint.Description = &hint.Description | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the exact risky pattern exists in the reviewed file.
rg -nP --type=go -C2 '&\s*hint\.Description' internal/service/frontend/api/v1/dags.go
# Inspect loop context around the match.
rg -nP --type=go -C6 'for\s+_,\s+hint\s*:=\s*range\s+hints' internal/service/frontend/api/v1/dags.goRepository: dagucloud/dagu
Length of output: 572
Avoid taking the address of a range-loop variable field.
At line 472, &hint.Description is taken from the loop variable hint. Since hint is reused across iterations in the for-range loop, the pointer will reference stale or uninitialized storage after the loop completes. All pointers in the returned slice will end up pointing to the last iteration's value (or invalid memory).
Suggested fix
if hint.Description != "" {
- apiHint.Description = &hint.Description
+ desc := hint.Description
+ apiHint.Description = &desc
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/service/frontend/api/v1/dags.go` around lines 471 - 473, The loop is
taking the address of the range variable field (&hint.Description), which leads
to all pointers referencing the same reused stack slot; instead, create a new
local variable per iteration and take its address (e.g., desc :=
hint.Description; apiHint.Description = &desc) or allocate a new string pointer
and assign it, ensuring each apiHint.Description points to a distinct copy;
update the code around the hint -> apiHint mapping where Description is assigned
to use this per-iteration local copy.
Comment on lines
+1
to
+12
| import { describe, expect, it } from 'vitest'; | ||
| import { | ||
| buildAugmentedDAGSchema, | ||
| extractLocalCustomStepTypeHints, | ||
| mergeCustomStepTypeHints, | ||
| } from '../customStepSchema'; | ||
| import { | ||
| dereferenceSchema, | ||
| getSchemaAtPath, | ||
| toPropertyInfo, | ||
| type JSONSchema, | ||
| } from '@/lib/schema-utils'; |
There was a problem hiding this comment.
Run make addlicense for this new source file.
This file is new and still missing the repository-mandated GPL header.
As per coding guidelines, **/*.{go,ts,tsx,js}: Apply GPL v3 license headers on source files, managed via make addlicense.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ui/src/features/dags/components/dag-editor/__tests__/customStepSchema.test.ts`
around lines 1 - 12, This new test file is missing the repository GPLv3 license
header; run the repository license tool (make addlicense) or insert the standard
GPL v3 header at the top of
ui/src/features/dags/components/dag-editor/__tests__/customStepSchema.test.ts
(the new test file importing buildAugmentedDAGSchema,
extractLocalCustomStepTypeHints, mergeCustomStepTypeHints, and schema-utils
symbols) so the header precedes the imports; prefer running `make addlicense` to
apply the correct header consistently.
Comment on lines
+1
to
+4
| import type { components } from '../../../../api/v1/schema'; | ||
| import { parse as parseYaml } from 'yaml'; | ||
| import { dereferenceSchema, type JSONSchema } from '@/lib/schema-utils'; | ||
|
|
There was a problem hiding this comment.
Run make addlicense for this new source file.
This file is new and still missing the repository-mandated GPL header.
As per coding guidelines, **/*.{go,ts,tsx,js}: Apply GPL v3 license headers on source files, managed via make addlicense.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/features/dags/components/dag-editor/customStepSchema.ts` around lines
1 - 4, This new TypeScript file (imports: components, parseYaml,
dereferenceSchema, JSONSchema) is missing the repository GPL v3 header; run the
repository tool to add it by executing `make addlicense` (or the project’s
license tooling) so the standard GPL header is prepended to
ui/src/features/dags/components/dag-editor/customStepSchema.ts, then re-stage
the file and update the PR.
Comment on lines
+131
to
+157
| function isStepLikeSchema(schema: JSONSchema): boolean { | ||
| const properties = schema.properties; | ||
| return ( | ||
| schema.type === 'object' && | ||
| !!properties && | ||
| isRecord(properties.type) && | ||
| isRecord(properties.config) | ||
| ); | ||
| } | ||
|
|
||
| function augmentStepSchema( | ||
| stepSchema: JSONSchema, | ||
| customStepRules: JSONSchema[], | ||
| customTypeNames: string[], | ||
| customTypeDescriptions: string[] | ||
| ) { | ||
| stepSchema.allOf = appendUniqueAllOf(stepSchema.allOf, customStepRules); | ||
|
|
||
| const typeSchema = stepSchema.properties?.type; | ||
| if (isRecord(typeSchema)) { | ||
| augmentExecutorTypeSchema( | ||
| typeSchema as JSONSchema, | ||
| customTypeNames, | ||
| customTypeDescriptions | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Limit post-dereference augmentation to actual DAG step schemas.
visitSchemas() walks the user-provided inputSchema trees too, and isStepLikeSchema() matches any object that happens to define properties.type and properties.config. A nested config object with those common field names will start getting executor-type completions and custom config switching even though it is not a DAG step.
Also applies to: 159-176, 350-359
Comment on lines
+153
to
+179
| const parsedLocalStepTypes = React.useMemo( | ||
| () => extractLocalCustomStepTypeHints(currentValue ?? serverSpec ?? ''), | ||
| [currentValue, serverSpec] | ||
| ); | ||
|
|
||
| useEffect(() => { | ||
| if (parsedLocalStepTypes.ok) { | ||
| setLastGoodLocalStepTypes(parsedLocalStepTypes.stepTypes); | ||
| } | ||
| }, [parsedLocalStepTypes]); | ||
|
|
||
| const effectiveLocalStepTypes = parsedLocalStepTypes.ok | ||
| ? parsedLocalStepTypes.stepTypes | ||
| : lastGoodLocalStepTypes; | ||
|
|
||
| const editorSchema = React.useMemo(() => { | ||
| if (!baseSchema) { | ||
| return null; | ||
| } | ||
| return buildAugmentedDAGSchema( | ||
| baseSchema, | ||
| mergeCustomStepTypeHints( | ||
| inheritedCustomStepTypes, | ||
| effectiveLocalStepTypes | ||
| ) | ||
| ); | ||
| }, [baseSchema, effectiveLocalStepTypes, inheritedCustomStepTypes]); |
There was a problem hiding this comment.
Keep editorSchema stable when the extracted hint set hasn't changed.
extractLocalCustomStepTypeHints() returns a fresh array on every edit, so this memo invalidates even when the user only touched an unrelated field. That forces buildAugmentedDAGSchema() to clone/dereference the full schema again and makes DAGEditor re-register the Monaco schema on every keystroke.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/features/dags/components/dag-editor/DAGSpec.tsx` around lines 153 -
179, The parsedLocalStepTypes value currently contains a new array identity on
every edit which invalidates the editorSchema memo; to fix this, stabilize
effectiveLocalStepTypes by comparing extracted step-type hints by value and only
replacing the previous array when the content actually changes (e.g., compute
parsedLocalStepTypes via extractLocalCustomStepTypeHints, then deep-equal its
stepTypes to lastGoodLocalStepTypes and reuse the previous array identity when
equal). Update the logic around parsedLocalStepTypes, setLastGoodLocalStepTypes,
and effectiveLocalStepTypes so buildAugmentedDAGSchema( baseSchema,
mergeCustomStepTypeHints(inheritedCustomStepTypes, effectiveLocalStepTypes) )
only reruns when the hint set's content changes, preserving identity to avoid
re-registering the Monaco schema.
Comment on lines
+181
to
+183
| const editorModelUri = React.useMemo( | ||
| () => `inmemory://dagu/dags/${encodeURIComponent(fileName)}.yaml`, | ||
| [fileName] |
There was a problem hiding this comment.
Namespace the Monaco model URI by remoteNode.
useContentEditor already keys drafts by ${fileName}:${remoteNode}, but Monaco is still keyed only by fileName. In a multi-node deployment, opening the same DAG name on two nodes will reuse one in-memory model/schema registration and leak undo history or transient edits across nodes.
Suggested fix
const editorModelUri = React.useMemo(
- () => `inmemory://dagu/dags/${encodeURIComponent(fileName)}.yaml`,
- [fileName]
+ () =>
+ `inmemory://dagu/${encodeURIComponent(remoteNode)}/dags/${encodeURIComponent(fileName)}.yaml`,
+ [fileName, remoteNode]
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const editorModelUri = React.useMemo( | |
| () => `inmemory://dagu/dags/${encodeURIComponent(fileName)}.yaml`, | |
| [fileName] | |
| const editorModelUri = React.useMemo( | |
| () => | |
| `inmemory://dagu/${encodeURIComponent(remoteNode)}/dags/${encodeURIComponent(fileName)}.yaml`, | |
| [fileName, remoteNode] | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/features/dags/components/dag-editor/DAGSpec.tsx` around lines 181 -
183, The Monaco model URI generation in editorModelUri currently only uses
fileName, causing models to be shared across remote nodes; update the memoized
value in editorModelUri to include the remoteNode (e.g.,
`${encodeURIComponent(fileName)}:${remoteNode}` or similar) so the in-memory URI
is namespaced per node, ensuring model/schema registrations, undo history, and
transient edits are isolated per remoteNode; reference the editorModelUri
variable and the existing useContentEditor keying to `${fileName}:${remoteNode}`
when making this change.
Comment on lines
+94
to
+125
| export function dereferenceSchema(schema: JSONSchema): JSONSchema { | ||
| const cache = new Map<string, JSONSchema>(); | ||
|
|
||
| function processNode(node: unknown): unknown { | ||
| if (!node || typeof node !== 'object') { | ||
| return node; | ||
| } | ||
|
|
||
| if (Array.isArray(node)) { | ||
| return node.map(processNode); | ||
| } | ||
|
|
||
| const obj = node as Record<string, unknown>; | ||
| if (typeof obj.$ref === 'string' && obj.$ref.startsWith('#')) { | ||
| const ref = obj.$ref; | ||
| let resolved = cache.get(ref); | ||
| if (!resolved) { | ||
| resolved = processNode(resolveInternalRef(schema, ref)) as JSONSchema; | ||
| cache.set(ref, resolved); | ||
| } | ||
| const { $ref: _ref, ...rest } = obj; | ||
| return processNode({ ...resolved, ...rest }); | ||
| } | ||
|
|
||
| const result: Record<string, unknown> = {}; | ||
| for (const [key, value] of Object.entries(obj)) { | ||
| result[key] = processNode(value); | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| return processNode(schema) as JSONSchema; |
There was a problem hiding this comment.
Break recursive internal $ref cycles before expanding them.
The cache is populated only after the referenced node is fully processed. A self-referential custom step_types.*.input_schema ($ref: '#' or a definition that points back to itself) will recurse until the stack blows up here.
One way to guard recursive refs
export function dereferenceSchema(schema: JSONSchema): JSONSchema {
const cache = new Map<string, JSONSchema>();
+ const resolving = new Set<string>();
function processNode(node: unknown): unknown {
if (!node || typeof node !== 'object') {
return node;
}
@@
const obj = node as Record<string, unknown>;
if (typeof obj.$ref === 'string' && obj.$ref.startsWith('#')) {
const ref = obj.$ref;
let resolved = cache.get(ref);
if (!resolved) {
+ if (resolving.has(ref)) {
+ return { ...obj };
+ }
+ resolving.add(ref);
resolved = processNode(resolveInternalRef(schema, ref)) as JSONSchema;
+ resolving.delete(ref);
cache.set(ref, resolved);
}
const { $ref: _ref, ...rest } = obj;
return processNode({ ...resolved, ...rest });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function dereferenceSchema(schema: JSONSchema): JSONSchema { | |
| const cache = new Map<string, JSONSchema>(); | |
| function processNode(node: unknown): unknown { | |
| if (!node || typeof node !== 'object') { | |
| return node; | |
| } | |
| if (Array.isArray(node)) { | |
| return node.map(processNode); | |
| } | |
| const obj = node as Record<string, unknown>; | |
| if (typeof obj.$ref === 'string' && obj.$ref.startsWith('#')) { | |
| const ref = obj.$ref; | |
| let resolved = cache.get(ref); | |
| if (!resolved) { | |
| resolved = processNode(resolveInternalRef(schema, ref)) as JSONSchema; | |
| cache.set(ref, resolved); | |
| } | |
| const { $ref: _ref, ...rest } = obj; | |
| return processNode({ ...resolved, ...rest }); | |
| } | |
| const result: Record<string, unknown> = {}; | |
| for (const [key, value] of Object.entries(obj)) { | |
| result[key] = processNode(value); | |
| } | |
| return result; | |
| } | |
| return processNode(schema) as JSONSchema; | |
| export function dereferenceSchema(schema: JSONSchema): JSONSchema { | |
| const cache = new Map<string, JSONSchema>(); | |
| const resolving = new Set<string>(); | |
| function processNode(node: unknown): unknown { | |
| if (!node || typeof node !== 'object') { | |
| return node; | |
| } | |
| if (Array.isArray(node)) { | |
| return node.map(processNode); | |
| } | |
| const obj = node as Record<string, unknown>; | |
| if (typeof obj.$ref === 'string' && obj.$ref.startsWith('#')) { | |
| const ref = obj.$ref; | |
| let resolved = cache.get(ref); | |
| if (!resolved) { | |
| if (resolving.has(ref)) { | |
| return { ...obj }; | |
| } | |
| resolving.add(ref); | |
| resolved = processNode(resolveInternalRef(schema, ref)) as JSONSchema; | |
| resolving.delete(ref); | |
| cache.set(ref, resolved); | |
| } | |
| const { $ref: _ref, ...rest } = obj; | |
| return processNode({ ...resolved, ...rest }); | |
| } | |
| const result: Record<string, unknown> = {}; | |
| for (const [key, value] of Object.entries(obj)) { | |
| result[key] = processNode(value); | |
| } | |
| return result; | |
| } | |
| return processNode(schema) as JSONSchema; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/lib/schema-utils.ts` around lines 94 - 125, The
dereferenceSchema/processNode flow currently only stores a resolved ref in cache
after recursively processing it, causing infinite recursion for self-referential
internal $ref; fix by inserting a placeholder into cache immediately when
encountering a local ref (use the ref string as key and a temporary empty object
or sentinel JSONSchema), then call resolveInternalRef and processNode to fill
the placeholder, and finally replace/update the cache entry with the
fully-processed schema before merging with rest. Update the logic around the
`$ref` handling in dereferenceSchema (processNode, cache, resolveInternalRef) so
the cache is populated prior to recursive processing to break cycles.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
step_typestypeinference/docs so dereferenced step schemas still suggest builtin and custom step names and resolve customconfigfields correctlyTest plan
make apigo test ./internal/core/spec ./internal/service/frontend/api/v1 -count=1cd ui && pnpm gen:apicd ui && pnpm exec vitest run src/features/dags/components/dag-editor/__tests__/customStepSchema.test.tscd ui && pnpm typecheckWhy
The editor previously used a single static schema, so custom step types from
step_typesor base config could not autocomplete or driveconfigdocs. The follow-up fix in this branch also addresses a regression where steptypeandconfiginference stopped working after schema dereferencing because the augmentation only patcheddefinitions.*, not the live inline step schemas consumed by Monaco and the docs sidebar.Summary by CodeRabbit