Merged
Conversation
…mbed the inner one
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis refactoring simplifies the main Interactive component by extracting job card rendering into a dedicated InteractiveJobCard component and introducing a new EmbeddableStreamingOutput component for streaming task output. Interactive modal components (Jupyter, VLLM, SSH, Ollama, VSCode) are updated to accept an embeddedOutput prop instead of onOpenOutput callbacks, enabling flexible embedded layouts. Changes
Sequence Diagram(s)sequenceDiagram
participant Job as Job List
participant Card as InteractiveJobCard
participant Modal as Interactive*Modal
participant Embedded as EmbeddableStreamingOutput
participant Output as PollingOutputTerminal
participant Logs as ProviderLogsTerminal
Job->>Card: Render job with delete callback
Card->>Card: Extract interactive_type from job_data
Card->>Card: Display icon, chip, Connect button
activate Card
Card->>Modal: onClick Connect → Open modal with jobId
activate Modal
Modal->>Embedded: Pass jobId to embeddedOutput prop
activate Embedded
Embedded->>Output: Render output tab
activate Output
Output-->>Embedded: Stream task output
deactivate Output
Embedded->>Logs: Render provider logs tab (if selected)
activate Logs
Logs->>Logs: Initialize xterm terminal
Logs-->>Embedded: Stream provider logs
deactivate Logs
deactivate Embedded
Modal->>Modal: Display status/URL info alongside embedded output
deactivate Modal
Card->>Card: Show Connect button for interactive jobs
deactivate Card
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Member
Author
Screen.Recording.2026-03-02.at.9.53.37.AM.mov |
deep1401
approved these changes
Mar 2, 2026
Paragon SummaryThis pull request review identified 8 issues across 3 categories in 10 files. The review analyzed code changes, potential bugs, security vulnerabilities, performance issues, and code quality concerns using automated analysis tools. This PR improves the UI of interactive task modals by consolidating output and instructions into a single unified screen and enhancing the visual presentation for a cleaner user experience. Key changes:
Confidence score: 3/5
10 files reviewed, 8 comments Severity breakdown: High: 1, Medium: 3, Low: 4 Tip: |
![paragon-review[bot]](https://avatars.githubusercontent.com/in/1544258?s=60&v=4){kind=small}
| size="sm" | ||
| onClick={() => setConnectOpen(true)} | ||
| > | ||
| Connect |
There was a problem hiding this comment.
Bug: EmbeddableStreamingOutput always renders when modal is closed
EmbeddableStreamingOutput always renders when modal is closed. Terminal instances mount for every card. Gate rendering on connectOpen state.
View Details
Location: src/renderer/components/Experiment/Interactive/InteractiveJobCard.tsx (lines 206)
Analysis
EmbeddableStreamingOutput always renders when modal is closed
| What fails | EmbeddableStreamingOutput (with xterm Terminal and SWR polling) is always rendered as embeddedOutput prop even when the ConnectModal is closed (jobId=-1). |
| Result | N terminal instances and N SWR polling loops run simultaneously for N job cards, even when no modal is open. |
| Expected | EmbeddableStreamingOutput should only mount when the Connect modal is actually open, avoiding unnecessary terminals and network requests. |
| Impact | Significant performance degradation with many interactive jobs — unnecessary DOM nodes, xterm instances, and API polling for every card. |
How to reproduce
Open the Interactive page with multiple job cards. Inspect React DevTree or network tab — every card mounts an EmbeddableStreamingOutput with active SWR polling.Patch Details
- embeddedOutput={
- <EmbeddableStreamingOutput jobId={jobIdNum} tabs={['provider']} />
- }
+ embeddedOutput={
+ connectOpen ? <EmbeddableStreamingOutput jobId={jobIdNum} tabs={['provider']} /> : undefined
+ }AI Fix Prompt
Fix this issue: EmbeddableStreamingOutput always renders when modal is closed. Terminal instances mount for every card. Gate rendering on connectOpen state.
Location: src/renderer/components/Experiment/Interactive/InteractiveJobCard.tsx (lines 206)
Problem: EmbeddableStreamingOutput (with xterm Terminal and SWR polling) is always rendered as embeddedOutput prop even when the ConnectModal is closed (jobId=-1).
Current behavior: N terminal instances and N SWR polling loops run simultaneously for N job cards, even when no modal is open.
Expected: EmbeddableStreamingOutput should only mount when the Connect modal is actually open, avoiding unnecessary terminals and network requests.
Steps to reproduce: Open the Interactive page with multiple job cards. Inspect React DevTree or network tab — every card mounts an EmbeddableStreamingOutput with active SWR polling.
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
| const jobData = job.job_data || {}; | ||
| const interactiveType = | ||
| jobData.interactive_type || | ||
| (typeof jobData === 'string' |
There was a problem hiding this comment.
Bug: JSON
JSON.parse in InteractiveJobCard has no try-catch. Malformed job_data string crashes the component. Wrap in try-catch with fallback to 'vscode'.
View Details
Location: src/renderer/components/Experiment/Interactive/InteractiveJobCard.tsx (lines 124)
Analysis
JSON.parse in InteractiveJobCard has no try-catch. Malformed job_data string crashes the component
| What fails | InteractiveJobCard crashes if job.job_data is a malformed JSON string, causing the entire card list to unmount. |
| Result | React error boundary catches unhandled JSON.parse exception, entire card list disappears. |
| Expected | Component should gracefully fall back to 'vscode' as the default interactive type. |
| Impact | One corrupted job record can break the entire Interactive jobs view for all users. |
How to reproduce
Set a job's job_data to an invalid JSON string like '{bad json'. Open the Interactive tab.Patch Details
- (typeof jobData === 'string'
- ? JSON.parse(jobData || '{}')?.interactive_type
+ (typeof jobData === 'string'
+ ? (() => { try { return JSON.parse(jobData || '{}')?.interactive_type; } catch { return null; } })()AI Fix Prompt
Fix this issue: JSON.parse in InteractiveJobCard has no try-catch. Malformed job_data string crashes the component. Wrap in try-catch with fallback to 'vscode'.
Location: src/renderer/components/Experiment/Interactive/InteractiveJobCard.tsx (lines 124)
Problem: InteractiveJobCard crashes if job.job_data is a malformed JSON string, causing the entire card list to unmount.
Current behavior: React error boundary catches unhandled JSON.parse exception, entire card list disappears.
Expected: Component should gracefully fall back to 'vscode' as the default interactive type.
Steps to reproduce: Set a job's job_data to an invalid JSON string like '{bad json'. Open the Interactive tab.
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
| onDeleteJob: (jobId: string) => void; | ||
| } | ||
|
|
||
| function VscodeIcon() { |
There was a problem hiding this comment.
Bug: Icon images load from external URL lab
Icon images load from external URL lab.cloud. Offline/air-gapped users see broken images. Bundle icons locally or add fallback.
View Details
Location: src/renderer/components/Experiment/Interactive/InteractiveJobCard.tsx (lines 27)
Analysis
Icon images load from external URL lab.cloud. Offline/air-gapped users see broken images
| What fails | All five icon components (VscodeIcon, JupyterIcon, SshIcon, VllmIcon, OllamaIcon) fetch images from https://lab\.cloud/img/icons/ at runtime. |
| Result | All interactive type icons show as broken image placeholders since the external URLs are unreachable. |
| Expected | Icons should render correctly in offline/air-gapped environments, either bundled locally or with graceful fallback. |
| Impact | Electron apps commonly run offline. Broken icons degrade the UI significantly for users without internet access. |
How to reproduce
Disconnect from the internet. Open the Interactive tab with active jobs.AI Fix Prompt
Fix this issue: Icon images load from external URL lab.cloud. Offline/air-gapped users see broken images. Bundle icons locally or add fallback.
Location: src/renderer/components/Experiment/Interactive/InteractiveJobCard.tsx (lines 27)
Problem: All five icon components (VscodeIcon, JupyterIcon, SshIcon, VllmIcon, OllamaIcon) fetch images from https://lab.cloud/img/icons/ at runtime.
Current behavior: All interactive type icons show as broken image placeholders since the external URLs are unreachable.
Expected: Icons should render correctly in offline/air-gapped environments, either bundled locally or with graceful fallback.
Steps to reproduce: Disconnect from the internet. Open the Interactive tab with active jobs.
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
| </Typography> | ||
| )} | ||
| </Stack> | ||
| {embeddedOutput ? ( |
There was a problem hiding this comment.
Maintainability: Massive UI code duplication in modal components
Massive UI code duplication in modal components. Each has identical embeddedOutput/fallback branches. Extract shared layout into a reusable wrapper.
View Details
Location: src/renderer/components/Experiment/Tasks/InteractiveJupyterModal.tsx (lines 100)
Analysis
Massive UI code duplication in modal components. Each has identical embeddedOutput/fallback branches
| What fails | Every interactive modal (VSCode, Jupyter, vLLM, Ollama, SSH) duplicates the full embeddedOutput two-pane layout vs fallback pattern, roughly doubling each component's size. |
| Result | 5 files each contain ~100-150 lines of duplicated layout code for the embeddedOutput vs fallback branching. |
| Expected | A shared InteractiveModalLayout component should handle the two-pane/single-pane branching, with each modal providing only its unique content. |
| Impact | Any layout bug must be fixed in 5 places. High maintenance burden and risk of divergence between modals. |
How to reproduce
Compare the ternary {embeddedOutput ? (...) : (...)} blocks across all five modal files. The outer structure is identical.AI Fix Prompt
Fix this issue: Massive UI code duplication in modal components. Each has identical embeddedOutput/fallback branches. Extract shared layout into a reusable wrapper.
Location: src/renderer/components/Experiment/Tasks/InteractiveJupyterModal.tsx (lines 100)
Problem: Every interactive modal (VSCode, Jupyter, vLLM, Ollama, SSH) duplicates the full embeddedOutput two-pane layout vs fallback pattern, roughly doubling each component's size.
Current behavior: 5 files each contain ~100-150 lines of duplicated layout code for the embeddedOutput vs fallback branching.
Expected: A shared InteractiveModalLayout component should handle the two-pane/single-pane branching, with each modal providing only its unique content.
Steps to reproduce: Compare the ternary {embeddedOutput ? (...) : (...)} blocks across all five modal files. The outer structure is identical.
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
|
|
||
| interface InteractiveJobCardProps { | ||
| job: any; | ||
| onDeleteJob: (jobId: string) => void; |
There was a problem hiding this comment.
Bug: Icon components receive a size prop but render plain img tags
Icon components receive a size prop but render plain img tags. This prop is silently ignored. Remove the size prop or use it for width/height.
View Details
Location: src/renderer/components/Experiment/Interactive/InteractiveJobCard.tsx (lines 24)
Analysis
Icon components receive a size prop but render plain img tags. This prop is silently ignored
| What fails | TypeIcon is invoked with but VscodeIcon, JupyterIcon, etc. don't accept a 'size' prop — they render hardcoded |
| Result | The size={20} prop is passed to the img-based component but silently ignored. Icon size is always hardcoded to 20x20. |
| Expected | Either the icon components should accept and use a size prop, or the call site should not pass one. |
| Impact | Misleading API — developers may believe they can resize icons but nothing changes. React also passes unknown props to the DOM. |
How to reproduce
Inspect <TypeIcon size={20} /> in InteractiveJobCard. Change size to 40 and observe — icon stays 20px.AI Fix Prompt
Fix this issue: Icon components receive a `size` prop but render plain img tags. This prop is silently ignored. Remove the size prop or use it for width/height.
Location: src/renderer/components/Experiment/Interactive/InteractiveJobCard.tsx (lines 24)
Problem: TypeIcon is invoked with <TypeIcon size={20} /> but VscodeIcon, JupyterIcon, etc. don't accept a 'size' prop — they render hardcoded <img width={20} height={20}>.
Current behavior: The size={20} prop is passed to the img-based component but silently ignored. Icon size is always hardcoded to 20x20.
Expected: Either the icon components should accept and use a size prop, or the call site should not pass one.
Steps to reproduce: Inspect <TypeIcon size={20} /> in InteractiveJobCard. Change size to 40 and observe — icon stays 20px.
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
| useEffect(() => { | ||
| if (!termRef.current) return; | ||
|
|
||
| const text = logsText || 'No provider log data yet.'; |
There was a problem hiding this comment.
Bug: ProviderLogsTerminal clears and rewrites all lines on every SWR refresh
ProviderLogsTerminal clears and rewrites all lines on every SWR refresh. Causes visual flicker. Use incremental write approach instead.
View Details
Location: src/renderer/components/Experiment/Tasks/EmbeddableStreamingOutput.tsx (lines 75)
Analysis
ProviderLogsTerminal clears and rewrites all lines on every SWR refresh. Causes visual flicker
| What fails | ProviderLogsTerminal clears the entire xterm screen (\x1b[2J\x1b[H) and rewrites all lines every time logsText changes from SWR polling. |
| Result | Terminal flashes/flickers as entire content is cleared and rewritten every 2-5 seconds during active polling. |
| Expected | Terminal should diff or append only new content, avoiding full clear-and-rewrite on each update. |
| Impact | Poor user experience with visible flicker in the terminal output, especially during active log streaming. |
How to reproduce
Open an interactive task with provider logs visible. Watch the terminal output — it flickers on every SWR poll interval.AI Fix Prompt
Fix this issue: ProviderLogsTerminal clears and rewrites all lines on every SWR refresh. Causes visual flicker. Use incremental write approach instead.
Location: src/renderer/components/Experiment/Tasks/EmbeddableStreamingOutput.tsx (lines 75)
Problem: ProviderLogsTerminal clears the entire xterm screen (\x1b[2J\x1b[H) and rewrites all lines every time logsText changes from SWR polling.
Current behavior: Terminal flashes/flickers as entire content is cleared and rewritten every 2-5 seconds during active polling.
Expected: Terminal should diff or append only new content, avoiding full clear-and-rewrite on each update.
Steps to reproduce: Open an interactive task with provider logs visible. Watch the terminal output — it flickers on every SWR poll interval.
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
| ); | ||
| } | ||
|
|
||
| EmbeddableStreamingOutput.defaultProps = { |
There was a problem hiding this comment.
Bug: React
React.defaultProps is deprecated since React 18.3. Triggers console warnings. Use JS default params instead.
View Details
Location: src/renderer/components/Experiment/Tasks/EmbeddableStreamingOutput.tsx (lines 335)
Analysis
React.defaultProps is deprecated since React 18.3. Triggers console warnings
| What fails | EmbeddableStreamingOutput uses React.defaultProps which is deprecated since React 18.3 and will be removed in React 19. |
| Result | Console logs 'EmbeddableStreamingOutput: Support for defaultProps will be removed' deprecation warning. |
| Expected | No deprecation warnings. Default props should use JS destructuring defaults (which are already present on line 119). |
| Impact | Will break on React 19 upgrade. The default is already set via destructuring, making this line redundant. |
How to reproduce
Open browser console while rendering EmbeddableStreamingOutput. Look for deprecation warnings.Patch Details
-EmbeddableStreamingOutput.defaultProps = {
- tabs: ['output', 'provider'],
-};AI Fix Prompt
Fix this issue: React.defaultProps is deprecated since React 18.3. Triggers console warnings. Use JS default params instead.
Location: src/renderer/components/Experiment/Tasks/EmbeddableStreamingOutput.tsx (lines 335)
Problem: EmbeddableStreamingOutput uses React.defaultProps which is deprecated since React 18.3 and will be removed in React 19.
Current behavior: Console logs 'EmbeddableStreamingOutput: Support for defaultProps will be removed' deprecation warning.
Expected: No deprecation warnings. Default props should use JS destructuring defaults (which are already present on line 119).
Steps to reproduce: Open browser console while rendering EmbeddableStreamingOutput. Look for deprecation warnings.
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
| import InteractiveOllamaModal from '../Tasks/InteractiveOllamaModal'; | ||
| import EmbeddableStreamingOutput from '../Tasks/EmbeddableStreamingOutput'; | ||
|
|
||
| interface InteractiveJobCardProps { |
There was a problem hiding this comment.
Bug: InteractiveJobCard uses job: any prop type
InteractiveJobCard uses job: any prop type. Weakens TypeScript safety throughout the component. Define a proper Job interface.
View Details
Location: src/renderer/components/Experiment/Interactive/InteractiveJobCard.tsx (lines 22)
Analysis
InteractiveJobCard uses job: any prop type. Weakens TypeScript safety throughout the component
| What fails | The InteractiveJobCardProps interface types 'job' as 'any', defeating TypeScript's type checking for all job property accesses in the component. |
| Result | No compile-time errors if job properties are misspelled or accessed incorrectly. Runtime errors possible. |
| Expected | A proper Job interface should be defined with known fields (id, status, job_data, etc.) to enable type checking. |
| Impact | Reduces TypeScript value; property access bugs won't be caught at compile time. AGENTS.md states: 'Avoid any. Define interfaces.' |
How to reproduce
Review the InteractiveJobCard component. Note job.job_data, job.status, job.id are all accessed without type safety.AI Fix Prompt
Fix this issue: InteractiveJobCard uses `job: any` prop type. Weakens TypeScript safety throughout the component. Define a proper Job interface.
Location: src/renderer/components/Experiment/Interactive/InteractiveJobCard.tsx (lines 22)
Problem: The InteractiveJobCardProps interface types 'job' as 'any', defeating TypeScript's type checking for all job property accesses in the component.
Current behavior: No compile-time errors if job properties are misspelled or accessed incorrectly. Runtime errors possible.
Expected: A proper Job interface should be defined with known fields (id, status, job_data, etc.) to enable type checking.
Steps to reproduce: Review the InteractiveJobCard component. Note job.job_data, job.status, job.id are all accessed without type safety.
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
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
2 participants
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 by CodeRabbit
New Features
UI Improvements