feat(ui): add Git visualization components (#272) #283
Conversation
Add Git branch and commit visualization to the Dashboard Overview tab: - Add Git types (GitBranch, GitCommit, GitStatus, GitState) - Add Git API client for consuming endpoints from ticket #270 - Add 7 Git reducer actions for state management - Create GitBranchIndicator component (current branch with dirty indicator) - Create CommitHistory component (recent commits with relative timestamps) - Create BranchList component (branches with status badges) - Create GitSection container with SWR data fetching (30s refresh) - Integrate GitSection in Dashboard Overview tab (active/review/complete phases) - Add WebSocket handlers for commit_created and branch_created events Test coverage: 74 new tests (all passing)
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a complete Git UI subsystem: typed Git API client, Git TypeScript types and initial state, reducer actions and WebSocket handlers for commit/branch events, four React components (GitSection, GitBranchIndicator, CommitHistory, BranchList) with dashboard integration, and extensive unit tests across layers. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as GitSection (UI)
participant SWR as SWR (fetch layer)
participant API as Git API client
participant Backend as Backend HTTP API
participant WS as WebSocket
participant Reducer as Agent Reducer / Store
UI->>SWR: subscribe to status, commits, branches
SWR->>API: getGitStatus / getCommits / getBranches
API->>Backend: HTTP requests (/api/projects/:id/git/...)
Backend-->>API: respond with JSON (status/commits/branches)
API-->>SWR: resolve promises
SWR-->>UI: provide data -> UI renders children
Note over WS,Reducer: real-time events
WS->>Reducer: receive commit_created / branch_created -> dispatch action
Reducer->>Reducer: update gitState (prepend commit / append branch)
Reducer-->>UI: store notifies subscribers -> UI updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (4)web-ui/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
web-ui/src/components/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
web-ui/src/reducers/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
web-ui/src/lib/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (10)📓 Common learnings📚 Learning: 2026-01-11T23:33:31.895ZApplied to files:
📚 Learning: 2025-11-25T19:08:37.203ZApplied to files:
📚 Learning: 2026-01-11T23:33:31.895ZApplied to files:
📚 Learning: 2026-01-11T23:33:31.895ZApplied to files:
📚 Learning: 2025-11-25T19:08:37.203ZApplied to files:
📚 Learning: 2026-01-11T23:33:31.895ZApplied to files:
📚 Learning: 2026-01-11T23:33:31.895ZApplied to files:
📚 Learning: 2026-01-11T23:33:31.895ZApplied to files:
📚 Learning: 2025-11-25T19:08:37.203ZApplied to files:
🧬 Code graph analysis (4)web-ui/src/components/git/CommitHistory.tsx (1)
web-ui/src/components/git/GitSection.tsx (4)
web-ui/src/reducers/agentReducer.ts (1)
web-ui/src/lib/websocketMessageMapper.ts (2)
⏰ 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). (5)
🔇 Additional comments (21)
Comment |
Add Git visualization to the Dashboard and implement authenticated Git API clients in
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| {(projectData.phase === 'active' || | ||
| projectData.phase === 'review' || | ||
| projectData.phase === 'complete') && ( |
There was a problem hiding this comment.
Phase check here bypasses normalizePhase(...). Consider using it so 'development' (or future aliases) still show GitSection.
| {(projectData.phase === 'active' || | |
| projectData.phase === 'review' || | |
| projectData.phase === 'complete') && ( | |
| {(normalizePhase(projectData.phase) === 'development' || | |
| normalizePhase(projectData.phase) === 'review' || | |
| normalizePhase(projectData.phase) === 'complete') && ( |
🚀 Want me to fix this? Reply ex: "fix it for me".
| function formatRelativeTime(timestamp: string): string { | ||
| const date = new Date(timestamp); | ||
| const now = new Date(); | ||
| const diffMs = now.getTime() - date.getTime(); |
There was a problem hiding this comment.
formatRelativeTime renders "Invalid Date" when commit.timestamp is invalid. Consider adding a guard (e.g., check isNaN(date.getTime())) and return a safe fallback string.
| const diffMs = now.getTime() - date.getTime(); | |
| const time = date.getTime(); | |
| if (Number.isNaN(time)) return ''; | |
| const diffMs = now.getTime() - time; |
🚀 Want me to fix this? Reply ex: "fix it for me".
Review: PR #283 - Git Visualization ComponentsOverall AssessmentThis is a well-structured implementation that follows the project's conventions and integrates cleanly with the existing state management system. The code quality is high with good TypeScript typing, proper use of memoization, and follows the Nova design system. Test coverage is comprehensive with 74 new tests. ✅ StrengthsCode Quality:
Architecture:
Testing:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @web-ui/__tests__/components/git/GitBranchIndicator.test.tsx:
- Around line 114-129: Update the GitBranchIndicator test to assert the branch
element is visible before checking styling: after rendering with the long-name
GitStatus and finding branchName via screen.getByText(/feature\/very-long/), add
an assertion that branchName is visible (e.g., expect(branchName).toBeVisible())
and then assert its closest('[data-testid="branch-indicator"]') has the
'truncate' class; this ensures the element is rendered and then verifies the
styling via the existing class check.
In @web-ui/src/components/git/CommitHistory.tsx:
- Around line 77-83: The <time> element in the CommitHistory component is using
an invalid ARIA role ("time"); remove the role attribute from the <time> tag
(the element rendering commit.timestamp in CommitHistory.tsx) so the markup
relies on the native <time> semantics (no change to formatRelativeTime is
required).
In @web-ui/src/components/git/GitSection.tsx:
- Around line 53-62: The SWR key used in useSWR inside GitSection.tsx omits the
maxCommits parameter causing stale cached results when maxCommits changes;
update the cache key string (the first arg to useSWR, currently
`git-commits-${projectId}`) to include maxCommits (e.g., append `-${maxCommits}`
or use a tuple) so the fetcher getCommits(projectId, { limit: maxCommits }) is
re-run whenever maxCommits changes and the cache is invalidated.
🧹 Nitpick comments (14)
web-ui/src/lib/websocketMessageMapper.ts (1)
250-270: Consider validatingcommit_hashforcommit_createdmessages.Unlike the
agent_createdhandler (lines 92-97) which validatesagent_idand returnsnullif missing,commit_createdsilently accepts an emptycommit_hash. This could result in commits with empty hashes appearing in the UI, making them unidentifiable.Suggested validation pattern
case 'commit_created': { const msg = message as WebSocketMessage; + + // Warn if commit_hash is missing - commits without hashes may be unidentifiable + if (!msg.commit_hash) { + if (process.env.NODE_ENV === 'development') { + console.warn('[WebSocketMapper] commit_created message missing commit_hash'); + } + } + // Create a GitCommit-compatible structure from the WebSocket message return { type: 'COMMIT_CREATED',web-ui/src/types/git.ts (1)
121-133: Minor: Function comments say "type guard" but these are regular boolean helpers.True TypeScript type guards use type predicates (
branch is X). These functions are simple boolean helpers, which is perfectly fine for their use case. Consider updating the comments to avoid confusion./** - * Type guard to check if branch is active + * Check if branch is active */ export function isBranchActive(branch: GitBranch): boolean { return branch.status === 'active'; } /** - * Type guard to check if branch is merged + * Check if branch is merged */ export function isBranchMerged(branch: GitBranch): boolean { return branch.status === 'merged'; }web-ui/src/api/git.ts (1)
46-85: Consider adding client-side validation for thelimitoption.The comment states
limitshould be 1-100, but there's no client-side validation. While the backend likely validates this, adding client-side bounds checking would provide better developer experience with immediate feedback.♻️ Optional: Add limit validation
export async function getCommits( projectId: number, options: GetCommitsOptions = {} ): Promise<GitCommit[]> { const params = new URLSearchParams(); if (options.branch) { params.append('branch', options.branch); } if (options.limit !== undefined) { + if (options.limit < 1 || options.limit > 100) { + throw new Error('Limit must be between 1 and 100'); + } params.append('limit', options.limit.toString()); }web-ui/src/components/Dashboard.tsx (1)
556-563: Consider usingnormalizePhase()for consistency with other phase checks.The Git section uses
projectData.phasedirectly, while other phase-dependent sections (lines 463, 471, 722, 738) usenormalizePhase(projectData.phase). SincenormalizePhasemaps'active'to'development', the current condition should work, but using the normalized form would be more consistent and safer if phase naming conventions change.♻️ Suggested refactor for consistency
{/* Git Visualization Section (Ticket #272) */} - {(projectData.phase === 'active' || - projectData.phase === 'review' || - projectData.phase === 'complete') && ( + {(['development', 'review', 'complete'].includes( + normalizePhase(projectData.phase) + )) && ( <div className="mb-6" data-testid="git-section-container"> <GitSection projectId={projectId} maxCommits={5} /> </div> )}web-ui/__tests__/api/git.test.ts (1)
1-213: Good test coverage for core API functions.The tests effectively cover:
- Authentication header inclusion
- Error handling (missing auth, API failures, 404)
- URL construction with query parameters
- Branch name encoding (
feature/test→feature%2Ftest)Consider adding tests for the convenience functions
getCurrentBranchandgetRecentCommitsto ensure complete coverage of the public API surface.web-ui/__tests__/components/git/CommitHistory.test.tsx (1)
9-9: Remove unusedfireEventimport.
fireEventis imported but not used in any test case.🔧 Suggested fix
-import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen } from '@testing-library/react';web-ui/__tests__/components/git/GitSection.test.tsx (1)
90-153: Consider extracting the repeated SWR mock setup.The same
mockUseSWR.mockImplementationlogic is duplicated in three test cases (lines 92-103, 112-123, 135-146). Extract it to a helper function to improve maintainability.♻️ Suggested refactor
+function mockLoadedData() { + mockUseSWR.mockImplementation((key: string) => { + if (key.includes('status')) { + return { data: mockStatus, error: undefined, isLoading: false }; + } + if (key.includes('commits')) { + return { data: mockCommits, error: undefined, isLoading: false }; + } + if (key.includes('branches')) { + return { data: mockBranches, error: undefined, isLoading: false }; + } + return { data: undefined, error: undefined, isLoading: false }; + }); +} describe('data display', () => { it('should render all Git components when data is loaded', () => { - mockUseSWR.mockImplementation((key: string) => { - // ... repeated logic - }); + mockLoadedData(); render(<GitSection projectId={1} />); expect(screen.getByText(/code & git/i)).toBeInTheDocument(); });web-ui/src/components/git/index.ts (1)
8-17: Consider using named exports in the source components.The barrel re-exports default exports as named exports, which works but adds indirection. Based on learnings, named exports are preferred. Consider changing the source components to use named exports directly.
Current pattern:
// Component file export default GitBranchIndicator; // Barrel export { default as GitBranchIndicator } from './GitBranchIndicator';Preferred pattern:
// Component file export const GitBranchIndicator = ...; // or: export function GitBranchIndicator() {...} // Barrel export { GitBranchIndicator } from './GitBranchIndicator';web-ui/src/reducers/agentReducer.ts (1)
383-487: Consider extracting the repeated gitState initialization pattern.The pattern
state.gitState ?? { ...INITIAL_GIT_STATE }is repeated in all 7 Git action handlers. Consider extracting a helper function to reduce duplication.♻️ Suggested helper function
+function getGitState(state: AgentState): GitState { + return state.gitState ?? { ...INITIAL_GIT_STATE }; +} case 'GIT_STATUS_LOADED': { const { status } = action.payload; - const currentGitState = state.gitState ?? { ...INITIAL_GIT_STATE }; + const currentGitState = getGitState(state); // ... }web-ui/src/components/git/CommitHistory.tsx (2)
32-45: Consider edge cases for invalid timestamps.The
formatRelativeTimefunction doesn't handle invalid date inputs. Iftimestampis an invalid date string,new Date(timestamp)returnsInvalid Date, and subsequent calculations will produceNaN, resulting in"NaNm ago"being rendered.🛡️ Suggested defensive handling
function formatRelativeTime(timestamp: string): string { const date = new Date(timestamp); + if (isNaN(date.getTime())) { + return 'unknown'; + } const now = new Date(); const diffMs = now.getTime() - date.getTime();
55-87: Improve accessibility for clickable commit items.The
CommitItemis rendered as a clickable<div>but lacks keyboard accessibility. Users navigating with a keyboard cannot focus or activate it. Consider using a<button>or addingtabIndex,role, andonKeyDownhandlers.♿ Suggested accessibility improvement
<div data-testid="commit-item" - className="flex items-start gap-3 py-2 px-3 rounded-md hover:bg-muted/50 transition-colors cursor-pointer" + role="button" + tabIndex={onClick ? 0 : undefined} + className="flex items-start gap-3 py-2 px-3 rounded-md hover:bg-muted/50 transition-colors cursor-pointer focus:outline-none focus:ring-2 focus:ring-ring" onClick={onClick} + onKeyDown={(e) => { + if (onClick && (e.key === 'Enter' || e.key === ' ')) { + e.preventDefault(); + onClick(); + } + }} >web-ui/src/components/git/BranchList.tsx (3)
32-41: Missing exhaustiveness check forgetStatusStyles.The switch statement covers all current
BranchStatusvalues, but TypeScript won't error if a new status is added to the union. Adding a default case with exhaustiveness check ensures future-proofing.🛡️ Suggested exhaustiveness check
function getStatusStyles(status: BranchStatus): { bgClass: string; textClass: string } { switch (status) { case 'active': return { bgClass: 'bg-primary/10', textClass: 'text-primary' }; case 'merged': return { bgClass: 'bg-secondary/10', textClass: 'text-secondary-foreground' }; case 'abandoned': return { bgClass: 'bg-muted', textClass: 'text-muted-foreground' }; + default: { + const _exhaustive: never = status; + return { bgClass: 'bg-muted', textClass: 'text-muted-foreground' }; + } } }
51-80: Same accessibility concern as CommitHistory.The
BranchItemhas the same keyboard accessibility issue asCommitItem. Consider addingrole="button",tabIndex, and keyboard event handlers for clickable items.♿ Suggested accessibility improvement
<div data-testid="branch-item" - className="flex items-center justify-between py-2 px-3 rounded-md hover:bg-muted/50 transition-colors cursor-pointer" + role="button" + tabIndex={onClick ? 0 : undefined} + className="flex items-center justify-between py-2 px-3 rounded-md hover:bg-muted/50 transition-colors cursor-pointer focus:outline-none focus:ring-2 focus:ring-ring" onClick={onClick} + onKeyDown={(e) => { + if (onClick && (e.key === 'Enter' || e.key === ' ')) { + e.preventDefault(); + onClick(); + } + }} >
13-13: UseGitBranchIconinstead ofGitCommitIconfor semantic clarity.The component displays git branches at line 61, but uses
GitCommitIconwhich is semantically incorrect. Hugeicons providesGitBranchIconwhich would better represent the branch concept. Update the import to use the appropriate icon.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
web-ui/__tests__/api/git.test.tsweb-ui/__tests__/components/git/BranchList.test.tsxweb-ui/__tests__/components/git/CommitHistory.test.tsxweb-ui/__tests__/components/git/GitBranchIndicator.test.tsxweb-ui/__tests__/components/git/GitSection.test.tsxweb-ui/__tests__/reducers/gitActions.test.tsweb-ui/__tests__/types/git.test.tsweb-ui/src/api/git.tsweb-ui/src/components/Dashboard.tsxweb-ui/src/components/git/BranchList.tsxweb-ui/src/components/git/CommitHistory.tsxweb-ui/src/components/git/GitBranchIndicator.tsxweb-ui/src/components/git/GitSection.tsxweb-ui/src/components/git/index.tsweb-ui/src/lib/websocketMessageMapper.tsweb-ui/src/reducers/agentReducer.tsweb-ui/src/types/agentState.tsweb-ui/src/types/git.tsweb-ui/src/types/index.tsweb-ui/test-utils/agentState.fixture.ts
🧰 Additional context used
📓 Path-based instructions (4)
web-ui/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web-ui/src/**/*.{ts,tsx}: Use TypeScript 5.3+ for frontend development
Use React 18 and Next.js 14 for frontend development
Use Tailwind CSS for styling with Nova design system template
Use shadcn/ui components from @/components/ui/ for UI elements
Use Hugeicons (@hugeicons/react) for all icons, never mix with lucide-react
Use Nova color palette variables (bg-card, text-foreground, etc.) instead of hardcoded color values
Use cn() utility for conditional CSS classes in React components
Use process.env.NEXT_PUBLIC_API_URL with fallback to http://localhost:8080 for API endpoint configuration
Include auth token as query parameter in WebSocket connections (?token=TOKEN)
Store auth tokens in localStorage with key 'auth_token' and include token in API requests via Authorization header
Files:
web-ui/src/components/git/GitSection.tsxweb-ui/src/components/git/BranchList.tsxweb-ui/src/types/agentState.tsweb-ui/src/components/git/index.tsweb-ui/src/types/index.tsweb-ui/src/api/git.tsweb-ui/src/lib/websocketMessageMapper.tsweb-ui/src/components/git/CommitHistory.tsxweb-ui/src/components/git/GitBranchIndicator.tsxweb-ui/src/types/git.tsweb-ui/src/components/Dashboard.tsxweb-ui/src/reducers/agentReducer.ts
web-ui/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web-ui/src/components/**/*.{ts,tsx}: Use React Context with useReducer for centralized state management in Dashboard
Wrap AgentStateProvider with ErrorBoundary component for graceful error handling
Use React.memo on Dashboard sub-components and useMemo for derived state to optimize performance
Files:
web-ui/src/components/git/GitSection.tsxweb-ui/src/components/git/BranchList.tsxweb-ui/src/components/git/index.tsweb-ui/src/components/git/CommitHistory.tsxweb-ui/src/components/git/GitBranchIndicator.tsxweb-ui/src/components/Dashboard.tsx
web-ui/src/lib/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Implement WebSocket automatic reconnection with exponential backoff (1s → 30s) and full state resync
Files:
web-ui/src/lib/websocketMessageMapper.ts
web-ui/src/reducers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use useReducer with 13+ action types for agent state management; implement timestamp conflict resolution with last-write-wins strategy
Files:
web-ui/src/reducers/agentReducer.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:37.203Z
Learning: Use feature branches from main with Conventional Commits format (feat/fix/docs scope): description
📚 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/git/CommitHistory.test.tsxweb-ui/__tests__/components/git/GitSection.test.tsxweb-ui/__tests__/components/git/BranchList.test.tsxweb-ui/__tests__/types/git.test.ts
📚 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/components/git/BranchList.tsxweb-ui/src/components/git/CommitHistory.tsx
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to web-ui/src/**/*.{ts,tsx} : Use React 18 and Next.js 14 for frontend development
Applied to files:
web-ui/src/components/git/BranchList.tsx
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to web-ui/src/reducers/**/*.{ts,tsx} : Use useReducer with 13+ action types for agent state management; implement timestamp conflict resolution with last-write-wins strategy
Applied to files:
web-ui/test-utils/agentState.fixture.tsweb-ui/__tests__/reducers/gitActions.test.tsweb-ui/src/types/agentState.tsweb-ui/src/types/index.tsweb-ui/src/lib/websocketMessageMapper.tsweb-ui/src/reducers/agentReducer.ts
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to web-ui/src/components/**/*.{ts,tsx} : Wrap AgentStateProvider with ErrorBoundary component for graceful error handling
Applied to files:
web-ui/src/types/agentState.ts
📚 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/**/*.{ts,tsx,js,jsx} : Use named exports instead of default exports in TypeScript/JavaScript
Applied to files:
web-ui/src/components/git/index.ts
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to web-ui/src/**/*.{ts,tsx} : Use shadcn/ui components from @/components/ui/ for UI elements
Applied to files:
web-ui/src/components/git/index.tsweb-ui/src/components/git/CommitHistory.tsx
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to web-ui/src/lib/**/*.{ts,tsx} : Implement WebSocket automatic reconnection with exponential backoff (1s → 30s) and full state resync
Applied to files:
web-ui/src/types/index.ts
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to web-ui/src/**/*.{ts,tsx} : Include auth token as query parameter in WebSocket connections (?token=TOKEN)
Applied to files:
web-ui/src/types/index.ts
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to web-ui/src/components/**/*.{ts,tsx} : Use React.memo on Dashboard sub-components and useMemo for derived state to optimize performance
Applied to files:
web-ui/src/components/git/CommitHistory.tsxweb-ui/src/components/Dashboard.tsx
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to web-ui/src/components/**/*.{ts,tsx} : Use React Context with useReducer for centralized state management in Dashboard
Applied to files:
web-ui/src/reducers/agentReducer.ts
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to tests/**/*.{ts,tsx,test.ts} : Never use test.skip() inside test logic; skip at describe level or use separate test projects for different states
Applied to files:
web-ui/__tests__/types/git.test.ts
🧬 Code graph analysis (11)
web-ui/__tests__/components/git/CommitHistory.test.tsx (2)
web-ui/src/types/git.ts (1)
GitCommit(39-46)web-ui/src/types/index.ts (1)
GitCommit(251-251)
web-ui/src/components/git/GitSection.tsx (3)
web-ui/src/components/git/index.ts (1)
GitSectionProps(17-17)web-ui/src/types/git.ts (3)
GitStatus(55-61)GitCommit(39-46)GitBranch(22-30)web-ui/src/api/git.ts (3)
getGitStatus(36-40)getCommits(64-85)getBranches(99-116)
web-ui/src/components/git/BranchList.tsx (3)
web-ui/src/components/git/index.ts (1)
BranchListProps(16-16)web-ui/src/types/git.ts (2)
GitBranch(22-30)BranchStatus(17-17)web-ui/src/types/index.ts (2)
GitBranch(250-250)BranchStatus(254-254)
web-ui/__tests__/components/git/GitBranchIndicator.test.tsx (2)
web-ui/src/types/git.ts (1)
GitStatus(55-61)web-ui/src/types/index.ts (1)
GitStatus(252-252)
web-ui/__tests__/api/git.test.ts (2)
web-ui/src/types/git.ts (3)
GitStatus(55-61)GitCommit(39-46)GitBranch(22-30)web-ui/src/api/git.ts (4)
getGitStatus(36-40)getCommits(64-85)getBranches(99-116)getBranch(126-134)
web-ui/src/types/agentState.ts (2)
web-ui/src/types/git.ts (4)
GitState(71-86)GitStatus(55-61)GitCommit(39-46)GitBranch(22-30)specs/005-project-schema-refactoring/contracts/agent-state-api.ts (1)
FullResyncAction(262-270)
web-ui/src/lib/websocketMessageMapper.ts (2)
web-ui/src/types/index.ts (1)
WebSocketMessage(126-204)specs/005-project-schema-refactoring/contracts/agent-state-api.ts (1)
WebSocketMessage(316-321)
web-ui/src/components/git/CommitHistory.tsx (2)
web-ui/src/components/git/index.ts (1)
CommitHistoryProps(15-15)web-ui/src/types/git.ts (1)
GitCommit(39-46)
web-ui/src/components/git/GitBranchIndicator.tsx (1)
web-ui/src/types/git.ts (1)
GitStatus(55-61)
web-ui/src/reducers/agentReducer.ts (2)
web-ui/src/types/git.ts (1)
INITIAL_GIT_STATE(113-119)web-ui/src/types/index.ts (1)
INITIAL_GIT_STATE(258-258)
web-ui/__tests__/types/git.test.ts (1)
web-ui/src/types/git.ts (5)
GitBranch(22-30)GitCommit(39-46)GitStatus(55-61)GitState(71-86)BranchStatus(17-17)
⏰ 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). (5)
- GitHub Check: Backend Unit Tests
- GitHub Check: Frontend Unit Tests
- GitHub Check: opencode-review
- GitHub Check: E2E Smoke Tests (Chromium)
- GitHub Check: Macroscope - Correctness Check
🔇 Additional comments (30)
web-ui/src/components/git/GitBranchIndicator.tsx (1)
1-104: LGTM! Well-structured component following best practices.The component correctly uses:
memo()for performance optimization as per coding guidelines- Hugeicons (
@hugeicons/react) for all icons- Nova color palette variables (
bg-muted,text-foreground,bg-destructive/10, etc.)- Proper loading/error/null state handling with appropriate test IDs
- Clear JSDoc documentation
The pluralization logic and tooltip implementation are correct.
web-ui/src/types/git.ts (1)
1-119: LGTM! Comprehensive and well-organized type definitions.The types are:
- Well-documented with JSDoc and section headers
- Properly aligned with backend API response models (as referenced in comments)
- Include sensible defaults in
INITIAL_GIT_STATE- Use appropriate optional fields (
merged_at,merge_commit,files_changed)web-ui/src/api/git.ts (1)
1-45: LGTM! Clean API client implementation.The API client correctly:
- Uses
process.env.NEXT_PUBLIC_API_URLwith localhost fallback per coding guidelines- Leverages
authFetchfor authenticated requests- Properly encodes branch names in URL paths with
encodeURIComponent- Uses
URLSearchParamsfor query string construction- Provides convenient wrapper functions (
getCurrentBranch,getRecentCommits)Also applies to: 86-161
web-ui/src/components/Dashboard.tsx (2)
40-40: LGTM! Clean import via barrel file.The import follows the established pattern of using barrel exports for component groups.
560-562: No action required.GitSectionis already wrapped withReact.memo(line 37 ofGitSection.tsx), meeting the coding guideline requirement for Dashboard sub-components.web-ui/test-utils/agentState.fixture.ts (1)
88-88: LGTM! Consider adding a helper for populated gitState in future.The
gitState: nulladditions are consistent across both factory functions. For more comprehensive test coverage of Git-related components, you might later want to add acreateMockGitState()helper or acreatePopulatedAgentStateWithGit()variant that returns a fully populatedgitState.Also applies to: 161-161
web-ui/__tests__/components/git/CommitHistory.test.tsx (1)
32-125: Well-structured test suite with comprehensive coverage.The test organization with nested
describeblocks for different aspects (rendering, loading, error, styling, limits, header) is clear and maintainable. Test coverage addresses the key functionality of the CommitHistory component.web-ui/__tests__/types/git.test.ts (1)
16-179: Type validation tests provide good contract verification.These tests serve as compile-time contracts ensuring the Git types remain stable. The coverage across all type variations (required fields, optional fields, nullable states) is thorough.
web-ui/src/components/git/GitSection.tsx (1)
37-151: Well-structured component with good SWR patterns.The component correctly:
- Uses
memofor performance optimization per coding guidelines- Implements stale-while-revalidate pattern (shows cached data during refresh)
- Uses Nova color palette variables (
bg-card,border-border,text-foreground, etc.)- Uses Hugeicons as required by coding guidelines
- Passes loading/error states to child components for granular feedback
web-ui/__tests__/components/git/BranchList.test.tsx (1)
39-134: Comprehensive test coverage for BranchList component.The test suite thoroughly covers:
- Empty and populated states
- All three branch status types with styling verification
- Loading and error states
- Filtering functionality
Test organization with nested
describeblocks is clean and maintainable.web-ui/__tests__/components/git/GitSection.test.tsx (1)
1-60: Test setup and fixtures look good.The test file is well-structured with clear mock fixtures that align with the
GitStatus,GitCommit, andGitBranchtypes. The SWR and API mocking approach is correct.web-ui/__tests__/components/git/GitBranchIndicator.test.tsx (1)
1-49: Well-structured tests for rendering and branch indicator.The test coverage for null status handling, branch name display, and icon presence is thorough and aligns with the
GitStatustype definition.web-ui/__tests__/reducers/gitActions.test.ts (3)
1-69: Excellent test fixtures and organization.The factory functions (
createMockGitStatus,createMockGitCommit,createMockGitBranch,createStateWithGitState) follow DRY principles and make the tests maintainable. Good use of partial overrides pattern.
199-256: Good FIFO limit test coverage for COMMIT_CREATED.The test at lines 219-239 correctly validates the 10-item limit with FIFO behavior, ensuring the oldest commit is dropped when a new one is added to a full list. This aligns with the reducer implementation.
348-374: GIT_ERROR tests are thorough.Tests cover both setting and clearing error state, and verify that
isLoadingis set tofalsewhen an error occurs. This matches the expected reducer behavior.web-ui/src/reducers/agentReducer.ts (3)
379-397: Git action handlers are well-implemented.The immutability pattern is correct, and the null coalescing with
INITIAL_GIT_STATEensures safe initialization. The handler correctly clears loading and error state when status is loaded.
383-397: Timestamp in payload is unused for conflict resolution.Per coding guidelines, the reducer should implement timestamp conflict resolution with last-write-wins strategy. The
GIT_STATUS_LOADEDpayload includes atimestampbut it's not used for LWW. If stale Git status updates can arrive out of order (e.g., from WebSocket events), consider adding conflict resolution similar toAGENT_UPDATED.If Git updates always come from SWR polling and are sequential, this may be acceptable. Otherwise, consider:
case 'GIT_STATUS_LOADED': { const { status, timestamp } = action.payload; const currentGitState = state.gitState ?? { ...INITIAL_GIT_STATE }; // Reject stale updates if (currentGitState.lastUpdated && currentGitState.lastUpdated > timestamp) { newState = state; break; } // ... rest of handler }
431-446: COMMIT_CREATED FIFO logic is correct.The
slice(0, 9)correctly keeps the 9 most recent existing commits, and prepending the new commit results in a maximum of 10 items. This matches the test expectations.web-ui/src/components/git/CommitHistory.tsx (2)
94-105: Good use of memoization for performance.The component correctly uses
React.memoanduseMemofor thedisplayedCommitsderivation, aligning with the coding guidelines for Dashboard sub-components. The slicing logic is clean and efficient.
161-185: Well-structured component with proper state handling.The component handles loading, error, and empty states appropriately with distinct visuals and test IDs. The use of Nova design system classes (
bg-card,text-foreground,text-muted-foreground, etc.) follows coding guidelines.web-ui/src/components/git/BranchList.tsx (2)
87-98: Good use of memoization and filtering logic.The component correctly applies
React.memoand usesuseMemofor filtered branches. The filtering logic is clean and handles the optionalfilterStatusprop appropriately.
154-173: Consistent structure with CommitHistory.The component follows the same pattern as
CommitHistorywith proper state handling, Nova design system classes, and test IDs. Good consistency across Git visualization components.web-ui/src/types/index.ts (2)
123-124: WebSocket message type addition looks correct.The
branch_createdmessage type is properly added to the union, aligning with the WebSocket handlers mentioned in the PR summary for real-time Git events.
248-258: Clean type re-exports for Git visualization.The Git types and utilities are properly re-exported from
./git, maintaining a clean public API surface. All exports (GitBranch, GitCommit, GitStatus, GitState, BranchStatus, BranchListResponse, CommitListResponse, INITIAL_GIT_STATE, isBranchActive, isBranchMerged) exist in the source module and are correctly re-exported. This follows good modular organization practices and enables consumers to import from the central types index.web-ui/src/types/agentState.ts (6)
12-12: Proper type imports for Git integration.The imports correctly pull in the Git types needed for the state and action definitions.
252-252: Good nullable typing for initial state.Using
GitState | nullallows the state to represent "not yet loaded" vs "loaded but empty", which is appropriate for the initialization flow before the first API fetch.
401-429: Git load actions follow established patterns.The
GitStatusLoadedAction,GitCommitsLoadedAction, andGitBranchesLoadedActioninterfaces correctly include timestamps for conflict resolution, consistent with the existing action patterns (e.g.,TaskStatusChangedAction,AgentUpdatedAction). Based on learnings, this aligns with the "last-write-wins" timestamp conflict resolution strategy.
431-452: WebSocket event actions are well-structured.
CommitCreatedActionandBranchCreatedActionproperly model real-time events with timestamps. The optionaltaskIdinCommitCreatedActionis a good design choice for linking commits to tasks when applicable.
454-468: Simple loading/error actions are appropriate.
GitLoadingActionandGitErrorActionare straightforward boolean/string payloads without timestamps, which is correct since these represent transient UI states rather than domain events that need conflict resolution.
486-494: AgentAction union properly extended.All seven Git actions are correctly added to the discriminated union. The comment annotation helps future maintainers understand the grouping.
- Add null checks in WebSocket commit_created/branch_created handlers - Add defensive checks in reducer for gitState arrays - Remove invalid ARIA role from CommitHistory time element - Fix SWR cache key to include maxCommits for proper invalidation - Update JSDoc for getBranch to clarify encoding behavior - Add visibility assertion in GitBranchIndicator test - Add 10 new tests for message validation
| ); | ||
| } | ||
|
|
||
| // Error state |
There was a problem hiding this comment.
CommitHistory.tsx renders CommitItem, but it’s not imported or defined. Consider importing CommitItem (or defining it here) to avoid a runtime ReferenceError.
🚀 Want me to fix this? Reply ex: "fix it for me".
Review SummaryThis is a well-structured implementation with comprehensive test coverage (74 new tests) and clean architecture. The code follows project conventions for React.memo, SWR, and the Nova design system. Critical Issue: WebSocket HandlerLocation: The handler reads fields from the wrong location in the WebSocket message. Backend sends with top-level fields, but frontend tries to access : Backend format (from ): Frontend handler (line 284-297): Impact: All messages are skipped (validation fails), so branch tracking doesn't work via WebSocket. Fix: TypeScript Type Definition MissingLocation: The interface is missing fields for messages. Add: ✅ Strengths
Minor Suggestions
RecommendationRequest changes required for the critical handler bug. This will completely break branch WebSocket updates. Overall: Excellent implementation with a critical bug in WebSocket message parsing. Fix the field access and type definition, then approve. |
Review CompleteI've reviewed PR #283 and posted a comprehensive comment at: #283 (comment) Key FindingsCritical Bug: The Missing Type Definition: Strengths:
Minor Suggestions:
The implementation is excellent overall but requires the critical WebSocket handler fix to merge. |
Summary
Test plan
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.