-
Notifications
You must be signed in to change notification settings - Fork 3.3k
improvement(schema): centralize derivation of block schemas #3175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: staging
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile OverviewGreptile SummaryThis PR centralizes block output/path/type derivation by introducing Key integration points:
Issues to address before merge mainly relate to option consistency (triggerMode vs preferToolOutputs) and payload/test shape mismatches that can cause consumers to see missing/incorrect outputs or inconsistent metadata. Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as UI (TagDropdown/OutputSelect)
participant Store as SubBlockStore/WorkflowStore
participant BO as block-outputs.ts
participant Reg as BlockRegistry/ToolsRegistry
participant Exec as Executor (BlockResolver)
UI->>Store: read block.type + subBlocks
UI->>BO: getEffectiveBlockOutputPaths/Type(type, subBlocks, opts)
BO->>Reg: getBlock(type)
alt preferToolOutputs && !triggerMode
BO->>Reg: blockConfig.tools.config.tool(params)
BO->>Reg: getTool(toolId).outputs
BO-->>UI: tool-derived outputs/paths/types
else triggerMode && triggers.enabled
BO->>Reg: getTrigger(triggerId).outputs
BO-->>UI: trigger-derived outputs
else
BO-->>UI: blockConfig.outputs + inputFormat applied
end
Exec->>BO: getEffectiveBlockOutputs(type, paramsToSubBlocks(config.params), includeHidden)
BO-->>Exec: output schema for validation
Exec-->>Exec: resolveBlockReference() validates path against schema
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
12 files reviewed, 1 comment
Additional Comments (1)
|
...onents/panel/components/editor/components/sub-block/components/tag-dropdown/tag-dropdown.tsx
Show resolved
Hide resolved
|
@cursor review |
|
@cursor review |
…-derivation Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # apps/sim/lib/copilot/tools/client/workflow/block-output-utils.ts
|
@cursor review |
|
@cursor review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
|
@greptile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6 files reviewed, 4 comments
apps/sim/lib/copilot/tools/server/workflow/edit-workflow/builders.test.ts
Show resolved
Hide resolved
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-block-output-fields.ts
Show resolved
Hide resolved
Additional Comments (1)
In |
|
@cursor review |
|
@cursor review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| if (blockType === 'evaluator') { | ||
| const metricOutputs = getEvaluatorMetricOutputs(subBlocks) | ||
| if (metricOutputs) { | ||
| return metricOutputs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Evaluator metrics discard base outputs in executor schema
Medium Severity
getEffectiveBlockOutputs computes baseOutputs (tool/block outputs like content, model, tokens, cost) but then entirely replaces them with only metric outputs when the evaluator has metrics defined. The old executor code in block-data.ts merged both: { ...block.outputs, ...metricOutputs }. Now, referencing a base output field like <evaluator.content> may throw an InvalidFieldError during schema validation when the evaluator's output isn't yet available, because content is no longer in the schema.


Summary
Derive the block/tool outputs the same way throughout codebase -- based on actual block registry. Previously, resolver takes it from block outputs [from state rather than registry]
Type of Change
Testing
Tested manually
Checklist