feat(terminal): structured output#3026
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile OverviewGreptile SummaryThis PR introduces structured output visualization for the terminal with two major components: a new collapsible tree view for nested JSON data and JSON collapse functionality in the code viewer. Key Changes
Previous Review NotesSeveral edge cases in the JSON parsing logic were identified in previous review threads and remain unaddressed:
These issues could cause incorrect collapse regions in edge cases but are unlikely to affect typical workflow outputs. Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant WorkflowExecution
participant ConsoleStore
participant Terminal
participant OutputPanel
participant StructuredOutput
User->>WorkflowExecution: Execute workflow
WorkflowExecution->>ConsoleStore: addConsole(blockId, isRunning=true)
ConsoleStore->>Terminal: Notify new entry
Terminal->>OutputPanel: Display running state
OutputPanel->>StructuredOutput: Render with isRunning=true
StructuredOutput-->>OutputPanel: Show "Running" badge
Note over WorkflowExecution: Block executes...
WorkflowExecution->>ConsoleStore: updateConsole(blockId, output, isRunning=false)
ConsoleStore->>Terminal: Update entry
Terminal->>OutputPanel: Re-render with output
alt Structured View Enabled
OutputPanel->>StructuredOutput: Render output data
StructuredOutput-->>OutputPanel: Nested collapsible structure
else Code View
OutputPanel->>Code.Viewer: Render JSON string
Code.Viewer-->>OutputPanel: Syntax highlighted JSON with collapse
end
User->>OutputPanel: Toggle search
OutputPanel->>StructuredOutput: searchQuery prop
StructuredOutput->>StructuredOutput: Find matches & highlight
StructuredOutput->>StructuredOutput: Auto-expand to current match
StructuredOutput-->>User: Scroll to highlighted match
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for (const char of line) { | ||
| if (char === '{' || char === '[') { | ||
| stack.push({ char, line: i }) | ||
| } else if (char === '}' || char === ']') { | ||
| const expected = char === '}' ? '{' : '[' | ||
| if (stack.length > 0 && stack[stack.length - 1].char === expected) { | ||
| const start = stack.pop()! | ||
| // Only create a region if it spans multiple lines | ||
| if (i > start.line) { | ||
| regions.set(start.line, { | ||
| startLine: start.line, | ||
| endLine: i, | ||
| type: 'block', | ||
| }) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The brace matching logic doesn't account for braces inside JSON strings, which will cause incorrect parsing. For example, {"key": "value with { brace"} will incorrectly match the brace inside the string.
This will cause incorrect collapse regions and potentially break the collapsing functionality.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/components/emcn/components/code/code.tsx
Line: 109:126
Comment:
The brace matching logic doesn't account for braces inside JSON strings, which will cause incorrect parsing. For example, `{"key": "value with { brace"}` will incorrectly match the brace inside the string.
This will cause incorrect collapse regions and potentially break the collapsing functionality.
How can I resolve this? If you propose a fix, please make it concise.| * Regex to match a JSON string value (key: "value" pattern). | ||
| * Pre-compiled for performance. | ||
| */ | ||
| const STRING_VALUE_REGEX = /:\s*"([^"\\]|\\.)*"[,]?\s*$/ |
There was a problem hiding this comment.
The regex doesn't handle escaped quotes within the string value correctly. For example, "key": "value with \" quote" will incorrectly match up to the escaped quote rather than the closing quote.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/components/emcn/components/code/code.tsx
Line: 76:76
Comment:
The regex doesn't handle escaped quotes within the string value correctly. For example, `"key": "value with \" quote"` will incorrectly match up to the escaped quote rather than the closing quote.
How can I resolve this? If you propose a fix, please make it concise.| const stringMatch = line.match(STRING_VALUE_REGEX) | ||
| if (stringMatch) { | ||
| const colonIdx = line.indexOf('":') | ||
| if (colonIdx !== -1) { | ||
| const valueStart = line.indexOf('"', colonIdx + 1) | ||
| const valueEnd = line.lastIndexOf('"') | ||
| if (valueStart !== -1 && valueEnd > valueStart) { | ||
| const stringValue = line.slice(valueStart + 1, valueEnd) | ||
| // Check if string is long enough or contains escaped newlines | ||
| if (stringValue.length >= MIN_COLLAPSIBLE_STRING_LENGTH || stringValue.includes('\\n')) { | ||
| regions.set(i, { startLine: i, endLine: i, type: 'string' }) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The string detection logic processes string regions before block regions in the same line, which could cause conflicts. If a line has both a long string value AND an opening brace, both will be added to the regions map with the same key (line index), causing the block region to overwrite the string region.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/components/emcn/components/code/code.tsx
Line: 92:106
Comment:
The string detection logic processes string regions before block regions in the same line, which could cause conflicts. If a line has both a long string value AND an opening brace, both will be added to the regions map with the same key (line index), causing the block region to overwrite the string region.
How can I resolve this? If you propose a fix, please make it concise.| const suffix = line.charCodeAt(line.length - 1) === 44 /* ',' */ ? '",' : '"' | ||
| const truncated = line.slice(valueStart + 1, valueStart + 31) |
There was a problem hiding this comment.
Magic number 31 used without explanation. Consider extracting to a named constant like MAX_TRUNCATED_STRING_LENGTH for clarity.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/components/emcn/components/code/code.tsx
Line: 200:201
Comment:
Magic number 31 used without explanation. Consider extracting to a named constant like `MAX_TRUNCATED_STRING_LENGTH` for clarity.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
|
@greptile |
| const prevChar = j > 0 ? line[j - 1] : '' | ||
|
|
||
| // Toggle string state on unescaped quotes | ||
| if (char === '"' && prevChar !== '\\') { |
There was a problem hiding this comment.
Escape sequence handling is incomplete. prevChar !== '\\' doesn't handle escaped backslashes (\\"). For example, "value with \\\\" quote" will incorrectly toggle string state at the escaped quote.
Need to count consecutive backslashes to determine if the quote is truly escaped.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/components/emcn/components/code/code.tsx
Line: 123:123
Comment:
Escape sequence handling is incomplete. `prevChar !== '\\'` doesn't handle escaped backslashes (`\\"`). For example, `"value with \\\\" quote"` will incorrectly toggle string state at the escaped quote.
Need to count consecutive backslashes to determine if the quote is truly escaped.
How can I resolve this? If you propose a fix, please make it concise.
|
@greptile |
|
@greptile |
emir-karabeg
changed the title
|
@greptile |
emir-karabeg
force-pushed
the
feat/terminal-structured-output
branch
from
2e29dbb to
3d51849
Compare
|
@greptile |
1 participant
Summary
Type of Change
Testing
Solo.
Checklist