Skip to content

improvement(response-copilot): prefer builder mode + fix builder/editor mode conversions - #1648

Merged
icecrasher321 merged 3 commits into
stagingfrom
improvement/context-response-block
Oct 16, 2025
Merged

improvement(response-copilot): prefer builder mode + fix builder/editor mode conversions#1648
icecrasher321 merged 3 commits into
stagingfrom
improvement/context-response-block

Conversation

@icecrasher321

Copy link
Copy Markdown
Collaborator

Summary

  • Moving between builder and editor mode correctly converts
  • Copilot best practice is to use builder mode since JSON generation is weaker.

Type of Change

  • Bug fix

Testing

Tested manually + trained copilot on conversion op

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercelBot commented Oct 16, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentPreviewCommentsUpdated (UTC)
docsSkippedSkippedOct 16, 2025 2:07am

// Use preview value when in preview mode, otherwise use store value
const value = isPreview ? previewValue : storeValue
const fields: Field[] = value || []
const fields: Field[] = Array.isArray(value) ? value : []

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this prevents nuking diff store if copilot outputs invalid fields i.e. not an array

@icecrasher321
icecrasher321 merged commit 701bf2b into stagingOct 16, 2025
9 checks passed

@greptile-appsgreptile-appsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Summary

Implements bidirectional conversion between builder and editor modes for the Response block, using refs to avoid stale closure issues and moving conversion logic to a useEffect.

Key changes:

  • Added previousModeRef to track mode changes and trigger conversions only when mode actually changes
  • Used refs (builderDataRef, dataRef) to access latest values in useEffect without stale closures
  • Moved conversion logic from handleSelect to dedicated useEffect for better separation
  • Updated best practices to prefer builder mode since JSON generation is weaker
  • Added defensive type guard in input-format.tsx to prevent crashes when value is not an array

Critical issue found:

  • The regex pattern in normalizeVariableReferences incorrectly captures the character before variable references, producing malformed JSON that will cause JSON.parse to fail when converting from editor to builder mode

Confidence Score: 1/5

  • This PR has a critical regex bug that will break editor-to-builder conversion
  • The normalizeVariableReferences regex pattern /([^"]<[^>]+>)/g captures one character before the variable reference (e.g., : <var> becomes ": <var>" resulting in "": <var>"), producing invalid JSON that will cause runtime errors when users switch from editor to builder mode
  • Pay close attention to dropdown.tsx - the regex bug on line 120 must be fixed before merge

Important Files Changed

File Analysis

FilenameScoreOverview
apps/sim/blocks/blocks/response.ts5/5Updated best practices to prefer builder mode over editor mode - simple documentation change
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/components/sub-block/components/starter/input-format.tsx5/5Added type guard for array values and improved placeholder text - safe defensive changes
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/components/sub-block/components/dropdown.tsx1/5Implements bidirectional mode conversion with refs to avoid stale closures, but regex bug will break JSON-to-builder conversion

Sequence Diagram

sequenceDiagram
participant User
participant Dropdown
participant Store
participant ResponseBlockHandler
User->>Dropdown: Select mode (structured/json)
Dropdown->>Dropdown: handleSelect(selectedValue)
Dropdown->>Store: setStoreValue(selectedValue)
Note over Dropdown: useEffect triggered by storeValue change
Dropdown->>Dropdown: Check if subBlockId === 'dataMode'
Dropdown->>Dropdown: Compare previousMode vs currentMode
alt Builder → Editor (structured → json)
Dropdown->>Dropdown: Get builderDataRef.current
Dropdown->>ResponseBlockHandler: convertBuilderDataToJsonString(builderData)
ResponseBlockHandler-->>Dropdown: jsonString
Dropdown->>Store: setData(jsonString)
else Editor → Builder (json → structured)
Dropdown->>Dropdown: Get dataRef.current
Dropdown->>Dropdown: normalizeVariableReferences(jsonString)
Note right of Dropdown: ⚠️ Regex bug: creates invalid JSON
Dropdown->>Dropdown: convertJsonToBuilderData(jsonString)
Dropdown-->>Dropdown: builderArray
Dropdown->>Store: setBuilderData(builderArray)
end
Dropdown->>Dropdown: Update previousModeRef.current
Loading

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +117 to +121
const normalizeVariableReferences = (jsonString: string): string => {
// Replace unquoted variable references with quoted ones
// Pattern: <variable.name> -> "<variable.name>"
return jsonString.replace(/([^"]<[^>]+>)/g, '"$1"')
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: regex pattern incorrectly captures the character before <, causing malformed JSON (e.g., : <var> becomes ": <var>""": <var>")

Suggested change
constnormalizeVariableReferences=(jsonString: string): string=>{
// Replace unquoted variable references with quoted ones
// Pattern: <variable.name> -> "<variable.name>"
returnjsonString.replace(/([^"]<[^>]+>)/g,'"$1"')
}
constnormalizeVariableReferences=(jsonString: string): string=>{
// Replace unquoted variable references with quoted ones
// Pattern: <variable.name> -> "<variable.name>"
returnjsonString.replace(/(?<!")(<[^>]+>)(?!")/g,'"$1"')
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/components/sub-block/components/dropdown.tsx
Line: 117:121
Comment:
**logic:** regex pattern incorrectly captures the character before `<`, causing malformed JSON (e.g., `: <var>` becomes `": <var>"``"": <var>"`)
```suggestion const normalizeVariableReferences = (jsonString: string): string => { // Replace unquoted variable references with quoted ones // Pattern: <variable.name> -> "<variable.name>" return jsonString.replace(/(?<!")(<[^>]+>)(?!")/g, '"$1"') }```
How can I resolve this? If you propose a fix, please make it concise.

@waleedlatif1
waleedlatif1 deleted the improvement/context-response-block branch October 30, 2025 06:22
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@icecrasher321