Skip to content

feat(web-ui): pipeline progress indicator (Think→Build→Prove→Ship)#488

Merged
frankbria merged 3 commits intomainfrom
feature/issue-466-pipeline-progress-indicator
Mar 23, 2026
Merged

feat(web-ui): pipeline progress indicator (Think→Build→Prove→Ship)#488
frankbria merged 3 commits intomainfrom
feature/issue-466-pipeline-progress-indicator

Conversation

@frankbria
Copy link
Owner

@frankbria frankbria commented Mar 23, 2026

Summary

  • Adds a persistent PipelineProgressBar to AppLayout showing the four CodeFRAME phases with live completion status derived from existing API endpoints — no new backend work needed
  • Creates usePipelineStatus hook (SWR + workspace-detection pattern) to aggregate completion from prd/tasks/proof/review APIs
  • Bar is hidden on / (workspace selector), shown on all other pages with the current phase highlighted

What was built

File Purpose
web-ui/src/hooks/usePipelineStatus.ts SWR hook — 4 parallel fetches, derives phase completion
web-ui/src/components/layout/PipelineProgressBar.tsx Horizontal stepper component, responsive
web-ui/src/components/layout/AppLayout.tsx Injects bar above {children}
web-ui/__mocks__/@hugeicons/react.js Added Tick01Icon mock

Phase completion logic

Phase Complete when
Think prdApi.getLatest() returns data
Build by_status.DONE + MERGED > 0
Prove proof.total > 0 && proof.open === 0
Ship reviewApi.getDiff().files_changed === 0

Test plan

  • 24 unit tests added (12 hook, 12 component) — all passing
  • 371 total tests pass, zero regressions
  • ESLint: no warnings
  • TypeScript: no errors in changed files
  • Next.js production build: passes

Acceptance criteria

  • Progress bar/stepper visible on all main pages
  • Step completion status indicated from live API data
  • Current step highlighted (bg-accent)
  • Clicking steps navigates to correct page (Next.js Link)
  • Responsive: abbreviated labels on mobile, full labels on lg:

Closes #466

Summary by CodeRabbit

  • New Features

    • Pipeline Progress Bar showing Think, Build, Prove, Ship with navigation, active highlighting, checkmarks for completed stages; responsive labels for mobile/desktop; hidden on root/non-pipeline pages
    • Workspace-aware pipeline status tracking to power progress state across pages
  • Tests

    • Added comprehensive tests for the progress bar and pipeline status logic, including a mocked tick icon for completion indicators

Adds a persistent PipelineProgressBar to AppLayout showing the four
CodeFRAME phases with live completion status from existing API endpoints.

- usePipelineStatus hook: aggregates prd/tasks/proof/review APIs via SWR,
  derives completion per phase (think=PRD exists, build=DONE+MERGED>0,
  prove=no open requirements, ship=no uncommitted diff)
- PipelineProgressBar: horizontal stepper with checkmark (Tick01Icon) for
  complete phases, numbered circle for incomplete, current phase highlighted
  with bg-accent, responsive (abbreviated labels on mobile, full on lg:)
- AppLayout: injects PipelineProgressBar above children in flex-col column
- Bar hidden on root path (/), workspace-aware via localStorage events

Tests: 24 new unit tests (hook + component), 371 total passing
Closes #466
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 23, 2026

Walkthrough

Adds a pipeline progress feature: a new client hook to compute per-phase status, a PipelineProgressBar UI component and tests, an icon mock for Tick01Icon, and integrates the bar into AppLayout.

Changes

Cohort / File(s) Summary
Icon mock
web-ui/__mocks__/@hugeicons/react.js
Exported Tick01Icon mock for tests.
Hook & types + tests
web-ui/src/hooks/usePipelineStatus.ts, web-ui/src/types/index.ts, web-ui/src/__tests__/hooks/usePipelineStatus.test.ts
New usePipelineStatus hook reading workspace selection, listening to storage/events, conditionally fetching four SWR endpoints, and deriving per-phase isComplete/isLoading/isError. Added PhaseStatus and PipelineStatus types and comprehensive unit tests.
Progress UI & tests
web-ui/src/components/layout/PipelineProgressBar.tsx, web-ui/src/__tests__/components/layout/PipelineProgressBar.test.tsx
New client-only PipelineProgressBar rendering four phase steps, route-based active highlighting, completion ticks, responsive labels, aria-current usage, and tests verifying rendering, routing, ticks, and hrefs.
Layout integration
web-ui/src/components/layout/AppLayout.tsx
Inserted PipelineProgressBar into AppLayout and changed main content container to a column flex layout to accommodate the bar.

Sequence Diagram(s)

sequenceDiagram
  participant Page as Page (Next.js)
  participant AppLayout as AppLayout
  participant ProgressBar as PipelineProgressBar
  participant Hook as usePipelineStatus
  participant SWR as SWR/API

  Page->>AppLayout: render
  AppLayout->>ProgressBar: include component
  ProgressBar->>Hook: call usePipelineStatus()
  Hook->>Hook: read workspacePath (storage/event)
  Hook->>SWR: conditionally fetch PRD, tasks, proof, review
  SWR-->>Hook: return data / loading / error
  Hook-->>ProgressBar: computed PipelineStatus
  ProgressBar->>ProgressBar: determine active phase from pathname
  ProgressBar->>Page: render step links, ticks, aria-current
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 I hop across phases, one small bound at a time,

Think, Build, Prove, Ship — each step marked with a chime.
A tick for the done, a link for the way,
I nibble at bugs and bounce through the day.
Hooray for the bar — off we go, hip-hip-hooray!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(web-ui): pipeline progress indicator (Think→Build→Prove→Ship)' accurately summarizes the main change: introducing a pipeline progress indicator component showing the four pipeline phases.
Linked Issues check ✅ Passed The PR fully implements all coding requirements from issue #466: persistent progress bar on all pages, phase completion status via live data, current step highlighting, clickable navigation links, responsive design, and integration via AppLayout.
Out of Scope Changes check ✅ Passed All changes are directly aligned with the PR objectives and issue #466 requirements. No out-of-scope modifications detected; additions focus solely on the pipeline progress indicator feature.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📋 Issue Planner

Built with CodeRabbit's Coding Plans for faster development and fewer bugs.

View plan used: #466

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/issue-466-pipeline-progress-indicator

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (6)
web-ui/src/__tests__/hooks/usePipelineStatus.test.ts (2)

1-5: Remove unused import.

waitFor is imported but never used in any test case.

♻️ Suggested fix
-import { renderHook, waitFor } from '@testing-library/react';
+import { renderHook } from '@testing-library/react';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-ui/src/__tests__/hooks/usePipelineStatus.test.ts` around lines 1 - 5,
Remove the unused imported symbol waitFor from the test file import list: locate
the import statement at the top of
web-ui/src/__tests__/hooks/usePipelineStatus.test.ts (currently importing
renderHook, waitFor from '@testing-library/react') and delete only the waitFor
token so the import reads import { renderHook } from '@testing-library/react';
leaving other imports (renderHook, useSWR, usePipelineStatus,
getSelectedWorkspacePath) unchanged.

203-214: Test only verifies two phases' loading states.

The test "returns isLoading true while any phase is loading" only asserts think.isLoading and build.isLoading. Consider verifying all four phases for completeness, or add separate tests for each phase's loading state.

♻️ Suggested enhancement
   it('returns isLoading true while any phase is loading', () => {
     mockSWRCalls({
       prd: { isLoading: true },
       tasks: { isLoading: false },
-      proof: { isLoading: false },
-      review: { isLoading: false },
+      proof: { isLoading: true },
+      review: { isLoading: false },
     });

     const { result } = renderHook(() => usePipelineStatus());
     expect(result.current.think.isLoading).toBe(true);
     expect(result.current.build.isLoading).toBe(false);
+    expect(result.current.prove.isLoading).toBe(true);
+    expect(result.current.ship.isLoading).toBe(false);
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-ui/src/__tests__/hooks/usePipelineStatus.test.ts` around lines 203 - 214,
The test only asserts two phases; update the "returns isLoading true while any
phase is loading" spec so it verifies all four phase loading flags returned by
usePipelineStatus: after calling mockSWRCalls({ prd: { isLoading: true }, tasks:
{ isLoading: false }, proof: { isLoading: false }, review: { isLoading: false
}}) assert result.current.think.isLoading, result.current.build.isLoading,
result.current.proof.isLoading, and result.current.review.isLoading match the
mocked values (true for think/prd, false for the others) so the test fully
covers all phase mappings in usePipelineStatus.
web-ui/src/__tests__/components/layout/PipelineProgressBar.test.tsx (2)

1-6: React import may be unnecessary.

With the modern JSX transform (React 17+), explicit import React from 'react' is no longer required in JSX files.

♻️ Suggested fix
-import React from 'react';
 import { render, screen } from '@testing-library/react';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-ui/src/__tests__/components/layout/PipelineProgressBar.test.tsx` around
lines 1 - 6, Remove the unnecessary top-level React import in the test file:
delete the line "import React from 'react';" from PipelineProgressBar.test.tsx
since the modern JSX transform makes it redundant; keep the other imports
(render, screen, usePathname, PipelineProgressBar, usePipelineStatus) intact and
run the test suite to verify nothing else relies on the explicit React symbol.

55-64: Consider adding aria-current assertion for active phase.

The test "highlights the Think phase when on /prd" verifies the href but doesn't assert that aria-current="step" is set on the active link. This would strengthen accessibility testing.

♻️ Suggested enhancement
   it('highlights the Think phase when on /prd', () => {
     mockUsePathname.mockReturnValue('/prd');
     mockUsePipelineStatus.mockReturnValue(allIncomplete);

     render(<PipelineProgressBar />);

     // Think link should have active styling indicator
     const thinkLink = screen.getByRole('link', { name: /think/i });
     expect(thinkLink).toHaveAttribute('href', '/prd');
+    expect(thinkLink).toHaveAttribute('aria-current', 'step');
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-ui/src/__tests__/components/layout/PipelineProgressBar.test.tsx` around
lines 55 - 64, Update the test "highlights the Think phase when on /prd" in
PipelineProgressBar.test.tsx to also assert accessibility state by checking that
the active link (thinkLink retrieved via screen.getByRole('link', { name:
/think/i })) has aria-current="step"; keep the existing href assertion but add
an expectation like expect(thinkLink).toHaveAttribute('aria-current', 'step') to
validate the active phase is exposed to assistive tech.
web-ui/src/hooks/usePipelineStatus.ts (2)

8-18: Consider moving exported interfaces to the shared types file.

Per coding guidelines, TypeScript types should be defined in web-ui/src/types/index.ts. The PhaseStatus and PipelineStatus interfaces are exported and could be consumed elsewhere (e.g., in components or tests).

♻️ Suggested refactor

Move to web-ui/src/types/index.ts:

export interface PhaseStatus {
  isComplete: boolean;
  isLoading: boolean;
}

export interface PipelineStatus {
  think: PhaseStatus;
  build: PhaseStatus;
  prove: PhaseStatus;
  ship: PhaseStatus;
}

Then import in the hook:

-export interface PhaseStatus {
-  isComplete: boolean;
-  isLoading: boolean;
-}
-
-export interface PipelineStatus {
-  think: PhaseStatus;
-  build: PhaseStatus;
-  prove: PhaseStatus;
-  ship: PhaseStatus;
-}
+import type { PipelineStatus } from '@/types';

As per coding guidelines: "TypeScript types must be defined in web-ui/src/types/index.ts".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-ui/src/hooks/usePipelineStatus.ts` around lines 8 - 18, Move the exported
interfaces PhaseStatus and PipelineStatus out of the hook and into the central
shared types module (the project's types index), then update the hook to import
these types instead of declaring them inline; specifically remove the local
declarations of PhaseStatus and PipelineStatus from usePipelineStatus, add
equivalent exports to the shared types index, and replace local references in
functions/types inside usePipelineStatus with imported PhaseStatus and
PipelineStatus to keep types centralized and consumable by other modules.

39-61: Consider surfacing error states to the consumer.

The hook currently only exposes isComplete and isLoading per phase. If any API call fails, the phase will appear incomplete without indicating an error occurred. This could confuse users when the pipeline status appears stuck.

♻️ Suggested enhancement
 export interface PhaseStatus {
   isComplete: boolean;
   isLoading: boolean;
+  isError: boolean;
 }

 // In each SWR call, destructure error:
-const { data: prdData, isLoading: prdLoading } = useSWR(...)
+const { data: prdData, isLoading: prdLoading, error: prdError } = useSWR(...)

 // Then in return:
 think: {
   isComplete: !!prdData,
   isLoading: prdLoading,
+  isError: !!prdError,
 },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-ui/src/hooks/usePipelineStatus.ts` around lines 39 - 61, The hook hides
API errors by only exposing isComplete/isLoading for each phase (prd, tasks,
proof, review); update each useSWR invocation (the ones returning
prdData/prdLoading, tasksData/tasksLoading, proofData/proofLoading,
reviewData/reviewLoading) to also capture the error (e.g., prdError, tasksError,
proofError, reviewError) and expose per-phase error flags/objects (e.g.,
isPrdError, prdError) in the hook's return shape so consumers can detect and
render error states instead of treating failures as simply incomplete.
🤖 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/components/layout/PipelineProgressBar.tsx`:
- Around line 36-39: The comment claims the bar should be hidden for root and
any path with no matching phase, but the code only checks pathname === '/' and
then computes activePhase via getActivePhase(pathname); update the
implementation to match the comment by adding a check after computing
activePhase (e.g., if getActivePhase(pathname) returns falsy) and return null
when there is no active phase, referencing getActivePhase and pathname in the
change; alternatively, if the current behavior is intended, update the comment
above the pathname check to remove the "no matching phase" clause to reflect
that non-pipeline pages still render the bar.

---

Nitpick comments:
In `@web-ui/src/__tests__/components/layout/PipelineProgressBar.test.tsx`:
- Around line 1-6: Remove the unnecessary top-level React import in the test
file: delete the line "import React from 'react';" from
PipelineProgressBar.test.tsx since the modern JSX transform makes it redundant;
keep the other imports (render, screen, usePathname, PipelineProgressBar,
usePipelineStatus) intact and run the test suite to verify nothing else relies
on the explicit React symbol.
- Around line 55-64: Update the test "highlights the Think phase when on /prd"
in PipelineProgressBar.test.tsx to also assert accessibility state by checking
that the active link (thinkLink retrieved via screen.getByRole('link', { name:
/think/i })) has aria-current="step"; keep the existing href assertion but add
an expectation like expect(thinkLink).toHaveAttribute('aria-current', 'step') to
validate the active phase is exposed to assistive tech.

In `@web-ui/src/__tests__/hooks/usePipelineStatus.test.ts`:
- Around line 1-5: Remove the unused imported symbol waitFor from the test file
import list: locate the import statement at the top of
web-ui/src/__tests__/hooks/usePipelineStatus.test.ts (currently importing
renderHook, waitFor from '@testing-library/react') and delete only the waitFor
token so the import reads import { renderHook } from '@testing-library/react';
leaving other imports (renderHook, useSWR, usePipelineStatus,
getSelectedWorkspacePath) unchanged.
- Around line 203-214: The test only asserts two phases; update the "returns
isLoading true while any phase is loading" spec so it verifies all four phase
loading flags returned by usePipelineStatus: after calling mockSWRCalls({ prd: {
isLoading: true }, tasks: { isLoading: false }, proof: { isLoading: false },
review: { isLoading: false }}) assert result.current.think.isLoading,
result.current.build.isLoading, result.current.proof.isLoading, and
result.current.review.isLoading match the mocked values (true for think/prd,
false for the others) so the test fully covers all phase mappings in
usePipelineStatus.

In `@web-ui/src/hooks/usePipelineStatus.ts`:
- Around line 8-18: Move the exported interfaces PhaseStatus and PipelineStatus
out of the hook and into the central shared types module (the project's types
index), then update the hook to import these types instead of declaring them
inline; specifically remove the local declarations of PhaseStatus and
PipelineStatus from usePipelineStatus, add equivalent exports to the shared
types index, and replace local references in functions/types inside
usePipelineStatus with imported PhaseStatus and PipelineStatus to keep types
centralized and consumable by other modules.
- Around line 39-61: The hook hides API errors by only exposing
isComplete/isLoading for each phase (prd, tasks, proof, review); update each
useSWR invocation (the ones returning prdData/prdLoading,
tasksData/tasksLoading, proofData/proofLoading, reviewData/reviewLoading) to
also capture the error (e.g., prdError, tasksError, proofError, reviewError) and
expose per-phase error flags/objects (e.g., isPrdError, prdError) in the hook's
return shape so consumers can detect and render error states instead of treating
failures as simply incomplete.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6f815411-1c02-4941-86ef-cf4855faa9ab

📥 Commits

Reviewing files that changed from the base of the PR and between a22d167 and 37fa851.

📒 Files selected for processing (6)
  • web-ui/__mocks__/@hugeicons/react.js
  • web-ui/src/__tests__/components/layout/PipelineProgressBar.test.tsx
  • web-ui/src/__tests__/hooks/usePipelineStatus.test.ts
  • web-ui/src/components/layout/AppLayout.tsx
  • web-ui/src/components/layout/PipelineProgressBar.tsx
  • web-ui/src/hooks/usePipelineStatus.ts

@claude
Copy link

claude bot commented Mar 23, 2026

Review: feat(web-ui): pipeline progress indicator

Clean implementation that reinforces the Think→Build→Prove→Ship mental model without requiring any new backend work. Good accessibility attributes, responsive layout, and solid test coverage. A few items worth addressing:


Issues

1. useState hydration lag in usePipelineStatus.ts (minor bug)

// Current — always null on first render, delaying all SWR fetches
const [workspacePath, setWorkspacePath] = useState<string | null>(null);

// Fix — lazy initializer avoids the null-on-first-render flash
const [workspacePath, setWorkspacePath] = useState<string | null>(() => getSelectedWorkspacePath());

Without this, the four SWR calls start with null keys on every mount, so the progress bar always shows all-incomplete for one render cycle before the effect fires. With the lazy initializer, the correct path is read synchronously and SWR starts fetching immediately.

2. Ship phase false-positive on a fresh project

isComplete: !!reviewData && reviewData.files_changed === 0,

A brand-new workspace with zero committed changes will mark Ship as complete before the user has done anything. Consider requiring at least one DONE/MERGED task as a prerequisite guard, or document this as intentional "clean state" behavior.


Nits

Unused imports in tests

usePipelineStatus.test.ts imports waitFor but never calls it:

import { renderHook, waitFor } from '@testing-library/react';
//                   ^^^^^^^^ unused

PipelineProgressBar.test.tsx mocks useRouter but the component uses Link only — the mock is unused.

Neither affects correctness, but they are lint-bait.

Comment vs. code mismatch

The inline comment says // Hide on root (workspace selector) and any path with no matching phase, but the guard only returns null for /. An unrecognized path like /settings still renders the bar with no phase highlighted — reasonable behavior, but the comment overpromises. Either update the comment or add the guard.


Looks good

  • WCAG touch targets (min-h-[44px] min-w-[44px]) ✅
  • aria-label, aria-current="step", aria-hidden on the separator ✅
  • Null SWR keys when no workspace selected (no spurious fetches) ✅
  • Event listener cleanup in useEffect
  • 24 tests covering all edge cases for each phase ✅

The lazy initializer fix (issue 1) is the most impactful change — everything else is polish.

- Fix comment/behavior mismatch: bar now returns null for non-pipeline
  paths (e.g. /settings) matching the stated intent (Option B)
- Move PhaseStatus/PipelineStatus types to src/types/index.ts per guidelines
- Add isError field to PhaseStatus to surface API call failures
- Remove unused waitFor import from hook test
- Expand isLoading test to assert all 4 phases independently
- Remove unnecessary React import from component test (modern JSX transform)
- Add aria-current="step" assertion to all active-phase tests
- Add test for null return on non-pipeline paths
@claude
Copy link

claude bot commented Mar 23, 2026


Review: Pipeline Progress Indicator

Good implementation overall — the hook design is clean, test coverage is solid, and the component correctly handles the active/complete/upcoming visual states. The latest revision already addresses CodeRabbit's earlier suggestions (types in index.ts, isError fields exposed, double null guard in component). A few things worth flagging:


Ship completion semantics

The most significant issue: ship.isComplete is true when reviewData.files_changed === 0. This means a freshly cloned repo with no changes shows Ship as complete on first visit. The intent of the Ship phase in CodeFrame's pipeline is that a PR was created and merged — not just that the working tree is clean.

Consider either:

  • Checking for a merged PR count instead of files_changed
  • Requiring files_changed === 0 AND at least one MERGED task (by_status.MERGED > 0), which is already available from the tasks fetch

No auto-refresh

All four SWR calls use revalidateOnFocus: false with no refreshInterval. If you complete a task and then navigate to the Prove tab, the Build indicator stays stale until you navigate away and back. Since this bar is meant to show live progress, a short refreshInterval (e.g., 30s) would make it useful:

{ revalidateOnFocus: false, refreshInterval: 30_000 }

Hydration flash (minor, known tradeoff)

The useState(null) + useEffect pattern for workspacePath means on initial render all SWR keys are null, so all phases appear incomplete (not loading — just silently incomplete). This is an inherent tradeoff with localStorage-based state in Next.js. A refreshInterval on SWR would mask this since data arrives quickly after mount.


Nitpick: gap-0 is a no-op

className="... gap-0 ..." on the nav has no effect since the separators use their own mx-1 spacing. Can be removed for clarity.


The CodeRabbit items still worth actioning: removing the unused waitFor import in the test file, and adding full 4-phase coverage to the loading state test. Both are quick fixes.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
web-ui/src/hooks/usePipelineStatus.ts (1)

28-49: Refactor SWR calls to use tuple keys with destructuring for type safety and idiomatic v2 pattern.

SWR v2 recommends tuple/array keys for dynamic parameters instead of string concatenation with closed-over variables. This improves type safety by eliminating non-null assertions and ensures the cache key fully represents request inputs.

Refactor pattern (apply to all four SWR calls)
-  const { data: prdData, isLoading: prdLoading, error: prdError } = useSWR(
-    workspacePath ? `/pipeline/prd?path=${workspacePath}` : null,
-    () => prdApi.getLatest(workspacePath!),
-    { revalidateOnFocus: false }
-  );
+  const { data: prdData, isLoading: prdLoading, error: prdError } = useSWR(
+    workspacePath ? ['pipeline/prd', workspacePath] as const : null,
+    ([, path]) => prdApi.getLatest(path),
+    { revalidateOnFocus: false }
+  );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-ui/src/hooks/usePipelineStatus.ts` around lines 28 - 49, Replace the
string-based SWR keys with tuple/array keys and remove the non-null assertions
by passing workspacePath as a tuple element so the key fully represents inputs;
update all four useSWR calls (the ones producing prdData/prdLoading/prdError,
tasksData/tasksLoading/tasksError, proofData/proofLoading/proofError,
reviewData/reviewLoading/reviewError) to use keys like ['pipeline/prd',
workspacePath] and change the fetcher to accept the key tuple and call the
corresponding API functions (prdApi.getLatest, tasksApi.getAll,
proofApi.getStatus, reviewApi.getDiff) using the workspacePath parameter instead
of closing over it. Ensure revalidateOnFocus remains set and that the fetcher
handles a possibly undefined workspacePath by returning null or not making the
request when workspacePath is falsy.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@web-ui/src/hooks/usePipelineStatus.ts`:
- Around line 28-49: Replace the string-based SWR keys with tuple/array keys and
remove the non-null assertions by passing workspacePath as a tuple element so
the key fully represents inputs; update all four useSWR calls (the ones
producing prdData/prdLoading/prdError, tasksData/tasksLoading/tasksError,
proofData/proofLoading/proofError, reviewData/reviewLoading/reviewError) to use
keys like ['pipeline/prd', workspacePath] and change the fetcher to accept the
key tuple and call the corresponding API functions (prdApi.getLatest,
tasksApi.getAll, proofApi.getStatus, reviewApi.getDiff) using the workspacePath
parameter instead of closing over it. Ensure revalidateOnFocus remains set and
that the fetcher handles a possibly undefined workspacePath by returning null or
not making the request when workspacePath is falsy.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cfe07f70-1748-49eb-9dc7-2345bf75bbd8

📥 Commits

Reviewing files that changed from the base of the PR and between 37fa851 and 9f50ccb.

📒 Files selected for processing (5)
  • web-ui/src/__tests__/components/layout/PipelineProgressBar.test.tsx
  • web-ui/src/__tests__/hooks/usePipelineStatus.test.ts
  • web-ui/src/components/layout/PipelineProgressBar.tsx
  • web-ui/src/hooks/usePipelineStatus.ts
  • web-ui/src/types/index.ts
✅ Files skipped from review due to trivial changes (2)
  • web-ui/src/types/index.ts
  • web-ui/src/tests/components/layout/PipelineProgressBar.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • web-ui/src/tests/hooks/usePipelineStatus.test.ts
  • web-ui/src/components/layout/PipelineProgressBar.tsx

Replace string-based SWR cache keys with tuple/array keys per SWR v2
pattern. Each key tuple ['pipeline/prd', workspacePath] fully encodes
the request inputs without closing over workspacePath with ! assertions.
@claude
Copy link

claude bot commented Mar 23, 2026

Follow-up Review

Good iteration — the latest push addresses nearly everything from both my previous reviews and CodeRabbit:

Addressed ✅

  • isError surfaced per phase
  • Types in types/index.ts
  • Double null guard in component (pathname === '/' then !activePhase)
  • Tuple SWR keys (no more ! assertions)
  • Full 4-phase coverage in the loading-state test
  • Unused waitFor import removed

Still open

Ship phase false-positive (raised in both previous reviews — still not addressed)

isComplete: !!reviewData && reviewData.files_changed === 0,

A freshly cloned repo with a clean working tree shows Ship as complete before anything has been built or proved. The by_status.MERGED count from the tasks fetch is already in scope — adding it as a guard costs one line:

isComplete: !!reviewData &&
  reviewData.files_changed === 0 &&
  (tasksData?.by_status?.MERGED ?? 0) > 0,

No auto-refresh (raised in previous review — still not addressed)

All four SWR calls use revalidateOnFocus: false with no refreshInterval. Complete a task in another tab and the Build step stays stale until a full page reload. A 30s interval would make the bar genuinely live:

{ revalidateOnFocus: false, refreshInterval: 30_000 }

Nit: gap-0 is a no-op (raised in previous review — unchanged)

The nav still has gap-0 in its className. Spacing is handled by the separator's mx-1, so gap-0 does nothing. Minor, but worth cleaning up.


The Ship semantics fix is the only blocker I'd hold for before merge. Auto-refresh is a meaningful UX improvement worth addressing in this PR since the bar is meant to show live status. The gap-0 nit can go in any cleanup pass.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
web-ui/src/hooks/usePipelineStatus.ts (1)

29-51: Optional: Extract shared SWR options to reduce repetition.

The same options object is repeated four times; centralizing it lowers drift risk if behavior changes later.

♻️ Small refactor
+  const swrOptions = { revalidateOnFocus: false } as const;
+
   const { data: prdData, isLoading: prdLoading, error: prdError } = useSWR(
     workspacePath ? (['pipeline/prd', workspacePath] as const) : null,
     ([, path]) => prdApi.getLatest(path),
-    { revalidateOnFocus: false }
+    swrOptions
   );
@@
   const { data: tasksData, isLoading: tasksLoading, error: tasksError } = useSWR(
     workspacePath ? (['pipeline/tasks', workspacePath] as const) : null,
     ([, path]) => tasksApi.getAll(path),
-    { revalidateOnFocus: false }
+    swrOptions
   );
@@
   const { data: proofData, isLoading: proofLoading, error: proofError } = useSWR(
     workspacePath ? (['pipeline/proof', workspacePath] as const) : null,
     ([, path]) => proofApi.getStatus(path),
-    { revalidateOnFocus: false }
+    swrOptions
   );
@@
   const { data: reviewData, isLoading: reviewLoading, error: reviewError } = useSWR(
     workspacePath ? (['pipeline/review', workspacePath] as const) : null,
     ([, path]) => reviewApi.getDiff(path),
-    { revalidateOnFocus: false }
+    swrOptions
   );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-ui/src/hooks/usePipelineStatus.ts` around lines 29 - 51, Extract the
repeated SWR options into a single constant and reuse it in each useSWR call:
create a const (e.g., swrOpts or commonSwrOptions) with { revalidateOnFocus:
false } and replace the inline options object in the useSWR calls for
prdData/prdLoading/prdError, tasksData/tasksLoading/tasksError,
proofData/proofLoading/proofError, and reviewData/reviewLoading/reviewError so
all four useSWR invocations pass the shared swr options instead of duplicating
the object.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@web-ui/src/hooks/usePipelineStatus.ts`:
- Around line 29-51: Extract the repeated SWR options into a single constant and
reuse it in each useSWR call: create a const (e.g., swrOpts or commonSwrOptions)
with { revalidateOnFocus: false } and replace the inline options object in the
useSWR calls for prdData/prdLoading/prdError, tasksData/tasksLoading/tasksError,
proofData/proofLoading/proofError, and reviewData/reviewLoading/reviewError so
all four useSWR invocations pass the shared swr options instead of duplicating
the object.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b123c6f5-8982-47bf-bf0d-27ffc0c21fde

📥 Commits

Reviewing files that changed from the base of the PR and between 9f50ccb and f1694e2.

📒 Files selected for processing (2)
  • web-ui/src/__tests__/hooks/usePipelineStatus.test.ts
  • web-ui/src/hooks/usePipelineStatus.ts
✅ Files skipped from review due to trivial changes (1)
  • web-ui/src/tests/hooks/usePipelineStatus.test.ts

@frankbria frankbria merged commit 1a6e499 into main Mar 23, 2026
11 checks passed
@frankbria frankbria deleted the feature/issue-466-pipeline-progress-indicator branch March 24, 2026 23:28
Sign up for free to 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.

UX: Add pipeline progress indicator across Think→Build→Prove→Ship

1 participant