Skip to content
Merged
484 changes: 484 additions & 0 deletions docs/design/frontend-state-flow.md

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions web-ui/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ function App() {

{viewMode === "jobs" && (
<div style={styles.panel}>
<JobList onJobSelect={setSelectedJob} />
<JobList onJobSelect={setSelectedJob} viewMode={viewMode} />
</div>
)}
</aside>
Expand Down Expand Up @@ -681,7 +681,7 @@ function App() {
/>
{isUploading && <p>Analyzing...</p>}
{uploadResult?.job_id && (
<JobStatus jobId={uploadResult.job_id} />
<JobStatus jobId={uploadResult.job_id} initialStatus={uploadResult.status} />
)}
</div>
)}
Expand Down Expand Up @@ -716,7 +716,7 @@ function App() {
<h3>Job Details</h3>
{selectedJob ? (
<>
<JobStatus jobId={selectedJob.job_id} />
<JobStatus jobId={selectedJob.job_id} initialStatus={selectedJob.status} />
</>
) : (
<p>Select a job</p>
Expand All @@ -737,7 +737,7 @@ function App() {
{uploadResult?.job_id && lockedTools && (
<div style={{ marginTop: "20px", borderTop: "1px solid var(--border-light)", paddingTop: "20px" }}>
<h3>Job Processing</h3>
<JobStatus jobId={uploadResult.job_id} />
<JobStatus jobId={uploadResult.job_id} initialStatus={uploadResult.status} />
</div>
)}
</div>
Expand Down
72 changes: 72 additions & 0 deletions web-ui/src/components/JobList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -332,4 +332,76 @@ describe("JobList", () => {
});
});
});

// Issue #365: viewMode prop triggers re-fetch when switching to jobs view
describe("Issue #365: viewMode prop and re-fetch on view change", () => {
it("should re-fetch jobs when viewMode changes to 'jobs'", async () => {
const mockListJobs = client.apiClient.listJobs as ReturnType<typeof vi.fn>;

// First call fails (simulating network error when Jobs clicked after video upload)
mockListJobs.mockRejectedValueOnce(new Error("failed to fetch"));
// Second call succeeds (simulating re-fetch after viewMode change)
const mockJob = createMockJobDone();
mockListJobs.mockResolvedValueOnce([mockJob]);

// Render with viewMode='jobs' initially
const { rerender } = render(
<JobList onJobSelect={vi.fn()} viewMode="jobs" />
);

// Should show error from first fetch
await waitFor(() => {
expect(screen.getByText(/failed to fetch/i)).toBeInTheDocument();
});

// First call happened
expect(mockListJobs).toHaveBeenCalledTimes(1);

// Simulate user clicking away and back to jobs view
// (viewMode changes from 'jobs' to something else and back)
rerender(<JobList onJobSelect={vi.fn()} viewMode="upload" />);
rerender(<JobList onJobSelect={vi.fn()} viewMode="jobs" />);

// Should re-fetch and show job
await waitFor(() => {
expect(screen.getByText("completed")).toBeInTheDocument();
});

// Should have been called again (re-fetch)
expect(mockListJobs).toHaveBeenCalledTimes(2);
});

it("should NOT fetch when viewMode is not 'jobs'", () => {
const mockListJobs = client.apiClient.listJobs as ReturnType<typeof vi.fn>;
mockListJobs.mockResolvedValue([]);

render(<JobList onJobSelect={vi.fn()} viewMode="upload" />);

// Should not fetch when viewMode is not 'jobs'
expect(mockListJobs).not.toHaveBeenCalled();
});

it("should fetch on initial render when viewMode is 'jobs'", async () => {
const mockListJobs = client.apiClient.listJobs as ReturnType<typeof vi.fn>;
mockListJobs.mockResolvedValue([]);

render(<JobList onJobSelect={vi.fn()} viewMode="jobs" />);

await waitFor(() => {
expect(mockListJobs).toHaveBeenCalledTimes(1);
});
});

it("should work without viewMode prop (backward compatibility)", async () => {
// Without viewMode prop, should behave as before (fetch on mount)
const mockListJobs = client.apiClient.listJobs as ReturnType<typeof vi.fn>;
mockListJobs.mockResolvedValue([]);

render(<JobList onJobSelect={vi.fn()} />);

await waitFor(() => {
expect(mockListJobs).toHaveBeenCalledTimes(1);
});
});
});
});
8 changes: 6 additions & 2 deletions web-ui/src/components/JobList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,18 @@ import { apiClient, Job } from "../api/client";

export interface JobListProps {
onJobSelect: (job: Job) => void;
viewMode?: string; // Issue #365: Trigger re-fetch when switching to jobs view
}

export function JobList({ onJobSelect }: JobListProps) {
export function JobList({ onJobSelect, viewMode }: JobListProps) {
const [jobs, setJobs] = useState<Job[]>([]);
const [loading, setLoading] = useState(true);
const [error, setError] = useState<string | null>(null);

useEffect(() => {
// Issue #365: Only fetch when viewMode is 'jobs' (or not provided for backward compat)
if (viewMode !== undefined && viewMode !== "jobs") return;

const loadJobs = async () => {
try {
const data = await apiClient.listJobs();
Expand All @@ -31,7 +35,7 @@ export function JobList({ onJobSelect }: JobListProps) {
};

loadJobs();
}, []);
}, [viewMode]); // Re-fetch when viewMode changes

const getStatusColor = (status: string) => {
switch (status) {
Expand Down
100 changes: 100 additions & 0 deletions web-ui/src/components/JobStatus.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -469,4 +469,104 @@ describe("JobStatus", () => {
expect(screen.getByText(/completed/i)).toBeInTheDocument();
});
});

// Issue #363: initialStatus prop prevents polling for completed jobs
describe("Issue #363: initialStatus prop", () => {
it("should NOT poll when initialStatus is 'completed'", async () => {
// The bug: JobStatus always initializes pollStatus to "pending"
// even when the job is already completed.
// Fix: Accept initialStatus prop and use it to initialize state.

mockUseJobProgress.mockReturnValue({
progress: null,
status: "pending",
error: null,
isConnected: false, // WebSocket disconnected
});

// This should NEVER be called if initialStatus="completed"
mockGetJob.mockResolvedValue({
status: "completed",
progress: 100,
result_url: "/v1/jobs/job-123/result",
});

// Pass initialStatus="completed" to indicate job is already done
render(<JobStatus jobId="job-123" initialStatus="completed" />);

// Should show completed immediately from initialStatus
expect(screen.getByText(/completed/i)).toBeInTheDocument();

// Wait a bit to ensure polling doesn't start
await new Promise((resolve) => setTimeout(resolve, 100));

// CRITICAL: getJob should NOT be called because initialStatus is terminal
expect(mockGetJob).not.toHaveBeenCalled();
});

it("should NOT poll when initialStatus is 'failed'", async () => {
mockUseJobProgress.mockReturnValue({
progress: null,
status: "pending",
error: null,
isConnected: false,
});

mockGetJob.mockResolvedValue({
status: "failed",
error_message: "Job failed",
});

render(<JobStatus jobId="job-123" initialStatus="failed" />);

expect(screen.getByText(/failed/i)).toBeInTheDocument();

await new Promise((resolve) => setTimeout(resolve, 100));

expect(mockGetJob).not.toHaveBeenCalled();
});

it("should poll when initialStatus is 'pending' (default behavior)", async () => {
mockUseJobProgress.mockReturnValue({
progress: null,
status: "pending",
error: null,
isConnected: false,
});

mockGetJob.mockResolvedValue({
status: "running",
progress: 50,
});

render(<JobStatus jobId="job-123" initialStatus="pending" />);

// Should start polling for pending jobs
await waitFor(() => {
expect(mockGetJob).toHaveBeenCalledWith("job-123");
});
});

it("should work without initialStatus prop (backward compatibility)", async () => {
// Without initialStatus, should behave as before (poll starting from "pending")
mockUseJobProgress.mockReturnValue({
progress: null,
status: "pending",
error: null,
isConnected: false,
});

mockGetJob.mockResolvedValue({
status: "completed",
progress: 100,
});

render(<JobStatus jobId="job-123" />);

// Should start polling (backward compatible)
await waitFor(() => {
expect(mockGetJob).toHaveBeenCalledWith("job-123");
});
});
});
});
6 changes: 4 additions & 2 deletions web-ui/src/components/JobStatus.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ import { useJobProgress } from "../hooks/useJobProgress";

type Props = {
jobId: string;
initialStatus?: Status; // Issue #363: Pass known status to prevent unnecessary polling
};

type Status = "pending" | "running" | "completed" | "failed";

export const JobStatus: React.FC<Props> = ({ jobId }) => {
export const JobStatus: React.FC<Props> = ({ jobId, initialStatus }) => {
// WebSocket progress (primary source)
const {
progress: wsProgress,
Expand All @@ -20,7 +21,8 @@ export const JobStatus: React.FC<Props> = ({ jobId }) => {

// HTTP polling fallback
const [pollProgress, setPollProgress] = useState<number | null>(null);
const [pollStatus, setPollStatus] = useState<Status>("pending");
// Issue #363: Initialize from initialStatus prop to prevent polling for completed jobs
const [pollStatus, setPollStatus] = useState<Status>(initialStatus || "pending");
const [pollError, setPollError] = useState<string | null>(null);

// Determine which source to use
Expand Down
Loading
Loading