Conversation
added 4 commits
March 23, 2026 03:51
- Add initialStatus prop to JobStatus component - Initialize pollStatus from initialStatus instead of hardcoded "pending" - Pass initialStatus from App.tsx for selectedJob and uploadResult - Add tests for initialStatus prop behavior This prevents unnecessary polling when a completed job is selected from the Jobs view. Previously, JobStatus always initialized pollStatus to "pending" on mount, causing polling to start even for terminal states. Fixes #363
- Add viewMode prop to JobList component - Only fetch jobs when viewMode === 'jobs' - Re-fetch when viewMode changes to 'jobs' - Pass viewMode from App.tsx - Add tests for viewMode prop behavior This fixes the "failed to fetch" stuck state after single-tool video upload. Previously, JobList only fetched once on mount with useEffect([]), causing it to get stuck in error state if the initial fetch failed. Fixes #365
- Add videoPath state to VideoUpload component - Skip upload if videoPath already set (re-use uploaded video) - Clear videoPath when different file selected - Show "Video uploaded" status indicator - Add tests for videoPath re-use and file change behavior This allows users to run multiple jobs on the same video without re-uploading each time. The video is uploaded once and the path is re-used for consecutive Run Job clicks. Fixes #366
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds frontend state-flow documentation; prevents redundant job polling via an Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant VideoComp as VideoUpload (web‑ui)
participant App as App (web‑ui)
participant API as Server API
participant JobStatus as JobStatus (web‑ui)
User->>VideoComp: select file
User->>VideoComp: click "Run Job"
VideoComp->>App: onRunJob(file, maybe videoPath)
App->>App: clear uploadResult, selectedJob
alt videoPath missing
App->>API: POST /v1/video/upload
API-->>App: { video_path }
end
App->>API: POST /v1/video/job
API-->>App: { job_id, status }
App->>JobStatus: render with initialStatus = job.status
alt initialStatus is terminal
Note right of JobStatus: no HTTP polling started
else
JobStatus->>API: poll GET /v1/jobs/:job_id (interval)
API-->>JobStatus: status updates...
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
web-ui/src/components/VideoUpload.tsx (1)
153-156: Good file change detection, but consider edge case with modified files.The comparison uses
nameandsizeto detect file changes. This works for typical use cases, but if a user modifies a file externally and re-selects it with the same name and size, the cachedvideoPathwon't be cleared.This is an acceptable trade-off for simplicity, but consider adding
lastModifiedto the comparison for more robust detection if this becomes an issue.♻️ Optional: Include lastModified in comparison
// v0.16.4: Clear videoPath if different file selected - if (file && (f.name !== file.name || f.size !== file.size)) { + if (file && (f.name !== file.name || f.size !== file.size || f.lastModified !== file.lastModified)) { setVideoPath(null); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-ui/src/components/VideoUpload.tsx` around lines 153 - 156, The current file-change check only compares f.name and f.size and may miss externally modified files with identical name/size; update the conditional in the VideoUpload component to also compare f.lastModified (using the existing variables file, f, videoPath and setVideoPath) so that when f.name !== file.name || f.size !== file.size || f.lastModified !== file.lastModified you clear the cached videoPath by calling setVideoPath(null).web-ui/src/components/JobList.tsx (1)
18-38: Consider resettingloadingstate when fetch is skipped.When
viewModeis not"jobs"(line 20), the effect returns early without resettingloading. If the component previously started loading data,loadingmight remaintrueafter navigating away, causing a stale "Loading jobs..." message when returning to the jobs view before the new fetch completes.♻️ Proposed fix to reset loading state
useEffect(() => { // Issue `#365`: Only fetch when viewMode is 'jobs' (or not provided for backward compat) - if (viewMode !== undefined && viewMode !== "jobs") return; + if (viewMode !== undefined && viewMode !== "jobs") { + return; + } + setLoading(true); const loadJobs = async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-ui/src/components/JobList.tsx` around lines 18 - 38, The useEffect currently returns early when viewMode is defined and not "jobs", leaving loading possibly stuck true; update the effect to reset loading (call setLoading(false)) before the early return so the component doesn't show stale loading state when switching away, e.g., in the effect that references viewMode and defines loadJobs, invoke setLoading(false) right before the `return` and ensure setLoading(true) is still called when starting loadJobs; modify the useEffect that contains useEffect, viewMode, loadJobs, setLoading, setJobs, setError accordingly.docs/design/frontend-state-flow.md (1)
48-54: Add language specifiers to fenced code blocks.Several code blocks lack language specifiers, which helps with syntax highlighting and accessibility. Consider adding
textor appropriate language identifiers.For example:
-``` +```text PluginSelector mount └─ useEffect([]) → apiClient.getPlugins()Also applies to: 72-79, 102-108, 202-213, 237-249, 275-281, 306-312, 338-350, 353-361, 440-474
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design/frontend-state-flow.md` around lines 48 - 54, The markdown fenced code blocks in docs/design/frontend-state-flow.md (e.g., the block showing "PluginSelector mount" with useEffect([]) → apiClient.getPlugins() and GET /v1/plugins) are missing language specifiers; update each fenced block by adding an appropriate language tag (use "text" for plain flow diagrams or a specific language if applicable) to improve syntax highlighting and accessibility—apply this change to the blocks around the ranges referenced (including those showing useEffect, apiClient.getPlugins, and other flow snippets at 72-79, 102-108, 202-213, 237-249, 275-281, 306-312, 338-350, 353-361, 440-474).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/frontend-state-flow.md`:
- Around line 407-416: The "Root Causes" section has duplicate subsection
numbering: change the heading "### 3. Polling in Multiple Layers" (the one whose
text begins "Problem: App.tsx polls AND JobStatus.tsx polls. No coordination.")
to "### 4. Polling in Multiple Layers" so headings increment correctly and no
duplicate "3." appears; update only that heading text (the headings "Duplicate
Polling State" and "Polling in Multiple Layers" identify the locations).
---
Nitpick comments:
In `@docs/design/frontend-state-flow.md`:
- Around line 48-54: The markdown fenced code blocks in
docs/design/frontend-state-flow.md (e.g., the block showing "PluginSelector
mount" with useEffect([]) → apiClient.getPlugins() and GET /v1/plugins) are
missing language specifiers; update each fenced block by adding an appropriate
language tag (use "text" for plain flow diagrams or a specific language if
applicable) to improve syntax highlighting and accessibility—apply this change
to the blocks around the ranges referenced (including those showing useEffect,
apiClient.getPlugins, and other flow snippets at 72-79, 102-108, 202-213,
237-249, 275-281, 306-312, 338-350, 353-361, 440-474).
In `@web-ui/src/components/JobList.tsx`:
- Around line 18-38: The useEffect currently returns early when viewMode is
defined and not "jobs", leaving loading possibly stuck true; update the effect
to reset loading (call setLoading(false)) before the early return so the
component doesn't show stale loading state when switching away, e.g., in the
effect that references viewMode and defines loadJobs, invoke setLoading(false)
right before the `return` and ensure setLoading(true) is still called when
starting loadJobs; modify the useEffect that contains useEffect, viewMode,
loadJobs, setLoading, setJobs, setError accordingly.
In `@web-ui/src/components/VideoUpload.tsx`:
- Around line 153-156: The current file-change check only compares f.name and
f.size and may miss externally modified files with identical name/size; update
the conditional in the VideoUpload component to also compare f.lastModified
(using the existing variables file, f, videoPath and setVideoPath) so that when
f.name !== file.name || f.size !== file.size || f.lastModified !==
file.lastModified you clear the cached videoPath by calling setVideoPath(null).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4d97a787-d4d3-466c-8fee-a43e1c365d98
📒 Files selected for processing (8)
docs/design/frontend-state-flow.mdweb-ui/src/App.tsxweb-ui/src/components/JobList.test.tsxweb-ui/src/components/JobList.tsxweb-ui/src/components/JobStatus.test.tsxweb-ui/src/components/JobStatus.tsxweb-ui/src/components/VideoUpload.test.tsxweb-ui/src/components/VideoUpload.tsx
added 3 commits
March 23, 2026 22:46
- Clear uploadResult and selectedJob at START of handleRunVideoJob - Prevents old 'completed' status from flashing when running consecutive jobs - Add TDD test for Issue #369
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
web-ui/src/App.run-job.test.tsx (1)
320-344: Make this suite’s mocks fully self-contained to avoid order coupling.Line 320-344 does not initialise
mockGetJob, so behaviour may depend on implementations left by earlier suites. Add explicit defaults in thisbeforeEachfor determinism.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-ui/src/App.run-job.test.tsx` around lines 320 - 344, The test suite's beforeEach sets up many mocks but omits initialization of mockGetJob, which leaves behavior dependent on other tests; update the beforeEach (the setup block that currently initializes mockGetPlugins, mockGetPluginManifest, mockSubmitVideoUpload, and mockListJobs) to explicitly set mockGetJob.mockResolvedValue(...) to a sensible default (e.g., null or an empty job object matching expected shape) so the suite is self-contained and deterministic; ensure the chosen default aligns with how the tests assert job state.web-ui/src/App.tsx (1)
574-577: Gate navigation debug logs to development mode.Line 575-577 and Line 583 will log on every tab click in production. Please gate these behind
import.meta.env.DEV(or remove once validated).♻️ Suggested tweak
onClick={() => { - // Issue `#368`: Debug logging for navigation state changes - console.log("[NAV] viewMode changing:", viewMode, "->", mode); - console.log("[NAV] selectedJob before:", selectedJob?.job_id, selectedJob?.status); + if (import.meta.env.DEV) { + console.log("[NAV] viewMode changing:", viewMode, "->", mode); + console.log("[NAV] selectedJob before:", selectedJob?.job_id, selectedJob?.status); + } setViewMode(mode); // PERFORMANCE: Clear selectedJob when entering video modes // to prevent dragging huge video_multi jobs into the video flow // which would cause UI freeze in ResultsPanel if (mode === "video-upload" || mode === "video-stream") { setSelectedJob(null); - console.log("[NAV] selectedJob cleared"); + if (import.meta.env.DEV) { + console.log("[NAV] selectedJob cleared"); + } } }}Also applies to: 583-583
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-ui/src/App.tsx` around lines 574 - 577, The debug console.log calls around the navigation state change (referencing viewMode, mode, selectedJob and the setViewMode call) should be gated to development only; wrap the console.log statements (the ones before calling setViewMode and the other occurrence later) in a runtime check using import.meta.env.DEV (e.g. if (import.meta.env.DEV) { ... }) so they do not run in production, or remove them entirely after validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web-ui/src/App.run-job.test.tsx`:
- Around line 350-440: The test currently only checks mockSubmitVideoJob was
called twice but doesn't assert the UI/state was cleared immediately; update the
test around the second click (after fireEvent.click(runJobButton) and before
resolveSecondJob) to assert that the previous job UI/state is gone by checking
the component state or DOM: verify uploadResult and/or selectedJob are
null/empty (or that the previous job-id/status text is not present via
screen.queryByText or similar) immediately after the second click and before
resolving the second promise; keep references to runJobButton,
mockSubmitVideoJob, resolveSecondJob and the DOM queries used earlier to locate
where to add this assertion.
---
Nitpick comments:
In `@web-ui/src/App.run-job.test.tsx`:
- Around line 320-344: The test suite's beforeEach sets up many mocks but omits
initialization of mockGetJob, which leaves behavior dependent on other tests;
update the beforeEach (the setup block that currently initializes
mockGetPlugins, mockGetPluginManifest, mockSubmitVideoUpload, and mockListJobs)
to explicitly set mockGetJob.mockResolvedValue(...) to a sensible default (e.g.,
null or an empty job object matching expected shape) so the suite is
self-contained and deterministic; ensure the chosen default aligns with how the
tests assert job state.
In `@web-ui/src/App.tsx`:
- Around line 574-577: The debug console.log calls around the navigation state
change (referencing viewMode, mode, selectedJob and the setViewMode call) should
be gated to development only; wrap the console.log statements (the ones before
calling setViewMode and the other occurrence later) in a runtime check using
import.meta.env.DEV (e.g. if (import.meta.env.DEV) { ... }) so they do not run
in production, or remove them entirely after validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67d7b026-86a6-45cd-8a0a-370295ad9df9
📒 Files selected for processing (2)
web-ui/src/App.run-job.test.tsxweb-ui/src/App.tsx
added 2 commits
March 24, 2026 05:33
- Add assertions to verify Job Processing section appears after first job - Add critical assertion to verify old job UI is cleared BEFORE second response - Add assertion to verify Job Processing reappears after second resolve - Fix duplicate heading number in frontend-state-flow.md
- JobList.tsx: Set loading=false when viewMode is not 'jobs' - JobList.tsx: Set loading=true when starting fetch - frontend-state-flow.md: Update pollStatus description to reflect initialStatus fix - frontend-state-flow.md: Mark action items as completed
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/frontend-state-flow.md`:
- Around line 48-54: The fenced code blocks in
docs/design/frontend-state-flow.md lack language identifiers (MD040); update
each triple-backtick fence (e.g., the block showing "PluginSelector mount" and
other blocks at ranges referenced: 48-54, 72-80, 102-108, 202-213, 237-249,
275-281, 306-312, 338-350, 353-361, 440-474) to include an appropriate language
tag such as text or typescript (e.g., ```text or ```typescript) so the linter
passes and readability improves; ensure you add the tag to both opening fences
for each block and keep content unchanged.
- Around line 167-194: Documentation is inconsistent about JobStatus.tsx initial
poll status: update the doc sections that state pollStatus always initializes to
"pending" (lines referenced around the old snippets) to reflect the current
behavior using initialStatus (i.e., that pollStatus now uses initialStatus to
preserve job state across remounts), and remove or correct the claims that
polling restarts on every remount; specifically reference JobStatus.tsx, the
pollStatus state and initialStatus prop/variable and adjust the paragraphs at
the impacted locations (around the earlier 170/192 and 471+ and 367-385 areas)
so the document consistently describes the new initialStatus-based
initialization and the conditions that stop polling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 88fcef59-d1df-4eff-8331-1a13a0ecb590
📒 Files selected for processing (2)
docs/design/frontend-state-flow.mdweb-ui/src/App.run-job.test.tsx
Comment on lines
+48
to
+54
| ``` | ||
| PluginSelector mount | ||
| └─ useEffect([]) → apiClient.getPlugins() | ||
| └─ GET /v1/plugins | ||
| ├─ SUCCESS → setPlugins(data), setLoading(false) | ||
| └─ FAIL → setError(msg), setPlugins([]) | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
Several fences are missing a language tag (MD040). Please label them (for example text or typescript) to satisfy linting and improve readability.
🛠️ Example fix pattern
-```
+```text
PluginSelector mount
└─ useEffect([]) → apiClient.getPlugins()
...
-```
+```
-```
+```text
┌─────────────────────────────────────────────────────────────┐
│ App.tsx │
...
-```
+```Also applies to: 72-80, 102-108, 202-213, 237-249, 275-281, 306-312, 338-350, 353-361, 440-474
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/design/frontend-state-flow.md` around lines 48 - 54, The fenced code
blocks in docs/design/frontend-state-flow.md lack language identifiers (MD040);
update each triple-backtick fence (e.g., the block showing "PluginSelector
mount" and other blocks at ranges referenced: 48-54, 72-80, 102-108, 202-213,
237-249, 275-281, 306-312, 338-350, 353-361, 440-474) to include an appropriate
language tag such as text or typescript (e.g., ```text or ```typescript) so the
linter passes and readability improves; ensure you add the tag to both opening
fences for each block and keep content unchanged.
rogermt
added
bug
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web-ui/src/components/JobList.tsx (1)
29-44:⚠️ Potential issue | 🟠 MajorAdd retry/backoff around
listJobsto handle transient failures.
apiClient.listJobs()is currently single-attempt. A temporary network/server blip goes straight to error state with no recovery attempt.Suggested patch
const loadJobs = async () => { - console.log("[JOBLIST] Fetching jobs..."); - try { - const data = await apiClient.listJobs(); - console.log("[JOBLIST] Success:", data.length, "jobs"); - setJobs(data); - setError(null); - } catch (err) { - console.error("[JOBLIST] Error:", err); - setError( - err instanceof Error ? err.message : "Failed to load jobs" - ); - setJobs([]); - } finally { - setLoading(false); - } + console.log("[JOBLIST] Fetching jobs..."); + const maxAttempts = 3; + let attempt = 0; + let delayMs = 400; + + while (attempt < maxAttempts) { + try { + const data = await apiClient.listJobs(); + console.log("[JOBLIST] Success:", data.length, "jobs"); + setJobs(data); + setError(null); + return; + } catch (err) { + attempt += 1; + if (attempt >= maxAttempts) { + console.error("[JOBLIST] Error:", err); + setError( + err instanceof Error ? err.message : "Failed to load jobs" + ); + setJobs([]); + return; + } + await new Promise((resolve) => setTimeout(resolve, delayMs)); + delayMs *= 2; + } finally { + if (attempt >= maxAttempts) setLoading(false); + } + } };As per coding guidelines "Wrap all operations involving external services, APIs, or LLMs in retry logic to handle transient failures", "Implement retries with exponential backoff", and "Design systems with backup routes or default behaviors if a primary external call fails after retry attempts".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-ui/src/components/JobList.tsx` around lines 29 - 44, The loadJobs function currently calls apiClient.listJobs() once and fails immediately; modify loadJobs to implement retry with exponential backoff (e.g., maxAttempts 3-5, backoffFactor 2, baseDelay ~200-500ms) around apiClient.listJobs(), retrying on transient/network errors and breaking early on non-retryable failures, and ensure setJobs, setError and setLoading are updated only after final success or final failure; use a small delay utility (or setTimeout/Promise) to implement backoff and include cancellation/timeout awareness if available from apiClient.
♻️ Duplicate comments (2)
docs/design/frontend-state-flow.md (2)
276-283:⚠️ Potential issue | 🟡 MinorJobStatus behaviour is described with outdated polling-start logic.
These sections still state fresh mounts start from
pollStatus="pending"for completed jobs, which conflicts with theinitialStatus-based behaviour now inJobStatus.tsx.Suggested wording direction
- JobStatus mounts with pollStatus="pending" - └─ Triggers JobStatus HTTP polling (BUG!) + JobStatus mounts with pollStatus initialised from initialStatus + └─ Polling starts only when status is non-terminalAlso applies to: 353-362
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design/frontend-state-flow.md` around lines 276 - 283, Update the doc text that describes JobStatus polling so it matches the current implementation: replace claims that fresh mounts always start from pollStatus="pending" with wording that JobStatus uses the initialStatus prop to decide whether to start polling (i.e., only non-terminal initialStatus triggers polling), and adjust the flow diagram entries referencing JobList onClick → onJobSelect(job) → setSelectedJob and App.selectedJob polling and JobStatus mounts to reference initialStatus instead of pollStatus; ensure the same correction is applied to the duplicate section later (the block currently at the alternate location describing the same logic).
48-54:⚠️ Potential issue | 🟡 MinorAdd language identifiers to all fenced code blocks.
Several fences are still untyped, which keeps MD040 warnings active.
Fix pattern
-``` +```text PluginSelector mount └─ useEffect([]) → apiClient.getPlugins() ... -``` +```Also applies to: 72-80, 102-108, 203-214, 238-250, 276-283, 307-313, 339-351, 354-362, 441-475
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design/frontend-state-flow.md` around lines 48 - 54, Several fenced code blocks in the document are untyped (e.g., the "PluginSelector mount" block shown) which triggers MD040; update every triple-backtick fence to include a language identifier (use "text" for these flow diagrams) — e.g., change ``` to ```text for the shown block and the other untyped fences referenced (72-80, 102-108, 203-214, 238-250, 276-283, 307-313, 339-351, 354-362, 441-475) so all fenced blocks have explicit language tags.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/frontend-state-flow.md`:
- Around line 102-115: The JobList flow in the diagram is wrong: it should
document a viewMode-triggered fetch (useEffect([viewMode])) instead of a
mount-only useEffect([]); update the sequence for JobList to show
useEffect([viewMode]) → apiClient.listJobs() with the same GET/Success/Fail
branches, change the "Polling Required" and "Dependencies" notes to reflect that
fetch is triggered by viewMode (not strictly mount-only), and mark Issue `#365` as
resolved/fixed in the diagram text; apply the same corrections to the other
occurrences referencing the JobList flow (the other two sections mentioned) so
all three places consistently reference viewMode-triggered fetching and Issue
`#365` state.
---
Outside diff comments:
In `@web-ui/src/components/JobList.tsx`:
- Around line 29-44: The loadJobs function currently calls apiClient.listJobs()
once and fails immediately; modify loadJobs to implement retry with exponential
backoff (e.g., maxAttempts 3-5, backoffFactor 2, baseDelay ~200-500ms) around
apiClient.listJobs(), retrying on transient/network errors and breaking early on
non-retryable failures, and ensure setJobs, setError and setLoading are updated
only after final success or final failure; use a small delay utility (or
setTimeout/Promise) to implement backoff and include cancellation/timeout
awareness if available from apiClient.
---
Duplicate comments:
In `@docs/design/frontend-state-flow.md`:
- Around line 276-283: Update the doc text that describes JobStatus polling so
it matches the current implementation: replace claims that fresh mounts always
start from pollStatus="pending" with wording that JobStatus uses the
initialStatus prop to decide whether to start polling (i.e., only non-terminal
initialStatus triggers polling), and adjust the flow diagram entries referencing
JobList onClick → onJobSelect(job) → setSelectedJob and App.selectedJob polling
and JobStatus mounts to reference initialStatus instead of pollStatus; ensure
the same correction is applied to the duplicate section later (the block
currently at the alternate location describing the same logic).
- Around line 48-54: Several fenced code blocks in the document are untyped
(e.g., the "PluginSelector mount" block shown) which triggers MD040; update
every triple-backtick fence to include a language identifier (use "text" for
these flow diagrams) — e.g., change ``` to ```text for the shown block and the
other untyped fences referenced (72-80, 102-108, 203-214, 238-250, 276-283,
307-313, 339-351, 354-362, 441-475) so all fenced blocks have explicit language
tags.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 63114db4-e6cf-42b9-8551-31215c3ffe4c
📒 Files selected for processing (2)
docs/design/frontend-state-flow.mdweb-ui/src/components/JobList.tsx
Comment on lines
+102
to
+115
| ``` | ||
| viewMode === "jobs" → JobList renders | ||
| └─ useEffect([]) → apiClient.listJobs() | ||
| └─ GET /v1/jobs?limit=10&skip=0 | ||
| ├─ SUCCESS → setJobs(data), setLoading(false) | ||
| └─ FAIL → setError(msg), setJobs([]) | ||
| ``` | ||
|
|
||
| **Dependencies:** | ||
| - Requires server running | ||
| - Does NOT require plugin to be loaded | ||
|
|
||
| **Polling Required:** NO - one-time fetch on mount | ||
|
|
Contributor
There was a problem hiding this comment.
JobList fetch flow is internally inconsistent in the document.
The flow still documents mount-only useEffect([]) and marks Issue #365 as open, but the action list marks the fix as done. Please align all three sections to the current viewMode-triggered fetch behaviour.
Also applies to: 387-407, 482-483
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 102-102: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/design/frontend-state-flow.md` around lines 102 - 115, The JobList flow
in the diagram is wrong: it should document a viewMode-triggered fetch
(useEffect([viewMode])) instead of a mount-only useEffect([]); update the
sequence for JobList to show useEffect([viewMode]) → apiClient.listJobs() with
the same GET/Success/Fail branches, change the "Polling Required" and
"Dependencies" notes to reflect that fetch is triggered by viewMode (not
strictly mount-only), and mark Issue `#365` as resolved/fixed in the diagram text;
apply the same corrections to the other occurrences referencing the JobList flow
(the other two sections mentioned) so all three places consistently reference
viewMode-triggered fetching and Issue `#365` state.
This was referenced Mar 24, 2026
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.
Problem
After uploading a video and running a job, pressing "Run Job" again tried to re-upload the same video file instead of re-using the already uploaded video path. This caused:
Solution
Store the uploaded
video_pathin local state and re-use it for consecutive job runs. Only upload when user selects a new file.Changes
VideoUpload.tsx:
videoPathstate to store uploaded video pathhandleRunJobandhandleStartStreamingto check videoPath firstVideoUpload.test.tsx (NEW):
State Flow
TEST-CHANGE JUSTIFICATION
Tests added to verify the new videoPath re-use behavior:
Fixes #366
Summary by CodeRabbit
Documentation
Bug Fixes
Tests