Skip to content

feat: Create project selection home page#176

Merged
frankbria merged 8 commits intomainfrom
feat/home-page-project-list
Jan 4, 2026
Merged

feat: Create project selection home page#176
frankbria merged 8 commits intomainfrom
feat/home-page-project-list

Conversation

@frankbria
Copy link
Owner

@frankbria frankbria commented Jan 4, 2026

Summary

Implements the core project selection home page functionality.

Closes #174

Changes

  • Home page transformation: Changed root route from direct project creation form to project list view
  • Project list with cards: Display projects in a responsive grid showing name, status, phase, and creation date
  • Create project flow: "Create New Project" button opens inline form with auto-start discovery
  • Back navigation: Added "← Projects" link in Dashboard header for easy return to project list
  • Enhanced empty state: Decorative icon, improved messaging, and "Get Started" CTA button
  • Authentication: Wrapped home page with ProtectedRoute

Files Changed

  • web-ui/src/app/page.tsx - Home page now renders ProjectList
  • web-ui/src/components/ProjectList.tsx - Enhanced with auto-discovery and improved empty state
  • web-ui/src/components/Dashboard.tsx - Added back navigation link
  • tests/e2e/test_project_creation.spec.ts - Updated E2E tests for new flow
  • tests/e2e/test-utils.ts - Updated helpers for new navigation

Test Coverage

  • Unit tests: 99+ new tests covering Navigation, ProjectList, Dashboard, and HomePage
  • E2E tests: 9 tests covering full user flows including back navigation

Test Plan

  • TypeScript type checking passes
  • ESLint passes with no warnings
  • Backend linting (ruff) passes
  • Frontend unit tests pass (CI)
  • Backend tests pass (CI)
  • E2E tests pass (CI)

Follow-up

Additional enhancements deferred to #175:

  • Agent status display on project cards
  • Deployment URLs
  • Enhanced project phase indicators

Summary by CodeRabbit

  • New Features

    • Home is now a protected "Your Projects" list with an improved empty state and an inline create flow that shows loading feedback and navigates to a project dashboard. Dashboards include a "Back to Projects" link.
  • Improvements

    • Creation flow gains clearer loading/error feedback, robust navigation after creation, and enhanced accessibility/keyboard support.
  • Tests

    • Expanded and reorganized test coverage for Home, Navigation, ProjectList, Dashboard, creation flows, accessibility, and auth scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

Replace project creation form with ProjectList component on home page.
Users now see their projects at root route with a "Create New Project"
button that opens an inline form. Enhanced ProjectList with auto-start
discovery and navigation to dashboard after project creation.

- Wrap home page with ProtectedRoute for authentication
- Add loading spinner with dynamic messages during creation
- Auto-start discovery process after project creation
- Navigate to project dashboard on successful creation
Unit Tests:
- Add page.test.tsx for HomePage with ProtectedRoute wrapper
- Add ProjectList.test.tsx with 32 test cases covering:
  - Loading/error/empty states
  - Project grid display
  - Create form toggle
  - Auto-start discovery after creation
  - Navigation to project dashboard

E2E Tests:
- Update test_project_creation.spec.ts for new flow
  (login -> project list -> create button -> form -> submit)
- Add test for navigating to existing projects
- Add test for form close button
- Update createTestProject helper in test-utils.ts
- Add navigateToProject helper function
Navigation Improvements:
- Add "Back to Projects" link in Dashboard header (← Projects)
- Verify Navigation logo links to project list (/)

Empty State Enhancement:
- Add decorative plus icon in circular background
- Improve messaging with "No projects yet" heading
- Add "Get Started" CTA button
- Better visual hierarchy and spacing

Tests (TDD approach):
- Add Navigation.test.tsx with 13 tests for logo, auth states, visibility
- Add Dashboard tests for back navigation link
- Add ProjectList tests for enhanced empty state
- Update E2E tests with back navigation and logo navigation tests
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 4, 2026

Caution

Review failed

The pull request is closed.

Walkthrough

Replace the immediate project-creation home with a protected project-list landing page that lists projects (or an empty state) and provides an inline create flow that starts discovery and navigates to a project dashboard; add a "Back to Projects" link in dashboards; update E2E utils and unit/e2e tests to the new list→create→dashboard flows.

Changes

Cohort / File(s) Summary
E2E utils & specs
tests/e2e/test-utils.ts, tests/e2e/test_project_creation.spec.ts
Add navigateToProject(page, projectName); refactor createTestProject to use the project-list UI (open inline create form → submit → wait for dashboard), extract/validate project ID from URL, and add explicit error handling. Tests updated to assert list→create→dashboard flow.
App entry / Home page
web-ui/src/app/page.tsx, web-ui/src/app/__tests__/page.test.tsx, web-ui/__tests__/app/page.test.tsx
Swap root page from create-centric view to ProjectList wrapped in ProtectedRoute; remove local create state/handlers; simplify tests to verify ProtectedRoute, layout, and ProjectList presence.
ProjectList & creation flow
web-ui/src/components/ProjectList.tsx, web-ui/src/components/__tests__/*ProjectList.test.tsx, web-ui/src/components/__tests__/ProjectList.test.tsx
Add isCreating/loadingMessage and spinner flow; wire handleSubmit/onSuccess/onError to call startProject, refresh list (mutate), and navigate to /projects/{id}; enhance empty-state UI and keyboard/accessibility on project cards. Tests mock startProject and verify creation/start/navigation behavior.
Dashboard navigation
web-ui/src/components/Dashboard.tsx, web-ui/__tests__/components/Dashboard.test.tsx
Add a "Back to Projects" Link (data-testid: back-to-projects) in dashboard header and tests asserting its presence and href /.
Navigation & auth-related tests
web-ui/__tests__/components/Navigation.test.tsx, web-ui/__tests__/components/ProjectList.test.tsx, web-ui/__tests__/components/Dashboard.test.tsx
New/expanded tests covering navigation visibility across routes, auth states (authenticated/unauthenticated/loading), user menu/logout behavior, project list states (loading/error/empty), and creation lifecycle with mocked APIs.
Unit test adjustments / mocks
web-ui/__tests__/app/page.test.tsx, web-ui/__tests__/components/*
Replace heavy mocks (ProjectCreationForm, Spinner) with test doubles; prefer test-id based lookups and update text/layout assertions to match new UI tokens.
API test surface (mock)
web-ui/src/lib/api (test mocks)
projectsApi test mock extended to include startProject (new API interaction surfaced in tests).
CI workflow note
.github/workflows/claude-code-review.yml
Text-only addition: reviewer NOTE block added to Claude Code Review prompt (no logic changes).

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Home as ProjectList (Home)
    participant API as Projects API
    participant Dashboard as Project Dashboard

    Note right of Home `#f0f4ff`: ProtectedRoute wraps Home

    User->>Home: Load page
    activate Home
    Home->>API: GET /projects
    API-->>Home: projects[]
    Home->>User: render list or empty state
    deactivate Home

    alt User creates project
        User->>Home: Open inline create form
        Home->>User: show form
        User->>Home: Submit form (POST /projects)
        activate Home
        Home->>API: POST /projects (create)
        API-->>Home: { id }
        Home->>API: POST /projects/{id}/start (startProject)
        API-->>Home: accepted
        Home->>Dashboard: navigate to /projects/{id}
        deactivate Home
        activate Dashboard
        Dashboard->>User: render project dashboard (with back link)
        deactivate Dashboard
    else User selects existing project
        User->>Home: Click project card
        Home->>Dashboard: navigate to /projects/{id}
    end

    User->>Dashboard: Click "Back to Projects"
    Dashboard->>Home: navigate to /
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 I hopped from root to project lanes,

Lists gleam where empty plains once were.
I press "Create", the discovery sings,
A dashboard waits with back-link wings.
I bound back home—happy little hops!

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main change: creating a project selection/listing home page instead of a direct creation flow.
Linked Issues check ✅ Passed The PR meets core requirements from #174: displays a project list on home page, shows empty state when no projects exist, provides Create Project action, and adds back navigation from Dashboard.
Out of Scope Changes check ✅ Passed All changes directly support the project list home page feature. The GitHub Actions workflow update is a minor documentation enhancement unrelated to core functionality.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d800e02 and 3b622d2.

📒 Files selected for processing (3)
  • web-ui/__tests__/components/ProjectList.test.tsx
  • web-ui/src/components/ProjectList.tsx
  • web-ui/src/components/__tests__/ProjectList.test.tsx

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 (8)
tests/e2e/test-utils.ts (2)

182-189: Consider extracting duplicate URL extraction logic.

The project ID extraction from URL (regex match + error throw) is duplicated in both createTestProject and navigateToProject. Consider extracting this into a helper function.

🔎 Proposed refactor
+/**
+ * Extract project ID from URL
+ * @param url - Full URL string
+ * @returns Project ID
+ * @throws Error if ID cannot be extracted
+ */
+function extractProjectIdFromUrl(url: string): string {
+  const match = url.match(/\/projects\/(\d+)/);
+  if (!match) {
+    throw new Error('Failed to extract project ID from URL');
+  }
+  return match[1];
+}

Then use it in both functions:

return extractProjectIdFromUrl(page.url());

Also applies to: 216-223


209-211: Text-based selector may be fragile for project names with special characters.

Using page.locator(text=${projectName}) could fail if project names contain regex-special characters or match multiple elements. Consider using a more specific selector pattern.

🔎 Proposed fix for robust selection
-  // Find and click the project card
-  const projectCard = page.locator(`text=${projectName}`).first();
-  await projectCard.click();
+  // Find and click the project card by exact text match
+  const projectCard = page.getByText(projectName, { exact: true }).first();
+  await projectCard.click();
tests/e2e/test_project_creation.spec.ts (2)

130-131: Use test-id selector instead of text content for close button.

Using getByText('✕') is fragile—it relies on a specific Unicode character that could change. Prefer a data-testid selector for better stability.

🔎 Proposed fix

The component should add a test-id to the close button:

<button
  onClick={() => setShowForm(false)}
  data-testid="close-create-form-button"
  className="text-muted-foreground hover:text-foreground"
></button>

Then update the test:

-    // Click close button (✕)
-    await page.getByText('✕').click();
+    // Click close button
+    await page.getByTestId('close-create-form-button').click();

167-169: Text-based locator may be fragile.

Same issue as in test-utils.ts—consider using getByText with exact match option for more reliable selection.

🔎 Proposed fix
-    // Find and click the project card
-    const projectCard = page.locator(`text=${projectName}`).first();
+    // Find and click the project card by exact text
+    const projectCard = page.getByText(projectName, { exact: true }).first();
web-ui/src/components/ProjectList.tsx (4)

73-79: Error handling swallows failure silently—consider user feedback.

The startProject error is logged but the user sees no indication that auto-start failed. While navigation continues (acceptable fallback), consider showing a toast or brief message so users know they may need to manually start discovery.


154-172: Use Hugeicons instead of inline SVG.

Per coding guidelines, use @hugeicons/react for all icons instead of inline SVGs. This maintains consistency across the codebase.

🔎 Proposed fix
+import { Add01Icon } from '@hugeicons/react';
...
           <div
             data-testid="empty-state-icon"
             className="mx-auto w-16 h-16 mb-6 rounded-full bg-primary/10 flex items-center justify-center"
           >
-            <svg
-              className="w-8 h-8 text-primary"
-              fill="none"
-              stroke="currentColor"
-              viewBox="0 0 24 24"
-              aria-hidden="true"
-            >
-              <path
-                strokeLinecap="round"
-                strokeLinejoin="round"
-                strokeWidth={2}
-                d="M12 6v6m0 0v6m0-6h6m-6 0H6"
-              />
-            </svg>
+            <Add01Icon className="w-8 h-8 text-primary" aria-hidden="true" />
           </div>

Based on coding guidelines: "Use Hugeicons (@hugeicons/react) for all icons instead of lucide-react".


132-137: Add data-testid to close button for E2E test stability.

The close button lacks a test-id, making E2E tests rely on the character which is fragile.

🔎 Proposed fix
             <button
               onClick={() => setShowForm(false)}
+              data-testid="close-create-form-button"
               className="text-muted-foreground hover:text-foreground"
             >
               ✕
             </button>

32-42: Consider adding React.memo for performance optimization.

Per coding guidelines, Dashboard sub-components should implement React.memo. If ProjectList is rendered as part of the Dashboard hierarchy, wrapping it with React.memo prevents unnecessary re-renders.

🔎 Proposed fix
-export default function ProjectList() {
+function ProjectList() {
   // ... component implementation
 }
+
+export default React.memo(ProjectList);

Also add React to imports:

-import { useState } from 'react';
+import React, { useState } from 'react';

Based on coding guidelines: "Implement React.memo on all Dashboard sub-components for performance optimization".

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4202976 and 84e4a91.

📒 Files selected for processing (9)
  • tests/e2e/test-utils.ts
  • tests/e2e/test_project_creation.spec.ts
  • web-ui/__tests__/app/page.test.tsx
  • web-ui/__tests__/components/Dashboard.test.tsx
  • web-ui/__tests__/components/Navigation.test.tsx
  • web-ui/__tests__/components/ProjectList.test.tsx
  • web-ui/src/app/page.tsx
  • web-ui/src/components/Dashboard.tsx
  • web-ui/src/components/ProjectList.tsx
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use TypeScript 5.3+ with strict mode for frontend development

Files:

  • web-ui/__tests__/app/page.test.tsx
  • web-ui/__tests__/components/ProjectList.test.tsx
  • web-ui/src/app/page.tsx
  • tests/e2e/test-utils.ts
  • tests/e2e/test_project_creation.spec.ts
  • web-ui/__tests__/components/Dashboard.test.tsx
  • web-ui/__tests__/components/Navigation.test.tsx
  • web-ui/src/components/ProjectList.tsx
  • web-ui/src/components/Dashboard.tsx
web-ui/**/*.{css,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Tailwind CSS with Nova design system template for styling

Files:

  • web-ui/__tests__/app/page.test.tsx
  • web-ui/__tests__/components/ProjectList.test.tsx
  • web-ui/src/app/page.tsx
  • web-ui/__tests__/components/Dashboard.test.tsx
  • web-ui/__tests__/components/Navigation.test.tsx
  • web-ui/src/components/ProjectList.tsx
  • web-ui/src/components/Dashboard.tsx
web-ui/{__tests__,tests}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use npm test for frontend component testing in web-ui

Files:

  • web-ui/__tests__/app/page.test.tsx
  • web-ui/__tests__/components/ProjectList.test.tsx
  • web-ui/__tests__/components/Dashboard.test.tsx
  • web-ui/__tests__/components/Navigation.test.tsx
web-ui/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

web-ui/src/**/*.{ts,tsx}: Use React 18 with TypeScript and Context + useReducer pattern for state management
Use shadcn/ui components from @/components/ui/ directory
Use Hugeicons (@hugeicons/react) for all icons instead of lucide-react
Implement WebSocket automatic reconnection with exponential backoff (1s → 30s)

Files:

  • web-ui/src/app/page.tsx
  • web-ui/src/components/ProjectList.tsx
  • web-ui/src/components/Dashboard.tsx
{codeframe/**/*.py,web-ui/src/**/*.{ts,tsx}}

📄 CodeRabbit inference engine (CLAUDE.md)

{codeframe/**/*.py,web-ui/src/**/*.{ts,tsx}}: Use WebSockets for real-time updates between frontend and backend
Use last-write-wins strategy with backend timestamps for timestamp conflict resolution in multi-agent scenarios

Files:

  • web-ui/src/app/page.tsx
  • web-ui/src/components/ProjectList.tsx
  • web-ui/src/components/Dashboard.tsx
web-ui/src/**/*.{tsx,css}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Nova color palette variables (bg-card, text-foreground, etc.) instead of hardcoded color values

Files:

  • web-ui/src/app/page.tsx
  • web-ui/src/components/ProjectList.tsx
  • web-ui/src/components/Dashboard.tsx
web-ui/src/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

web-ui/src/**/*.tsx: Use cn() utility for conditional Tailwind CSS classes
Wrap AgentStateProvider with ErrorBoundary component for graceful error handling
Use useMemo for derived state calculations in React components

Files:

  • web-ui/src/app/page.tsx
  • web-ui/src/components/ProjectList.tsx
  • web-ui/src/components/Dashboard.tsx
tests/**/*.{py,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use TestSprite and Playwright for E2E testing of workflows

Files:

  • tests/e2e/test-utils.ts
  • tests/e2e/test_project_creation.spec.ts
web-ui/src/components/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Implement React.memo on all Dashboard sub-components for performance optimization

Files:

  • web-ui/src/components/ProjectList.tsx
  • web-ui/src/components/Dashboard.tsx
🧠 Learnings (8)
📚 Learning: 2025-12-24T04:24:43.825Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-24T04:24:43.825Z
Learning: Applies to web-ui/{__tests__,tests}/**/*.{ts,tsx} : Use npm test for frontend component testing in web-ui

Applied to files:

  • web-ui/__tests__/app/page.test.tsx
  • web-ui/__tests__/components/ProjectList.test.tsx
  • tests/e2e/test-utils.ts
  • tests/e2e/test_project_creation.spec.ts
  • web-ui/__tests__/components/Dashboard.test.tsx
  • web-ui/__tests__/components/Navigation.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/**/*.{ts,tsx} : Use Next.js 14 with React 18 App Router for the frontend

Applied to files:

  • web-ui/__tests__/app/page.test.tsx
  • web-ui/src/app/page.tsx
  • web-ui/__tests__/components/Navigation.test.tsx
  • 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__/app/page.test.tsx
  • web-ui/__tests__/components/ProjectList.test.tsx
  • tests/e2e/test-utils.ts
  • tests/e2e/test_project_creation.spec.ts
  • web-ui/__tests__/components/Dashboard.test.tsx
  • web-ui/__tests__/components/Navigation.test.tsx
📚 Learning: 2025-12-24T04:24:43.825Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-24T04:24:43.825Z
Learning: Applies to tests/**/*.{py,ts,tsx} : Use TestSprite and Playwright for E2E testing of workflows

Applied to files:

  • web-ui/__tests__/components/ProjectList.test.tsx
  • tests/e2e/test_project_creation.spec.ts
📚 Learning: 2025-12-24T04:24:43.825Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-24T04:24:43.825Z
Learning: Applies to web-ui/src/**/*.{ts,tsx} : Use React 18 with TypeScript and Context + useReducer pattern for state management

Applied to files:

  • web-ui/src/app/page.tsx
  • web-ui/src/components/ProjectList.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/src/**/*.{ts,tsx} : Use SWR for server state management and useState for local state in React

Applied to files:

  • web-ui/src/app/page.tsx
  • web-ui/src/components/ProjectList.tsx
  • web-ui/src/components/Dashboard.tsx
📚 Learning: 2025-12-24T04:24:43.825Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-24T04:24:43.825Z
Learning: Applies to web-ui/src/components/**/*.tsx : Implement React.memo on all Dashboard sub-components for performance optimization

Applied to files:

  • web-ui/__tests__/components/Dashboard.test.tsx
  • web-ui/src/components/Dashboard.tsx
📚 Learning: 2025-12-24T04:24:43.825Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-24T04:24:43.825Z
Learning: Applies to web-ui/src/**/*.{ts,tsx} : Use shadcn/ui components from @/components/ui/ directory

Applied to files:

  • web-ui/src/components/Dashboard.tsx
🧬 Code graph analysis (4)
web-ui/__tests__/app/page.test.tsx (1)
web-ui/src/app/page.tsx (1)
  • HomePage (14-35)
web-ui/__tests__/components/ProjectList.test.tsx (1)
web-ui/src/components/ProjectList.tsx (1)
  • ProjectList (32-218)
web-ui/__tests__/components/Navigation.test.tsx (2)
web-ui/src/contexts/AuthContext.tsx (1)
  • useAuth (74-80)
web-ui/src/components/Navigation.tsx (1)
  • Navigation (14-78)
web-ui/src/components/ProjectList.tsx (1)
web-ui/src/components/Spinner.tsx (1)
  • Spinner (12-31)
⏰ 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: Frontend Unit Tests
  • GitHub Check: Backend Unit Tests
  • GitHub Check: claude-review
  • GitHub Check: E2E Smoke Tests (Chromium)
🔇 Additional comments (16)
web-ui/src/components/Dashboard.tsx (1)

8-8: LGTM! Clean navigation implementation.

The back-to-projects link is well-implemented using Next.js Link for client-side navigation, follows Nova design tokens, and includes proper test identifiers. The arrow indicator (←) provides clear visual affordance for navigation.

Also applies to: 243-251

web-ui/__tests__/components/ProjectList.test.tsx (3)

1-76: LGTM! Excellent test infrastructure setup.

The test file demonstrates best practices with proper SWR cache isolation via renderWithSWR helper, comprehensive mocking of dependencies, and clear test data fixtures. This ensures deterministic test behavior.


331-459: LGTM! Comprehensive creation flow testing.

The project creation flow tests thoroughly cover the UX journey: form submission, loading states with spinner, discovery initiation, navigation to dashboard, and error recovery. The console.error spy at line 406 correctly suppresses expected error logs during failure scenario testing.


488-513: LGTM! Proper accessibility coverage.

The accessibility tests verify semantic HTML structure with proper heading hierarchy and ensure interactive elements are keyboard-accessible via cursor-pointer styling. This aligns with WCAG best practices.

web-ui/__tests__/app/page.test.tsx (1)

1-97: LGTM! Solid page-level test coverage.

The test suite effectively validates the HomePage component's key responsibilities: authentication protection via ProtectedRoute, proper layout with Nova design tokens (bg-background, max-w-7xl), semantic HTML structure, and ProjectList integration. The focused mocking strategy allows isolated testing of page-level concerns.

web-ui/src/app/page.tsx (1)

1-35: LGTM! Clean architectural refactor.

The refactor successfully separates concerns by moving project creation logic into the ProjectList component while keeping the home page focused on authentication and layout. The ProtectedRoute wrapper ensures security, and the Nova design system tokens maintain visual consistency. This aligns with React component composition best practices.

web-ui/__tests__/components/Dashboard.test.tsx (1)

1161-1204: No duplicate test suites found.

The "Back to Projects Navigation" test suite appears only once in the file at line 1166. There is no duplication that needs to be addressed.

Likely an incorrect or invalid review comment.

tests/e2e/test-utils.ts (1)

147-149: LGTM on updated JSDoc!

The documentation clearly explains the new flow with inline form creation from the project list.

tests/e2e/test_project_creation.spec.ts (3)

23-33: Good coverage of project list display.

Tests properly verify the page header and create button visibility with appropriate test-id selectors.


225-233: Empty state test is acknowledged as potentially flaky—consider skipping or fixing.

The comment notes the test may be flaky if other tests leave projects behind. Consider either:

  1. Using test.skip with a tracking issue
  2. Adding cleanup logic in beforeEach to ensure a clean state
  3. Converting to a pure assertion that the page loads correctly (current approach)

The current implementation is acceptable as it documents the limitation and still provides value.


138-143: Good test isolation with fresh login per test.

Each test in the navigation flow suite properly clears cookies and logs in fresh, ensuring test independence.

web-ui/__tests__/components/Navigation.test.tsx (2)

1-22: Well-structured test setup with proper mocking.

The mock setup for next/navigation and AuthContext is clean and follows best practices for isolating component tests.


62-114: Good coverage of visibility rules.

Tests properly verify that Navigation hides on login/signup pages and renders on project list and detail pages, matching the component's conditional rendering logic.

web-ui/src/components/ProjectList.tsx (3)

48-63: Good separation of loading state handlers.

The handleSubmit and handleError callbacks properly manage the isCreating state for the creation flow, providing clear feedback to users.


120-147: Well-implemented loading state during creation.

The conditional rendering between the Spinner and form based on isCreating provides good UX feedback during the creation flow, with clear loading messages.


150-191: Enhanced empty state UI is well-designed.

The empty state now includes a decorative icon, clear messaging, and a prominent CTA button. Good use of Nova color palette variables (bg-primary/10, text-primary, text-foreground, text-muted-foreground).

Update tests in src/ directory to match new home page architecture:
- page.test.tsx: Test ProtectedRoute wrapper and ProjectList display
- ProjectList.test.tsx: Test navigation after creation, auto-start discovery

The home page now displays a project list instead of a creation form.
The logout handler is async - it awaits logout() before calling
router.push('/login'). Updated test to use waitFor to await the
async side-effect and verify the redirect assertion.
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: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
web-ui/src/components/__tests__/ProjectList.test.tsx (2)

63-286: Add test coverage for creation loading state and startProject error handling.

The test suite covers the happy path well, but is missing coverage for two important scenarios:

  1. Loading state during project creation: The ProjectList component shows a loading spinner and message while creating a project (see isCreating state in ProjectList.tsx), but no test verifies this behavior.

  2. Error handling for startProject failures: If the startProject API call fails after successful project creation, the error path is untested. This could leave users in an inconsistent state.

Recommended test additions

Add these tests to improve coverage:

test('shows loading state during project creation', async () => {
  (projectsApi.list as jest.Mock).mockResolvedValue({
    data: { projects: [] },
  });

  renderWithSWR(<ProjectList />);

  await waitFor(() => {
    expect(screen.getByTestId('create-project-button')).toBeInTheDocument();
  });

  const createButton = screen.getByTestId('create-project-button');
  await userEvent.click(createButton);

  // Click submit but don't wait for completion
  const submitButton = screen.getByText('Submit Form');
  await userEvent.click(submitButton);

  // Should show loading state
  expect(screen.getByTestId('spinner')).toBeInTheDocument();
});

test('handles startProject error gracefully', async () => {
  (projectsApi.list as jest.Mock).mockResolvedValue({
    data: { projects: [] },
  });
  (projectsApi.startProject as jest.Mock).mockRejectedValue(
    new Error('Failed to start discovery')
  );

  renderWithSWR(<ProjectList />);

  await waitFor(() => {
    expect(screen.getByTestId('create-project-button')).toBeInTheDocument();
  });

  const createButton = screen.getByTestId('create-project-button');
  await userEvent.click(createButton);

  const submitButton = screen.getByText('Submit Form');
  await userEvent.click(submitButton);

  // Should handle error appropriately
  await waitFor(() => {
    expect(screen.getByText(/error/i)).toBeInTheDocument();
  });
});

66-72: Use a mock response that matches the StartProjectResponse interface.

The mock resolves to an empty object {}, but StartProjectResponse requires message and status fields. Replace with:

(projectsApi.startProject as jest.Mock).mockResolvedValue({
  message: 'Project started successfully',
  status: 'active',
});

This ensures test fidelity and matches the API contract defined in web-ui/src/types/index.ts.

🧹 Nitpick comments (2)
web-ui/src/app/__tests__/page.test.tsx (1)

33-40: Consider removing unused router mock setup.

The mockPush is set up in beforeEach but never verified in any test. Since the tests focus on structural verification rather than navigation behavior, this setup could be removed to reduce noise.

🔎 Proposed cleanup
 describe('HomePage', () => {
-  const mockPush = jest.fn();
-
   beforeEach(() => {
-    mockPush.mockClear();
     (useRouter as jest.Mock).mockReturnValue({
-      push: mockPush,
+      push: jest.fn(),
     });
   });
web-ui/src/components/__tests__/ProjectList.test.tsx (1)

23-45: Consider reducing setTimeout delay for faster, more reliable tests.

The mock correctly simulates the async project creation flow. However, the 50ms delay in setTimeout could potentially cause flakiness in CI environments, though waitFor should handle it.

Optional: Reduce delay for faster test execution
           onSubmit?.();
           // Simulate async success after a brief delay
-          setTimeout(() => onSuccess(3), 50);
+          setTimeout(() => onSuccess(3), 10);

This maintains the async behavior while reducing test execution time and potential timeout issues.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 84e4a91 and aa5a986.

📒 Files selected for processing (2)
  • web-ui/src/app/__tests__/page.test.tsx
  • web-ui/src/components/__tests__/ProjectList.test.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use TypeScript 5.3+ with strict mode for frontend development

Files:

  • web-ui/src/app/__tests__/page.test.tsx
  • web-ui/src/components/__tests__/ProjectList.test.tsx
web-ui/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

web-ui/src/**/*.{ts,tsx}: Use React 18 with TypeScript and Context + useReducer pattern for state management
Use shadcn/ui components from @/components/ui/ directory
Use Hugeicons (@hugeicons/react) for all icons instead of lucide-react
Implement WebSocket automatic reconnection with exponential backoff (1s → 30s)

Files:

  • web-ui/src/app/__tests__/page.test.tsx
  • web-ui/src/components/__tests__/ProjectList.test.tsx
web-ui/**/*.{css,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Tailwind CSS with Nova design system template for styling

Files:

  • web-ui/src/app/__tests__/page.test.tsx
  • web-ui/src/components/__tests__/ProjectList.test.tsx
{codeframe/**/*.py,web-ui/src/**/*.{ts,tsx}}

📄 CodeRabbit inference engine (CLAUDE.md)

{codeframe/**/*.py,web-ui/src/**/*.{ts,tsx}}: Use WebSockets for real-time updates between frontend and backend
Use last-write-wins strategy with backend timestamps for timestamp conflict resolution in multi-agent scenarios

Files:

  • web-ui/src/app/__tests__/page.test.tsx
  • web-ui/src/components/__tests__/ProjectList.test.tsx
web-ui/src/**/*.{tsx,css}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Nova color palette variables (bg-card, text-foreground, etc.) instead of hardcoded color values

Files:

  • web-ui/src/app/__tests__/page.test.tsx
  • web-ui/src/components/__tests__/ProjectList.test.tsx
web-ui/src/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

web-ui/src/**/*.tsx: Use cn() utility for conditional Tailwind CSS classes
Wrap AgentStateProvider with ErrorBoundary component for graceful error handling
Use useMemo for derived state calculations in React components

Files:

  • web-ui/src/app/__tests__/page.test.tsx
  • web-ui/src/components/__tests__/ProjectList.test.tsx
web-ui/src/components/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Implement React.memo on all Dashboard sub-components for performance optimization

Files:

  • web-ui/src/components/__tests__/ProjectList.test.tsx
🧠 Learnings (6)
📚 Learning: 2025-12-24T04:24:43.825Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-24T04:24:43.825Z
Learning: Applies to web-ui/{__tests__,tests}/**/*.{ts,tsx} : Use npm test for frontend component testing in web-ui

Applied to files:

  • web-ui/src/app/__tests__/page.test.tsx
  • web-ui/src/components/__tests__/ProjectList.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/**/*.{ts,tsx} : Use Next.js 14 with React 18 App Router for the frontend

Applied to files:

  • web-ui/src/app/__tests__/page.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/src/app/__tests__/page.test.tsx
  • web-ui/src/components/__tests__/ProjectList.test.tsx
📚 Learning: 2025-12-24T04:24:43.825Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-24T04:24:43.825Z
Learning: Applies to web-ui/src/**/*.{ts,tsx} : Use React 18 with TypeScript and Context + useReducer pattern for state management

Applied to files:

  • web-ui/src/app/__tests__/page.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/src/components/**/*.{ts,tsx} : Use functional React components with TypeScript interfaces

Applied to files:

  • web-ui/src/app/__tests__/page.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/src/**/*.{ts,tsx} : Use SWR for server state management and useState for local state in React

Applied to files:

  • web-ui/src/components/__tests__/ProjectList.test.tsx
🧬 Code graph analysis (2)
web-ui/src/app/__tests__/page.test.tsx (1)
web-ui/src/app/page.tsx (1)
  • HomePage (14-35)
web-ui/src/components/__tests__/ProjectList.test.tsx (2)
web-ui/src/lib/api.ts (1)
  • projectsApi (30-54)
web-ui/src/components/ProjectList.tsx (1)
  • ProjectList (32-218)
⏰ 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 (7)
web-ui/src/app/__tests__/page.test.tsx (3)

18-30: LGTM! Clean mock setup.

The ProtectedRoute and ProjectList mocks are well-structured for testing the HomePage component in isolation. The pass-through approach with test IDs is appropriate for verifying structural composition.


42-66: LGTM! Structural tests align with implementation.

The Page Structure tests appropriately verify:

  • ProtectedRoute wrapper (per PR objectives)
  • Heading and tagline content matching the actual implementation
  • ProjectList component presence

These structural assertions are well-suited for unit testing, while behavioral flows are covered by E2E tests as noted in the PR objectives.


68-82: LGTM! Layout tests follow design system guidelines.

The responsive layout tests correctly verify:

  • Nova design system color variables (bg-background)
  • Responsive container classes (min-h-screen)
  • Max-width constraint (max-w-7xl)

All assertions align with the actual implementation in page.tsx.

web-ui/src/components/__tests__/ProjectList.test.tsx (4)

14-19: LGTM! Correct mock setup for the new auto-start discovery flow.

The addition of startProject to the mocked API surface correctly reflects the new behavior where project creation automatically triggers discovery.


47-52: LGTM! Appropriate Spinner mock for test isolation.

The mock correctly isolates the Spinner component while preserving testability through the test-id and size prop.


196-221: LGTM! Test correctly verifies navigation to project dashboard after creation.

The test properly validates the new flow where successful project creation navigates to the project dashboard. Using test-ids instead of text selectors makes the test more resilient to UI copy changes.


223-245: LGTM! Test correctly verifies auto-start discovery feature.

This test properly validates that the new auto-start discovery flow calls startProject with the correct project ID after creation, which is a key feature of this PR.

@claude
Copy link

claude bot commented Jan 4, 2026

Code Review: PR #176 - Project Selection Home Page

Overall Assessment: APPROVED with Minor Recommendations ⭐⭐⭐⭐☆ (4/5)

This PR successfully implements a project selection home page with solid architecture and comprehensive test coverage.

Key Strengths ✅

  • Comprehensive test coverage (99+ unit tests)
  • Proper authentication integration with ProtectedRoute
  • Follows Context + Reducer pattern from CLAUDE.md
  • Enhanced UX with auto-discovery and empty states
  • Consistent use of shadcn/ui Nova components

Priority Issues to Address Before Merge

1. Missing Error Boundary (Priority: HIGH)

Location: web-ui/src/app/page.tsx

No ErrorBoundary wrapping async components. Should wrap ProtectedRoute with ErrorBoundary component following the pattern from AgentStateProvider.tsx.

2. Race Condition in Auto-Discovery (Priority: HIGH)

Location: web-ui/src/components/ProjectList.tsx

Sequential API calls without proper error propagation could leave projects in inconsistent state if discovery fails after project creation succeeds. Wrap discovery call in try-catch and always refresh project list even if discovery fails.

3. Missing ARIA Labels (Priority: HIGH)

Location: ProjectList.tsx interactive elements

Project cards lack proper accessibility attributes. Add role="button", tabIndex, aria-label, and keyboard event handlers (Enter/Space) to clickable cards. Include live region for status updates.

4. Keyboard Navigation Support (Priority: HIGH)

Location: Project grid and creation form

Missing focus management, skip links, and keyboard event handlers. Add focus management after project creation and proper keyboard navigation throughout.

Performance Optimizations (Post-Merge)

  • Add React.memo to ProjectList and ProjectCard components
  • Use useMemo for sorted projects array
  • Use useCallback for event handlers

Test Coverage Gaps

Consider adding tests for:

  • Concurrent project creation
  • Discovery failure recovery
  • Network timeout handling

Security ✅

  • Authentication properly implemented
  • Follows FastAPI Users JWT pattern
  • Auth token correctly included in API calls

Recommendation

APPROVED with required fixes for Issues 1-4 before merge. Post-merge optimizations can be addressed in follow-up PRs. Great work on comprehensive test coverage!

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 (1)
.github/workflows/claude-code-review.yml (1)

60-68: Consider tightening the NOTE block for clarity.

The NOTE block is functional but could be more concise and direct. Consider simplifying the language to improve readability.

🔎 Suggested refinement
-            NOTE: review the other comments on the pull request - including yours.
-            If you are reviewing changes or enhancements beyond the first creation of the pull request,
-            make sure your comments are consistent with your previous reviews, or are 
-            referring to them in a consistent way.
-
-            There's no need to repeat information unless it is critical and not
-            being reflected in comments or code. Be aware of your prior reviews and that the new file information
-            may be changes because of previous reviews.
+            NOTE: Review other comments on this PR, including your own previous reviews.
+            When reviewing subsequent changes, ensure consistency with your prior feedback
+            and reference earlier comments where relevant. Avoid repeating information
+            unless critical and not yet addressed. Remember that file changes may reflect
+            earlier review suggestions.
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aa5a986 and 798e8b4.

📒 Files selected for processing (2)
  • .github/workflows/claude-code-review.yml
  • web-ui/__tests__/components/Navigation.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • web-ui/tests/components/Navigation.test.tsx
⏰ 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). (2)
  • GitHub Check: Backend Unit Tests
  • GitHub Check: E2E Smoke Tests (Chromium)

1. ErrorBoundary wrapper (page.tsx)
   - Wrap ProtectedRoute with ErrorBoundary for graceful error handling

2. Race condition fix (ProjectList.tsx)
   - Add mutate() call in finally block to refresh list after creation
   - Ensures data consistency even if discovery fails

3. Accessibility improvements (ProjectList.tsx)
   - Add role="button" to project cards
   - Add tabIndex={0} for keyboard navigation
   - Add aria-label with project context
   - Add keyboard handlers for Enter/Space
   - Add focus ring styles for visibility
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 (1)
web-ui/src/app/page.tsx (1)

17-36: Consider removing the duplicate "Your Projects" heading.

The page header at lines 23-28 duplicates the heading in ProjectList.tsx (line 128). This creates redundant information for screen reader users and adds unnecessary visual hierarchy. Since ProjectList already renders "Your Projects" as an h2, consider either:

  1. Removing the heading from this page and letting ProjectList own it, OR
  2. Removing the heading from ProjectList and keeping it here at the page level

The second option would better align with typical Next.js page composition patterns where the page owns the top-level heading.

🔎 Proposed refactor to remove duplicate heading

Option 1: Remove heading from page.tsx (simpler change)

-            {/* Page Header */}
-            <div className="mb-8">
-              <h1 className="text-3xl font-bold text-foreground">
-                Your Projects
-              </h1>
-              <p className="mt-2 text-muted-foreground">
-                AI coding agents that work autonomously while you sleep
-              </p>
-            </div>
-
             {/* Project List with Create Button */}
             <ProjectList />

Then update ProjectList.tsx line 128 to use h1 instead of h2.

Option 2: Remove heading from ProjectList.tsx (better separation of concerns)

Keep the heading here and remove line 128 from ProjectList.tsx.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 798e8b4 and 83c9af9.

📒 Files selected for processing (3)
  • .github/workflows/claude-code-review.yml
  • web-ui/src/app/page.tsx
  • web-ui/src/components/ProjectList.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/claude-code-review.yml
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use TypeScript 5.3+ with strict mode for frontend development

Files:

  • web-ui/src/components/ProjectList.tsx
  • web-ui/src/app/page.tsx
web-ui/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

web-ui/src/**/*.{ts,tsx}: Use React 18 with TypeScript and Context + useReducer pattern for state management
Use shadcn/ui components from @/components/ui/ directory
Use Hugeicons (@hugeicons/react) for all icons instead of lucide-react
Implement WebSocket automatic reconnection with exponential backoff (1s → 30s)

Files:

  • web-ui/src/components/ProjectList.tsx
  • web-ui/src/app/page.tsx
web-ui/**/*.{css,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Tailwind CSS with Nova design system template for styling

Files:

  • web-ui/src/components/ProjectList.tsx
  • web-ui/src/app/page.tsx
{codeframe/**/*.py,web-ui/src/**/*.{ts,tsx}}

📄 CodeRabbit inference engine (CLAUDE.md)

{codeframe/**/*.py,web-ui/src/**/*.{ts,tsx}}: Use WebSockets for real-time updates between frontend and backend
Use last-write-wins strategy with backend timestamps for timestamp conflict resolution in multi-agent scenarios

Files:

  • web-ui/src/components/ProjectList.tsx
  • web-ui/src/app/page.tsx
web-ui/src/**/*.{tsx,css}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Nova color palette variables (bg-card, text-foreground, etc.) instead of hardcoded color values

Files:

  • web-ui/src/components/ProjectList.tsx
  • web-ui/src/app/page.tsx
web-ui/src/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

web-ui/src/**/*.tsx: Use cn() utility for conditional Tailwind CSS classes
Wrap AgentStateProvider with ErrorBoundary component for graceful error handling
Use useMemo for derived state calculations in React components

Files:

  • web-ui/src/components/ProjectList.tsx
  • web-ui/src/app/page.tsx
web-ui/src/components/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Implement React.memo on all Dashboard sub-components for performance optimization

Files:

  • web-ui/src/components/ProjectList.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-24T04:24:43.825Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-24T04:24:43.825Z
Learning: Applies to web-ui/src/**/*.{ts,tsx} : Use React 18 with TypeScript and Context + useReducer pattern for state management

Applied to files:

  • web-ui/src/app/page.tsx
🧬 Code graph analysis (2)
web-ui/src/components/ProjectList.tsx (2)
web-ui/src/lib/api.ts (1)
  • projectsApi (30-54)
web-ui/src/components/Spinner.tsx (1)
  • Spinner (12-31)
web-ui/src/app/page.tsx (2)
web-ui/src/components/auth/ProtectedRoute.tsx (1)
  • ProtectedRoute (7-26)
web-ui/src/components/ProjectList.tsx (1)
  • ProjectList (32-240)
⏰ 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 (6)
web-ui/src/app/page.tsx (1)

17-36: LGTM! Proper authentication and error boundary wrapping.

The component structure correctly implements the required patterns:

  • ErrorBoundary wrapping for graceful error handling (per coding guidelines)
  • ProtectedRoute for authentication protection
  • Semantic HTML with proper Tailwind design system tokens (bg-background, text-foreground, text-muted-foreground)
web-ui/src/components/ProjectList.tsx (5)

35-36: LGTM! Clear state management for creation flow.

The isCreating and loadingMessage states effectively control the loading UI during project creation and discovery phases.


48-53: LGTM! Proper keyboard accessibility implementation.

The keyboard handler correctly:

  • Supports both Enter and Space keys per ARIA authoring practices
  • Prevents default behavior to avoid unwanted page scroll
  • Delegates to the existing click handler for consistent behavior

55-102: LGTM! Well-designed creation lifecycle with graceful error handling.

The three handlers work together effectively:

  • handleSubmit: Sets loading state before API call
  • handleError: Cleans up loading state on failure
  • handleProjectCreated: Orchestrates post-creation flow with intentional error tolerance

The error handling strategy in handleProjectCreated is particularly well-thought-out:

  • Discovery errors are logged but don't block navigation (user can manually retry)
  • mutate() errors are silently handled to ensure navigation proceeds
  • Navigation always happens regardless of discovery/refresh outcome

This fail-forward approach prevents users from being stuck in a loading state while still providing useful functionality.


169-209: LGTM! Well-designed empty state with proper accessibility.

The enhanced empty state provides excellent UX with:

  • Decorative icon correctly marked with aria-hidden="true"
  • Clear, engaging messaging that communicates the product value
  • Prominent CTA that reveals the creation form
  • Consistent use of Nova design system tokens
  • Test IDs for reliable E2E testing

The duplicate "Get Started" button (vs. header "Create New Project") is acceptable as it provides a more prominent call-to-action when the list is empty.


212-235: LGTM! Excellent accessibility implementation for project cards.

The project cards demonstrate best practices for accessible interactive elements:

  • role="button" with tabIndex={0} for keyboard navigation
  • Descriptive aria-label including project name, status, and phase
  • Keyboard handler supporting Enter and Space keys
  • Clear focus ring styles (focus:ring-2 focus:ring-primary focus:ring-offset-2)
  • Proper hover and transition effects

This implementation ensures keyboard-only users and screen reader users have full access to project navigation.

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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: 0

🧹 Nitpick comments (2)
web-ui/src/components/ProjectList.tsx (2)

15-22: Consider adding validation for invalid date strings.

If dateString is malformed, new Date() produces an Invalid Date, and toLocaleDateString() returns "Invalid Date" which would display in the UI.

🔎 Proposed defensive handling
 const formatDate = (dateString: string): string => {
   const date = new Date(dateString);
+  if (isNaN(date.getTime())) {
+    return '';
+  }
   return date.toLocaleDateString('en-US', {
     year: 'numeric',
     month: 'long',
     day: 'numeric',
   });
 };

177-190: Replace inline SVG with Hugeicons per coding guidelines.

The coding guidelines specify using Hugeicons (@hugeicons/react) for all icons. Consider replacing this inline SVG with an appropriate Hugeicons icon.

🔎 Proposed change using Hugeicons
+import { Add01Icon } from '@hugeicons/react';

Then replace the SVG:

             <div
               data-testid="empty-state-icon"
               className="mx-auto w-16 h-16 mb-6 rounded-full bg-primary/10 flex items-center justify-center"
             >
-              <svg
-                className="w-8 h-8 text-primary"
-                fill="none"
-                stroke="currentColor"
-                viewBox="0 0 24 24"
-                aria-hidden="true"
-              >
-                <path
-                  strokeLinecap="round"
-                  strokeLinejoin="round"
-                  strokeWidth={2}
-                  d="M12 6v6m0 0v6m0-6h6m-6 0H6"
-                />
-              </svg>
+              <Add01Icon className="w-8 h-8 text-primary" aria-hidden="true" />
             </div>

Based on coding guidelines: "Use Hugeicons (@hugeicons/react) for all icons instead of lucide-react".

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 83c9af9 and d800e02.

📒 Files selected for processing (1)
  • web-ui/src/components/ProjectList.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use TypeScript 5.3+ with strict mode for frontend development

Files:

  • web-ui/src/components/ProjectList.tsx
web-ui/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

web-ui/src/**/*.{ts,tsx}: Use React 18 with TypeScript and Context + useReducer pattern for state management
Use shadcn/ui components from @/components/ui/ directory
Use Hugeicons (@hugeicons/react) for all icons instead of lucide-react
Implement WebSocket automatic reconnection with exponential backoff (1s → 30s)

Files:

  • web-ui/src/components/ProjectList.tsx
web-ui/**/*.{css,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Tailwind CSS with Nova design system template for styling

Files:

  • web-ui/src/components/ProjectList.tsx
{codeframe/**/*.py,web-ui/src/**/*.{ts,tsx}}

📄 CodeRabbit inference engine (CLAUDE.md)

{codeframe/**/*.py,web-ui/src/**/*.{ts,tsx}}: Use WebSockets for real-time updates between frontend and backend
Use last-write-wins strategy with backend timestamps for timestamp conflict resolution in multi-agent scenarios

Files:

  • web-ui/src/components/ProjectList.tsx
web-ui/src/**/*.{tsx,css}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Nova color palette variables (bg-card, text-foreground, etc.) instead of hardcoded color values

Files:

  • web-ui/src/components/ProjectList.tsx
web-ui/src/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

web-ui/src/**/*.tsx: Use cn() utility for conditional Tailwind CSS classes
Wrap AgentStateProvider with ErrorBoundary component for graceful error handling
Use useMemo for derived state calculations in React components

Files:

  • web-ui/src/components/ProjectList.tsx
web-ui/src/components/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Implement React.memo on all Dashboard sub-components for performance optimization

Files:

  • web-ui/src/components/ProjectList.tsx
🧬 Code graph analysis (1)
web-ui/src/components/ProjectList.tsx (2)
web-ui/src/lib/api.ts (1)
  • projectsApi (30-54)
web-ui/src/components/Spinner.tsx (1)
  • Spinner (12-31)
⏰ 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/src/components/ProjectList.tsx (4)

48-53: Good accessibility implementation for keyboard navigation.

The keyboard handler correctly supports both Enter and Space keys with proper default prevention, enabling keyboard-only users to navigate project cards.


79-102: Solid error handling with graceful degradation.

The pattern of logging discovery failures while still navigating ensures users aren't blocked by non-critical errors. The nested try-catch in the finally block properly isolates cache refresh failures.


141-164: LGTM! Creation form with loading state is well implemented.

The spinner with dynamic loading message provides good user feedback. The accessibility issue from the previous review (missing aria-label on close button) has been addressed on line 152.


214-221: Excellent accessibility enhancements on project cards.

The additions of role="button", tabIndex={0}, descriptive aria-label, keyboard handler, and visible focus styles make the cards fully accessible to keyboard and screen reader users.

Per coding guidelines, use @hugeicons/react for all icons.
Replaced inline plus SVG in empty state with Add01Icon.
Added Hugeicons mock to test files.
@frankbria frankbria merged commit d0b4437 into main Jan 4, 2026
6 of 7 checks passed
@frankbria frankbria deleted the feat/home-page-project-list branch January 4, 2026 06:25
@claude
Copy link

claude bot commented Jan 4, 2026

Follow-up Code Review: PR #176 - Project Selection Home Page

Status: APPROVED ✅

Excellent work addressing all the HIGH priority issues from my previous review! The code quality has significantly improved.

✅ Issues Resolved

All four HIGH priority issues from my previous review have been successfully addressed:

  1. ✅ Error Boundary Added (web-ui/src/app/page.tsx:17)

    • ErrorBoundary now properly wraps ProtectedRoute
    • Follows the pattern from AgentStateProvider as recommended
  2. ✅ Race Condition Fixed (web-ui/src/components/ProjectList.tsx:84-103)

    • Discovery errors are now caught with try-catch
    • Project list is always refreshed via mutate() regardless of discovery outcome
    • Clear error logging for debugging
    • User navigation proceeds even if discovery fails
  3. ✅ ARIA Labels Added (web-ui/src/components/ProjectList.tsx:204-209)

    • Project cards now have proper role="button"
    • tabIndex={0} enables keyboard focus
    • Descriptive aria-label includes project name, status, and phase
    • Focus styling with ring-2 ring-primary
  4. ✅ Keyboard Navigation (web-ui/src/components/ProjectList.tsx:49-54, 208)

    • onKeyDown handler supports both Enter and Space keys
    • preventDefault() prevents page scroll on Space
    • Consistent with WCAG 2.1 keyboard interaction patterns
    • Close button has aria-label for screen readers

Minor Recommendations (Low Priority)

These are minor improvements that could be addressed in a future PR:

  1. Test Selector Fragility (tests/e2e/test_project_creation.spec.ts:131)

    • The test still uses getByText('✕') for the close button
    • Consider adding a data-testid="close-create-form-button" for more stable selection
    • Current approach works but may break if the icon changes to an SVG/component
  2. Performance Optimizations (Post-merge)

    • Consider adding React.memo to ProjectList component for re-render optimization
    • Use useMemo for the sorted projects array if sorting logic is added
    • Use useCallback for event handlers to prevent unnecessary re-renders
    • These are optimizations, not blockers
  3. Live Region for Status Updates (Future Enhancement)

    • Consider adding an ARIA live region for screen reader announcements during project creation
    • Example: <div aria-live="polite" aria-atomic="true" className="sr-only">{loadingMessage}</div>
    • This would announce "Creating your project..." and "Starting discovery..." to screen reader users

Test Coverage ✅

The PR includes comprehensive test coverage:

  • 99+ new unit tests covering all major flows
  • 9 E2E tests including navigation scenarios
  • Tests cover loading, error, empty states, and creation lifecycle
  • All critical user paths are tested

Code Quality ✅

  • Follows shadcn/ui Nova design system guidelines
  • Uses Hugeicons consistently
  • Proper semantic color variables (bg-card, text-foreground, etc.)
  • Clean error handling with appropriate fallbacks
  • Well-documented code with JSDoc comments

Security & Authentication ✅

  • Properly wrapped with ProtectedRoute
  • JWT authentication flow correctly implemented
  • No security concerns identified

Recommendation

APPROVED for merge. All critical issues have been resolved. The minor recommendations above can be addressed in follow-up PRs if desired, but they are not blocking issues.

Great work on the comprehensive fixes! 🎉

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.

[P0] Create project selection home page

1 participant