Fix PRD generation progress tracking UI - #194
Conversation
- Backend: Add progress_pct: 100 to prd_generation_completed WebSocket message - Frontend DiscoveryProgress: Add onViewPRD callback prop for View PRD button - Frontend DiscoveryProgress: Add section minimization after PRD completion (3s delay) - Frontend DiscoveryProgress: Add "Next: Task Creation" phase indicator - Frontend DiscoveryProgress: Add View PRD button in both expanded and minimized states - Frontend Dashboard: Update View PRD button state based on PRD availability - Frontend Dashboard: Show spinner and "Generating PRD..." during generation - Frontend Dashboard: Pass onViewPRD handler to DiscoveryProgress component - Tests: Add PRD progress tracking test cases
WalkthroughBackend adds Changes
Sequence DiagramsequenceDiagram
autonumber
actor User
participant Dashboard
participant DiscoveryProgress
participant WebSocket
participant Backend
Backend->>WebSocket: emit `prd_generation_completed` (progress_pct: 100)
WebSocket->>DiscoveryProgress: broadcast `prd_generation_completed`
DiscoveryProgress->>DiscoveryProgress: set prdCompleted = true
Note over DiscoveryProgress: schedule auto-minimize after 3s
DiscoveryProgress->>Dashboard: render minimized PRD section (or expanded if user opens)
rect rgb(230,245,235)
Dashboard->>Dashboard: update PRD button state & styling (based on prdData?.status)
end
User->>Dashboard: click "View PRD" (if visible)
Dashboard->>DiscoveryProgress: invoke `onViewPRD` callback
DiscoveryProgress->>DiscoveryProgress: open PRD modal / expand view
alt User manually expands
User->>DiscoveryProgress: click Expand
DiscoveryProgress->>DiscoveryProgress: isSectionMinimized = false
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (5)📓 Common learnings📚 Learning: 2026-01-04T06:26:12.870ZApplied to files:
📚 Learning: 2026-01-04T06:26:12.870ZApplied to files:
📚 Learning: 2025-11-25T19:08:37.203ZApplied to files:
📚 Learning: 2026-01-04T06:26:12.870ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (8)
Comment |
Code Review: PRD Generation Progress Tracking UII've reviewed the changes in this PR. Overall, this is a solid UX improvement with good implementation quality. Here are my findings: ✅ Strengths1. Backend Change
2. Frontend Architecture
3. UI/UX
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @web-ui/__tests__/components/DiscoveryProgress.test.tsx:
- Around line 1896-1945: The tests in DiscoveryProgress.test.tsx claim to verify
PRD behavior but never trigger the WebSocket-driven prdCompleted state or
interact with the button; update the two failing specs to either (A) mock the
WebSocket client used by the component (e.g., mock getWebSocketClient or the
module in '@/lib/websocket') and emit a prd_generation_completed message for
project_id 1 so the component sets prdCompleted and the "View PRD" button
renders, or (B) avoid relying on WebSocket behavior and instead set the
component state via the API response and then assert on the presence of the
"View PRD" button and call the onViewPRD mock to verify clicks; specifically
modify the tests "should show View PRD button when prdCompleted is true" and
"should call onViewPRD callback when View PRD button is clicked" to simulate the
prd_generation_completed message (or set prdCompleted) and then query and click
the button to assert rendering and the onViewPRD invocation.
In @web-ui/src/components/DiscoveryProgress.tsx:
- Around line 20-23: Update the exported interface DiscoveryProgressProps so it
matches the component’s shape: add refreshInterval?: number and onViewPRD?: ()
=> void to the exported DiscoveryProgressProps declaration (the same interface
name that is exported from types/discovery.ts) so imports elsewhere get the
correct optional props.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
codeframe/ui/routers/discovery.pyweb-ui/__tests__/components/DiscoveryProgress.test.tsxweb-ui/src/components/Dashboard.tsxweb-ui/src/components/DiscoveryProgress.tsx
🧰 Additional context used
📓 Path-based instructions (5)
web-ui/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web-ui/src/**/*.{ts,tsx}: Use TypeScript 5.3+ for frontend development with React 18, Next.js 14, Tailwind CSS, and Hugeicons for icons
Use TypeScript strict mode and ensure 100% type safety with no hardcoded types or any usage in frontend code
Files:
web-ui/src/components/Dashboard.tsxweb-ui/src/components/DiscoveryProgress.tsx
web-ui/src/components/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
web-ui/src/components/**/*.tsx: Use shadcn/ui Nova template components with semantic color palette (bg-card, text-foreground, etc.) and avoid hardcoded color values
Use cn() utility for conditional Tailwind CSS classes and follow Nova's compact spacing conventions
Files:
web-ui/src/components/Dashboard.tsxweb-ui/src/components/DiscoveryProgress.tsx
web-ui/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Replace all icon usage with Hugeicons (@hugeicons/react) and do not mix with lucide-react
Files:
web-ui/src/components/Dashboard.tsxweb-ui/src/components/DiscoveryProgress.tsx
web-ui/src/components/Dashboard.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use React.memo on all Dashboard sub-components and useMemo for derived state to optimize performance with multi-agent support
Files:
web-ui/src/components/Dashboard.tsx
codeframe/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
codeframe/**/*.py: Use Python 3.11+ for backend development with FastAPI, AsyncAnthropic, SQLite with async support (aiosqlite), and tiktoken for token counting
Use token counting via tiktoken library for token budget management with ~50,000 token limit per conversation
Use asyncio patterns with AsyncAnthropic for async/await in Python backend for concurrent operations
Implement quality gates with multi-stage pre-completion checks (tests → type → coverage → review) and Git + SQLite + context snapshots for project state rollback
Use tiered memory system (HOT/WARM/COLD) with importance scoring using hybrid exponential decay algorithm for context management with 30-50% token reduction
Implement session lifecycle management with auto-save/restore using file-based storage at .codeframe/session_state.json
Files:
codeframe/ui/routers/discovery.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T06:26:12.870Z
Learning: Applies to web-ui/src/app/page.tsx : Implement automatic project discovery start after project creation with loading state transitions and 'Start Discovery' button for idle projects
📚 Learning: 2026-01-04T06:26:12.870Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T06:26:12.870Z
Learning: Applies to web-ui/src/app/page.tsx : Implement automatic project discovery start after project creation with loading state transitions and 'Start Discovery' button for idle projects
Applied to files:
web-ui/src/components/Dashboard.tsxweb-ui/__tests__/components/DiscoveryProgress.test.tsxweb-ui/src/components/DiscoveryProgress.tsx
📚 Learning: 2026-01-04T06:26:12.870Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T06:26:12.870Z
Learning: Applies to web-ui/src/components/Dashboard.tsx : Use React.memo on all Dashboard sub-components and useMemo for derived state to optimize performance with multi-agent support
Applied to files:
web-ui/src/components/Dashboard.tsx
📚 Learning: 2025-11-25T19:08:37.203Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:37.203Z
Learning: Applies to docs/web-ui/**/__tests__/**/*.test.{ts,tsx} : Create JavaScript test files colocated or in __tests__/ as *.test.ts
Applied to files:
web-ui/__tests__/components/DiscoveryProgress.test.tsx
🧬 Code graph analysis (2)
web-ui/__tests__/components/DiscoveryProgress.test.tsx (2)
web-ui/src/types/api.ts (1)
DiscoveryProgressResponse(135-139)web-ui/src/lib/api.ts (1)
projectsApi(30-62)
web-ui/src/components/DiscoveryProgress.tsx (1)
web-ui/src/types/discovery.ts (1)
DiscoveryProgressProps(46-52)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Backend Unit Tests
- GitHub Check: Frontend Unit Tests
- GitHub Check: E2E Smoke Tests (Chromium)
- GitHub Check: claude-review
🔇 Additional comments (8)
codeframe/ui/routers/discovery.py (1)
119-128: LGTM!The addition of
progress_pct: 100to theprd_generation_completedbroadcast message is correct and maintains consistency with theprd_generation_progressmessage structure. This enables the frontend to properly display 100% completion.web-ui/__tests__/components/DiscoveryProgress.test.tsx (1)
1994-2014: Valid test case.This test correctly verifies that the PRD generation status section is rendered when discovery is completed. The
getByTestId('prd-generation-status')assertion is meaningful.web-ui/src/components/Dashboard.tsx (2)
279-296: LGTM - PRD button state management is correct.The button correctly handles three states:
available: Primary styling, enabledgenerating: Secondary styling with spinner, disabled viacursor-wait- Other (unavailable): Muted styling, disabled
The spinner animation is appropriate and the conditional rendering is clean.
412-412: LGTM - onViewPRD callback integration.Passing
onViewPRD={() => setShowPRD(true)}enables the DiscoveryProgress component to trigger the PRD modal, providing a consistent user experience from both the header button and the discovery progress section.web-ui/src/components/DiscoveryProgress.tsx (4)
387-395: LGTM - Auto-minimize effect is correctly implemented.The effect properly:
- Only triggers when
prdCompletedbecomes true- Uses a 3-second delay giving users time to see the completion state
- Returns cleanup function to clear the timeout on unmount or re-render
604-636: LGTM - Minimized view implementation.The minimized view correctly:
- Only renders when both
isSectionMinimizedandprdCompletedare true (line 605)- Shows a compact success state with the "View PRD" button (when
onViewPRDis provided) and an "Expand" button- Uses proper accessibility attributes (
aria-labelon Expand button)- Follows the semantic color palette (
bg-success/10,border-success)
658-747: LGTM - PRD generation status section.The status section correctly handles all four states:
- Generating (lines 667-694): Shows spinner, progress message, progress bar with percentage, and stage indicator
- Completed (lines 695-713): Shows success icon, completion message, and View PRD button
- Error (lines 714-733): Shows error icon, error message, and retry button
- Starting (lines 734-745): Shows spinner with "Starting PRD Generation..." message
The progress bar updates smoothly via the
prdProgressPctstate driven by WebSocket messages.
749-762: LGTM - Task creation phase indicator.The "Next: Task Creation" indicator appropriately appears only when:
- PRD generation is completed (
prdCompleted)- The project phase is
planningThis provides clear guidance to users about the next step in the workflow.
…dge cases Increase DiscoveryProgress component test coverage from 54% to 100% function coverage: - Add WebSocket message handler tests for all 7 message types - Add stuck state detection tests with 30-second timeout - Add PRD error state and retry functionality tests - Add minimized view and auto-minimize behavior tests - Add duplicate submission prevention test - Fix timing issues with act() wrapper for timer advancement - Mock WebSocket client and message handlers properly
Updated Code Review: PRD Generation Progress Tracking UIGreat work addressing the test quality issues! 🎉 I've reviewed the latest commit (bbdcb7b) and I'm pleased to see that all the critical test assertion issues have been fixed. The test suite now properly validates the behavior rather than just checking for DOM existence. ✅ Issues Resolved1. Test Assertions - FIXED ✅All 6 PRD progress tracking tests now have proper assertions:
📊 Test Coverage SummaryThe new test suite now includes:
Total: 29+ comprehensive tests covering the full PRD generation lifecycle
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @web-ui/__tests__/components/DiscoveryProgress.test.tsx:
- Around line 2669-2718: The test "should disable retry button during retry
attempt" never asserts the disabled state; after clicking the retry button
(data-testid 'retry-prd-button') and before resolving the mocked retry promise
(mockRetryPrdGeneration), add an assertion that the button is disabled (e.g.,
expect(screen.getByTestId('retry-prd-button')).toBeDisabled()) to verify the
loading/disabled UI during the in-flight retry; ensure you check this state
immediately after firing the click and before calling resolveRetry().
- Around line 2072-2095: The test "should display PRD progress percentage during
generation" only asserts the status element exists but never simulates
prd_generation_progress WebSocket messages or checks the shown percentage;
update the test to mock or stub the WebSocket/EventSource used by
DiscoveryProgress, emit a sequence of prd_generation_progress messages (e.g.,
with progress_percentage 0, 10, 30, 80, 100) after rendering, and assert that
the element obtained via screen.getByTestId('prd-generation-status') (or the
component's percentage display element) updates to the expected percentage text
after each message using waitFor/expect(screen.getByTestId(...).textContent).
Ensure you reuse the existing projectsApi.getDiscoveryProgress mock
(mockResolvedValue with completedData) and clean up the mocked socket between
tests.
🧹 Nitpick comments (1)
web-ui/__tests__/components/DiscoveryProgress.test.tsx (1)
2532-2579: Duplicate test with incomplete assertion.This test is nearly identical to
should call restartDiscovery when restart button is clicked(lines 2444-2493). The only difference is it uses a slow-resolving promise, but it doesn't assert on the loading state during the delay—it just verifies the mock was called, which the other test already covers.Consider either:
- Removing this duplicate test, or
- Adding assertions for the loading state (e.g., button disabled, spinner visible) to differentiate it.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web-ui/__tests__/components/DiscoveryProgress.test.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-04T06:26:12.870Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T06:26:12.870Z
Learning: Applies to web-ui/src/app/page.tsx : Implement automatic project discovery start after project creation with loading state transitions and 'Start Discovery' button for idle projects
Applied to files:
web-ui/__tests__/components/DiscoveryProgress.test.tsx
📚 Learning: 2025-11-25T19:08:37.203Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:37.203Z
Learning: Applies to docs/web-ui/**/__tests__/**/*.test.{ts,tsx} : Create JavaScript test files colocated or in __tests__/ as *.test.ts
Applied to files:
web-ui/__tests__/components/DiscoveryProgress.test.tsx
📚 Learning: 2026-01-04T06:26:12.870Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T06:26:12.870Z
Learning: Applies to tests/e2e/**/*.ts : Implement E2E tests using Playwright + TestSprite with loginUser() helper from tests/e2e/test-utils.ts for authentication
Applied to files:
web-ui/__tests__/components/DiscoveryProgress.test.tsx
🧬 Code graph analysis (1)
web-ui/__tests__/components/DiscoveryProgress.test.tsx (2)
web-ui/src/types/api.ts (1)
DiscoveryProgressResponse(135-139)web-ui/src/lib/api.ts (1)
projectsApi(30-62)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Backend Unit Tests
- GitHub Check: Frontend Unit Tests
- GitHub Check: E2E Smoke Tests (Chromium)
🔇 Additional comments (4)
web-ui/__tests__/components/DiscoveryProgress.test.tsx (4)
31-54: Well-structured WebSocket mock implementation.The mock correctly manages handler registration/cleanup and provides a clean
simulateWsMessagehelper for driving WebSocket-based state changes in tests. This properly addresses the previous review feedback about incomplete PRD tests.
2101-2404: Comprehensive WebSocket message handler coverage.Tests cover all 7 WebSocket message types and include an important edge case for ignoring messages from different projects. This ensures the component only responds to relevant project updates.
2725-2894: Good coverage of minimized view behavior with proper timer handling.Tests correctly use
act()wrapper when advancing timers that trigger React state updates. This follows React Testing Library best practices and prevents "act" warnings.
2935-2979: Good test for duplicate submission prevention.The test correctly verifies that rapid clicks only result in a single API call, which is important for preventing duplicate form submissions.
- Update PRD progress test to actually verify percentage updates (10%, 30%, 80%) by simulating WebSocket prd_generation_progress messages - Fix retry button test to verify correct component behavior: clicking retry clears the error state and transitions to loading spinner, not a disabled button - Both tests now have proper assertions that match actual component behavior
Final Review: PRD Generation Progress Tracking UI ✅Excellent work on this PR! I've reviewed the latest changes and I'm pleased to confirm this is ready to merge. 📊 What This PR DeliversThis PR successfully implements a comprehensive PRD generation progress tracking system with: Backend Changes
Frontend UX Improvements
Test Coverage (1,150+ lines added!)The test suite is exceptionally comprehensive with 29+ tests covering:
All tests include proper assertions - this addresses the test quality concerns from Issue #172. 🎯 Code Quality Assessment
💡 Optional Future EnhancementsThese are NOT blockers - just ideas for future iteration:
✨ HighlightsWhat I especially appreciate:
📝 RecommendationStatus: ✅ APPROVE - Ready to Merge This PR is production-ready and delivers significant UX improvements with robust test coverage. The optional enhancements listed above can be addressed in follow-up PRs if desired. Great work on this feature! 🎉 |
2 participants
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.