refactor(ui): migrate from emojis to Hugeicons for consistent icon system#286
Conversation
…stem Replace all emoji characters with Hugeicons React components across the web-ui codebase. This ensures consistency with the Nova design system and provides proper accessibility attributes (aria-hidden) for icons. Key changes: - Replace emoji literals (🔒,⚠️ , ✅, etc.) with Hugeicons components - Rename utility files to .tsx for JSX return types - Add getCategoryIcon() and getStatusIcon() functions for type-safe icons - Update all test files with comprehensive Hugeicons mocks - Update jest.setup.js with global icon mocks Components updated: AgentCard, BlockerBadge, ErrorBoundary, TaskTreeView, QualityGateStatus, ReviewFindings, ReviewSummary, DiscoveryProgress, and more Test results: 1805 tests passing, build successful
|
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. WalkthroughMigrates emoji-based UI icons to Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Migrate UI and tests from emoji icons to Hugeicons across agent, task, review, quality gate, checkpoint, and dashboard componentsReplace emoji-based badges and status indicators with 📍Where to StartStart with the icon utilities that drive component rendering: Macroscope summarized 6c8b460. |
There was a problem hiding this comment.
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 (5)
web-ui/src/components/checkpoints/CheckpointRestore.tsx (1)
117-132: Duplicate warning icons - incomplete migration?The warning block now contains two warning indicators:
- An inline SVG icon (lines 117-127)
- The new
Alert02IconHugeicon (line 130)This appears to be an incomplete migration. Based on the PR objective to replace emojis with Hugeicons for a consistent icon system, the inline SVG should likely be replaced with a Hugeicon rather than having both.
🛠️ Suggested fix: Replace inline SVG with Hugeicon
<div className="flex items-start"> - <svg - className="h-5 w-5 text-destructive mr-2 mt-0.5" - fill="none" - strokeLinecap="round" - strokeLinejoin="round" - strokeWidth="2" - viewBox="0 0 24 24" - stroke="currentColor" - > - <path d="M12 9v2m0 4h.01m-6.938 4h13.856c1.54 0 2.502-1.667 1.732-3L13.732 4c-.77-1.333-2.694-1.333-3.464 0L3.34 16c-.77 1.333.192 3 1.732 3z"></path> - </svg> + <Alert02Icon className="h-5 w-5 text-destructive mr-2 mt-0.5" aria-hidden="true" /> <div> - <p className="text-sm font-medium text-destructive flex items-center gap-1"> - <Alert02Icon className="h-4 w-4" aria-hidden="true" /> - <span>Warning: Destructive Operation</span> - </p> + <p className="text-sm font-medium text-destructive">Warning: Destructive Operation</p> <p className="text-sm text-muted-foreground mt-1">web-ui/__tests__/components/review/ReviewFindingsList.test.tsx (2)
100-106: Stale CSS selector in test assertion.The test queries for
.bg-blue-50but the component uses.bg-mutedfor the suggestion box (seeReviewFindingsList.tsxline 117). While the negative assertion still passes, the selector should be updated for maintainability.Proposed fix
it('does not render suggestion section when suggestion is not provided', () => { const findingWithoutSuggestion = { ...mockFinding, suggestion: undefined }; const { container } = render(<ReviewFindingsList findings={[findingWithoutSuggestion]} />); - const suggestionBox = container.querySelector('.bg-blue-50'); + const suggestionBox = container.querySelector('.bg-muted.flex.items-start'); expect(suggestionBox).not.toBeInTheDocument(); });
392-406: Same stale selector issue in edge case test.Similar to line 104, this test uses
.bg-blue-50which doesn't match the component's actual class.bg-muted.Proposed fix
it('handles empty suggestion string', () => { const finding: ReviewFinding = { file_path: 'test.ts', line_number: 1, category: 'security', severity: 'low', message: 'Test', suggestion: '', }; const { container } = render(<ReviewFindingsList findings={[finding]} />); // Empty string is falsy, so suggestion box should not render - const suggestionBox = container.querySelector('.bg-blue-50'); + const suggestionBox = container.querySelector('.bg-muted.flex.items-start'); expect(suggestionBox).not.toBeInTheDocument(); });web-ui/__tests__/components/AgentList.test.tsx (1)
256-260: Misleading comment about data-testid source.The comment on line 258 suggests Hugeicons adds
data-testidautomatically, but it's actually the test mock that adds this attribute. This could confuse future maintainers.Suggested fix
await waitFor(() => { - // Check for BotIcon (Hugeicons adds data-testid automatically) + // Check for BotIcon (mocked with data-testid in jest.mock) expect(screen.getByTestId('bot-icon')).toBeInTheDocument(); });web-ui/src/components/quality-gates/QualityGateStatus.tsx (1)
170-173: Replacetext-2xlwith appropriate Hugeicons sizing classes.The
getStatusIcon()function returns Hugeicons components already sized withh-5 w-5classes. Wrapping them in<span className="text-2xl">is incorrect—text-2xlis a font-size class for text content, not icon sizing. Useh-*andw-*classes instead (e.g.,h-6 w-6) or remove the wrapper if the current 20px size is appropriate.
🧹 Nitpick comments (9)
web-ui/src/components/TaskReview.tsx (1)
459-462: Clean icon implementation with proper accessibility.The migration from emoji to
CheckmarkCircle01Iconlooks good:
aria-hidden="true"correctly marks the decorative iconinline-flex items-center gap-2provides proper alignment- Nested
<span>for the text maintains semantic structureConsider adding a
data-testidattribute for test consistency with other migrated components in this PR (e.g.,data-testid="checkmark-icon"), though this is optional since the icon is decorative and tests can assert on the text content.Optional: Add test ID for consistency
- <CheckmarkCircle01Icon className="h-5 w-5 text-secondary" aria-hidden="true" /> + <CheckmarkCircle01Icon className="h-5 w-5 text-secondary" aria-hidden="true" data-testid="checkmark-icon" />web-ui/src/components/review/ReviewResultsPanel.tsx (1)
107-119: Consider replacing inline SVG with Hugeicons for consistency.The close button uses an inline SVG while the rest of the component now uses Hugeicons. Per coding guidelines, all icons should use
@hugeicons/reactto avoid mixing icon systems.♻️ Suggested refactor
-import { Alert02Icon, FileEditIcon } from '@hugeicons/react'; +import { Alert02Icon, FileEditIcon, Cancel01Icon } from '@hugeicons/react';Then replace the inline SVG:
<button onClick={onClose} className="text-muted-foreground hover:text-foreground transition-colors" aria-label="Close" > - <svg - className="w-6 h-6" - fill="none" - stroke="currentColor" - viewBox="0 0 24 24" - > - <path - strokeLinecap="round" - strokeLinejoin="round" - strokeWidth={2} - d="M6 18L18 6M6 6l12 12" - /> - </svg> + <Cancel01Icon className="w-6 h-6" aria-hidden="true" /> </button>web-ui/__tests__/components/ReviewSummary.test.tsx (1)
23-43: Consider removing redundant local mock.This local
@hugeicons/reactmock duplicates the global mock injest.setup.js, which already provides the same factory pattern for all icons used in the codebase. The local mock will override the global one, so it's not incorrect, but it adds maintenance overhead.If this test file needs specific icons not in the global mock, adding them to
jest.setup.jswould be more maintainable. Otherwise, this local mock can be removed.♻️ Optional: Remove local mock if global coverage is sufficient
-// Mock Hugeicons -jest.mock('@hugeicons/react', () => { - const React = require('react'); - const createMockIcon = (name: string) => { - const Icon = ({ className }: { className?: string }) => ( - <svg data-testid={name} className={className} aria-hidden="true" /> - ); - Icon.displayName = name; - return Icon; - }; - return { - Alert02Icon: createMockIcon('Alert02Icon'), - CheckmarkCircle01Icon: createMockIcon('CheckmarkCircle01Icon'), - Idea01Icon: createMockIcon('Idea01Icon'), - LockIcon: createMockIcon('LockIcon'), - FlashIcon: createMockIcon('FlashIcon'), - SparklesIcon: createMockIcon('SparklesIcon'), - Settings01Icon: createMockIcon('Settings01Icon'), - PaintBrush01Icon: createMockIcon('PaintBrush01Icon'), - }; -}); -All these icons are already mocked globally in
jest.setup.js.web-ui/src/components/checkpoints/CheckpointRestore.tsx (1)
94-104: Consider migrating success icon to Hugeicons for consistency.The success message block uses an inline SVG checkmark icon. For consistency with the Hugeicons migration, consider replacing it with
CheckmarkCircle01Iconor similar from@hugeicons/react.web-ui/src/components/reviews/ReviewFindings.tsx (1)
145-148: Consider using a success-indicating color for the checkmark icon.The
text-secondaryclass may not clearly convey the positive "all good" state. Consider usingtext-green-500or a semantic success color variable if available in the Nova design system to better indicate the positive outcome.💡 Suggested change
- <CheckmarkCircle01Icon className="h-5 w-5 text-secondary inline-block" aria-hidden="true" /> + <CheckmarkCircle01Icon className="h-5 w-5 text-green-500 inline-block" aria-hidden="true" />web-ui/__tests__/components/DiscoveryProgress.testutils.tsx (1)
69-79: Standardize data-testid naming conventions across icon mocks in the test suite.The test suite uses at least three inconsistent patterns for icon testIds: semantic kebab-case names (e.g.,
'cancel-icon','checkmark-icon'inDiscoveryProgress.testutils.tsxandAgentCard.test.tsx), component names in PascalCase (e.g.,'Alert02Icon','CheckmarkCircle01Icon'inQualityGateStatus.test.tsx,ErrorBoundary.test.tsx, andBlockerBadge.test.tsx), and icon-prefixed patterns (e.g.,'icon-checkmark'inPRList.test.tsx). Adopt one consistent convention across all mocks to reduce maintenance friction and improve test readability.web-ui/__tests__/components/BlockerPanel.test.tsx (1)
23-37: Inconsistent test ID naming pattern across test files.This mock uses the icon component name as the
data-testid(e.g.,CheckmarkCircle01Icon), while other test files in this PR use kebab-case names (e.g.,checkmark-iconin Dashboard.test.tsx). Consider aligning to a consistent convention across all test files to avoid confusion when writing assertions.♻️ Suggested fix to align with kebab-case convention
jest.mock('@hugeicons/react', () => { const React = require('react'); - const createMockIcon = (name: string) => { + const createMockIcon = (name: string, testId: string) => { const Icon = ({ className }: { className?: string }) => ( - <svg data-testid={name} className={className} aria-hidden="true" /> + <svg data-testid={testId} className={className} aria-hidden="true" /> ); Icon.displayName = name; return Icon; }; return { - CheckmarkCircle01Icon: createMockIcon('CheckmarkCircle01Icon'), - BotIcon: createMockIcon('BotIcon'), + CheckmarkCircle01Icon: createMockIcon('CheckmarkCircle01Icon', 'checkmark-icon'), + BotIcon: createMockIcon('BotIcon', 'bot-icon'), }; });Then update assertions at lines 51 and 271:
-expect(screen.getByTestId('CheckmarkCircle01Icon')).toBeInTheDocument(); +expect(screen.getByTestId('checkmark-icon')).toBeInTheDocument();-expect(screen.getByTestId('BotIcon')).toBeInTheDocument(); +expect(screen.getByTestId('bot-icon')).toBeInTheDocument();web-ui/src/lib/qualityGateUtils.tsx (1)
110-123: RedundanticonPropsspread—classNameis always overwritten.The
iconPropsobject includesclassName: 'h-5 w-5', but each return statement overwrites it with a newclassNamethat includes color classes. Onlyaria-hiddenis effectively preserved from the spread.Consider extracting just the shared accessibility props to make the intent clearer:
♻️ Suggested refactor
export function getStatusIcon(status: QualityGateStatusValue): JSX.Element { - const iconProps = { className: 'h-5 w-5', 'aria-hidden': true as const }; + const ariaProps = { 'aria-hidden': true as const }; switch (status) { case 'passed': - return <CheckmarkCircle01Icon {...iconProps} className="h-5 w-5 text-secondary" />; + return <CheckmarkCircle01Icon {...ariaProps} className="h-5 w-5 text-secondary" />; case 'failed': - return <Cancel01Icon {...iconProps} className="h-5 w-5 text-destructive" />; + return <Cancel01Icon {...ariaProps} className="h-5 w-5 text-destructive" />; case 'running': - return <Loading03Icon {...iconProps} className="h-5 w-5 text-primary animate-spin" />; + return <Loading03Icon {...ariaProps} className="h-5 w-5 text-primary animate-spin" />; case 'pending': - return <PauseIcon {...iconProps} className="h-5 w-5 text-muted-foreground" />; + return <PauseIcon {...ariaProps} className="h-5 w-5 text-muted-foreground" />; default: - return <HelpCircleIcon {...iconProps} className="h-5 w-5 text-muted-foreground" />; + return <HelpCircleIcon {...ariaProps} className="h-5 w-5 text-muted-foreground" />; } }web-ui/__tests__/lib/qualityGateUtils.test.tsx (1)
286-290: Consider adding a comment clarifying whitespace behavior is intentional.The test documents that
' tests 'falls through to the default case, which may surprise future maintainers expecting automatic trimming. A brief comment would make this design decision explicit.📝 Suggested clarification
- it('should handle whitespace in inputs', () => { + it('should handle whitespace in inputs (no trimming, falls through to default)', () => { render(<div data-testid="icon-container">{getGateIcon(' tests ')}</div>); + // Whitespace-padded inputs don't match exact case values, so default icon is returned expect(screen.getByTestId('Settings01Icon')).toBeInTheDocument(); expect(getGateName(' tests ')).toBe(' tests '); });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
web-ui/__tests__/components/AgentCard.test.tsxweb-ui/__tests__/components/AgentList.test.tsxweb-ui/__tests__/components/BlockerBadge.test.tsxweb-ui/__tests__/components/BlockerPanel.test.tsxweb-ui/__tests__/components/Dashboard.test.tsxweb-ui/__tests__/components/DiscoveryProgress.test.tsxweb-ui/__tests__/components/DiscoveryProgress.testutils.tsxweb-ui/__tests__/components/ErrorBoundary.test.tsxweb-ui/__tests__/components/QualityGateStatus.test.tsxweb-ui/__tests__/components/ReviewFindings.test.tsxweb-ui/__tests__/components/ReviewSummary.test.tsxweb-ui/__tests__/components/TaskTreeView.test.tsxweb-ui/__tests__/components/checkpoints/CheckpointRestore.test.tsxweb-ui/__tests__/components/quality-gates/QualityGatesPanelFallback.test.tsxweb-ui/__tests__/components/review/ReviewFindingsList.test.tsxweb-ui/__tests__/components/review/ReviewResultsPanel.test.tsxweb-ui/__tests__/integration/dashboard-realtime-updates.test.tsxweb-ui/__tests__/integration/discovery-answer-flow.test.tsxweb-ui/__tests__/lib/qualityGateUtils.test.tsxweb-ui/jest.setup.jsweb-ui/src/components/AgentAssignmentCard.tsxweb-ui/src/components/AgentCard.tsxweb-ui/src/components/AgentList.tsxweb-ui/src/components/BlockerBadge.tsxweb-ui/src/components/BlockerPanel.tsxweb-ui/src/components/DiscoveryProgress.tsxweb-ui/src/components/ErrorBoundary.tsxweb-ui/src/components/ProjectCreationForm.tsxweb-ui/src/components/TaskList.tsxweb-ui/src/components/TaskReview.tsxweb-ui/src/components/TaskTreeView.tsxweb-ui/src/components/checkpoints/CheckpointRestore.tsxweb-ui/src/components/pr/PRList.tsxweb-ui/src/components/quality-gates/QualityGateStatus.tsxweb-ui/src/components/quality-gates/QualityGatesPanel.tsxweb-ui/src/components/quality-gates/QualityGatesPanelFallback.tsxweb-ui/src/components/review/ReviewFindingsList.tsxweb-ui/src/components/review/ReviewResultsPanel.tsxweb-ui/src/components/reviews/ReviewFindings.tsxweb-ui/src/components/reviews/ReviewSummary.tsxweb-ui/src/lib/qualityGateUtils.tsxweb-ui/src/types/reviews.tsx
🧰 Additional context used
📓 Path-based instructions (3)
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/pr/PRList.tsxweb-ui/src/components/quality-gates/QualityGatesPanelFallback.tsxweb-ui/src/components/TaskTreeView.tsxweb-ui/src/components/TaskList.tsxweb-ui/src/components/review/ReviewResultsPanel.tsxweb-ui/src/types/reviews.tsxweb-ui/src/components/BlockerPanel.tsxweb-ui/src/components/ErrorBoundary.tsxweb-ui/src/components/AgentList.tsxweb-ui/src/components/DiscoveryProgress.tsxweb-ui/src/components/quality-gates/QualityGateStatus.tsxweb-ui/src/components/ProjectCreationForm.tsxweb-ui/src/lib/qualityGateUtils.tsxweb-ui/src/components/TaskReview.tsxweb-ui/src/components/AgentCard.tsxweb-ui/src/components/checkpoints/CheckpointRestore.tsxweb-ui/src/components/review/ReviewFindingsList.tsxweb-ui/src/components/BlockerBadge.tsxweb-ui/src/components/quality-gates/QualityGatesPanel.tsxweb-ui/src/components/reviews/ReviewSummary.tsxweb-ui/src/components/AgentAssignmentCard.tsxweb-ui/src/components/reviews/ReviewFindings.tsx
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/pr/PRList.tsxweb-ui/src/components/quality-gates/QualityGatesPanelFallback.tsxweb-ui/src/components/TaskTreeView.tsxweb-ui/src/components/TaskList.tsxweb-ui/src/components/review/ReviewResultsPanel.tsxweb-ui/src/components/BlockerPanel.tsxweb-ui/src/components/ErrorBoundary.tsxweb-ui/src/components/AgentList.tsxweb-ui/src/components/DiscoveryProgress.tsxweb-ui/src/components/quality-gates/QualityGateStatus.tsxweb-ui/src/components/ProjectCreationForm.tsxweb-ui/src/components/TaskReview.tsxweb-ui/src/components/AgentCard.tsxweb-ui/src/components/checkpoints/CheckpointRestore.tsxweb-ui/src/components/review/ReviewFindingsList.tsxweb-ui/src/components/BlockerBadge.tsxweb-ui/src/components/quality-gates/QualityGatesPanel.tsxweb-ui/src/components/reviews/ReviewSummary.tsxweb-ui/src/components/AgentAssignmentCard.tsxweb-ui/src/components/reviews/ReviewFindings.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/qualityGateUtils.tsx
🧠 Learnings (16)
📓 Common learnings
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 Hugeicons (hugeicons/react) for all icons, never mix with lucide-react
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
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
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 PascalCase for React component names
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
📚 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 Hugeicons (hugeicons/react) for all icons, never mix with lucide-react
Applied to files:
web-ui/src/components/quality-gates/QualityGatesPanelFallback.tsxweb-ui/src/components/TaskTreeView.tsxweb-ui/__tests__/components/checkpoints/CheckpointRestore.test.tsxweb-ui/__tests__/integration/discovery-answer-flow.test.tsxweb-ui/__tests__/components/BlockerPanel.test.tsxweb-ui/src/components/TaskList.tsxweb-ui/__tests__/components/AgentCard.test.tsxweb-ui/__tests__/components/ReviewSummary.test.tsxweb-ui/__tests__/components/AgentList.test.tsxweb-ui/__tests__/components/BlockerBadge.test.tsxweb-ui/src/components/review/ReviewResultsPanel.tsxweb-ui/src/types/reviews.tsxweb-ui/__tests__/components/TaskTreeView.test.tsxweb-ui/__tests__/components/review/ReviewResultsPanel.test.tsxweb-ui/jest.setup.jsweb-ui/src/components/BlockerPanel.tsxweb-ui/src/components/ErrorBoundary.tsxweb-ui/__tests__/components/DiscoveryProgress.test.tsxweb-ui/__tests__/components/QualityGateStatus.test.tsxweb-ui/src/components/AgentList.tsxweb-ui/src/components/DiscoveryProgress.tsxweb-ui/src/components/quality-gates/QualityGateStatus.tsxweb-ui/src/components/ProjectCreationForm.tsxweb-ui/src/lib/qualityGateUtils.tsxweb-ui/src/components/TaskReview.tsxweb-ui/src/components/AgentCard.tsxweb-ui/src/components/checkpoints/CheckpointRestore.tsxweb-ui/src/components/review/ReviewFindingsList.tsxweb-ui/__tests__/components/Dashboard.test.tsxweb-ui/__tests__/components/ReviewFindings.test.tsxweb-ui/__tests__/integration/dashboard-realtime-updates.test.tsxweb-ui/src/components/BlockerBadge.tsxweb-ui/src/components/quality-gates/QualityGatesPanel.tsxweb-ui/src/components/reviews/ReviewSummary.tsxweb-ui/__tests__/components/ErrorBoundary.test.tsxweb-ui/__tests__/components/quality-gates/QualityGatesPanelFallback.test.tsxweb-ui/src/components/AgentAssignmentCard.tsxweb-ui/__tests__/lib/qualityGateUtils.test.tsxweb-ui/src/components/reviews/ReviewFindings.tsxweb-ui/__tests__/components/DiscoveryProgress.testutils.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/components/TaskTreeView.tsxweb-ui/src/lib/qualityGateUtils.tsxweb-ui/src/components/AgentCard.tsxweb-ui/src/components/checkpoints/CheckpointRestore.tsxweb-ui/src/components/BlockerBadge.tsxweb-ui/src/components/AgentAssignmentCard.tsxweb-ui/__tests__/components/DiscoveryProgress.testutils.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.memo on Dashboard sub-components and useMemo for derived state to optimize performance
Applied to files:
web-ui/src/components/TaskTreeView.tsxweb-ui/src/components/AgentCard.tsxweb-ui/__tests__/components/Dashboard.test.tsxweb-ui/__tests__/integration/dashboard-realtime-updates.test.tsx
📚 Learning: 2025-11-25T19:08:37.203Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:37.203Z
Learning: Applies to docs/web-ui/**/__tests__/**/*.test.{ts,tsx} : Create JavaScript test files colocated or in __tests__/ as *.test.ts
Applied to files:
web-ui/__tests__/components/checkpoints/CheckpointRestore.test.tsxweb-ui/__tests__/components/ReviewSummary.test.tsxweb-ui/__tests__/components/AgentList.test.tsxweb-ui/__tests__/components/BlockerBadge.test.tsxweb-ui/__tests__/components/TaskTreeView.test.tsxweb-ui/__tests__/components/review/ReviewResultsPanel.test.tsxweb-ui/__tests__/components/DiscoveryProgress.test.tsxweb-ui/__tests__/components/QualityGateStatus.test.tsxweb-ui/__tests__/components/Dashboard.test.tsxweb-ui/__tests__/components/ReviewFindings.test.tsxweb-ui/__tests__/integration/dashboard-realtime-updates.test.tsxweb-ui/__tests__/components/quality-gates/QualityGatesPanelFallback.test.tsxweb-ui/__tests__/lib/qualityGateUtils.test.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 tests/e2e/**/*.{ts,test.ts} : Assert UI elements existence with expect(element).toBeVisible() - fail if missing, never silently pass
Applied to files:
web-ui/__tests__/components/checkpoints/CheckpointRestore.test.tsxweb-ui/__tests__/components/BlockerPanel.test.tsxweb-ui/__tests__/components/AgentCard.test.tsxweb-ui/__tests__/components/ReviewSummary.test.tsxweb-ui/__tests__/components/BlockerBadge.test.tsxweb-ui/__tests__/components/TaskTreeView.test.tsxweb-ui/__tests__/components/review/ReviewResultsPanel.test.tsxweb-ui/__tests__/components/QualityGateStatus.test.tsxweb-ui/__tests__/components/ReviewFindings.test.tsxweb-ui/__tests__/components/review/ReviewFindingsList.test.tsxweb-ui/__tests__/components/ErrorBoundary.test.tsxweb-ui/__tests__/components/quality-gates/QualityGatesPanelFallback.test.tsxweb-ui/__tests__/lib/qualityGateUtils.test.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 shadcn/ui components from `@/components/ui/` for UI elements
Applied to files:
web-ui/src/components/TaskList.tsxweb-ui/src/components/review/ReviewResultsPanel.tsxweb-ui/src/components/ErrorBoundary.tsxweb-ui/__tests__/components/DiscoveryProgress.test.tsxweb-ui/src/components/AgentList.tsxweb-ui/src/components/DiscoveryProgress.tsxweb-ui/src/components/ProjectCreationForm.tsxweb-ui/src/components/checkpoints/CheckpointRestore.tsxweb-ui/src/components/review/ReviewFindingsList.tsxweb-ui/__tests__/integration/dashboard-realtime-updates.test.tsxweb-ui/src/components/quality-gates/QualityGatesPanel.tsxweb-ui/__tests__/components/ErrorBoundary.test.tsxweb-ui/src/components/AgentAssignmentCard.tsxweb-ui/src/components/reviews/ReviewFindings.tsxweb-ui/__tests__/components/DiscoveryProgress.testutils.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} : Wrap AgentStateProvider with ErrorBoundary component for graceful error handling
Applied to files:
web-ui/src/components/TaskList.tsxweb-ui/src/components/ErrorBoundary.tsxweb-ui/src/components/AgentList.tsxweb-ui/src/components/ProjectCreationForm.tsxweb-ui/__tests__/components/ErrorBoundary.test.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 tests/e2e/**/*.{ts,test.ts} : Use TEST_PROJECT_IDS.ACTIVE for tests requiring pre-seeded active phase with agents
Applied to files:
web-ui/__tests__/components/AgentCard.test.tsxweb-ui/__tests__/components/AgentList.test.tsxweb-ui/__tests__/components/DiscoveryProgress.test.tsx
📚 Learning: 2025-11-25T19:08:37.203Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:37.203Z
Learning: Applies to docs/web-ui/src/**/*.{ts,tsx} : Use SWR for server state management and useState for local state in React
Applied to files:
web-ui/__tests__/components/AgentList.test.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 tests/e2e/**/*.{ts,test.ts} : Use TEST_PROJECT_IDS.PLANNING for tests requiring pre-seeded planning phase tasks
Applied to files:
web-ui/__tests__/components/TaskTreeView.test.tsxweb-ui/__tests__/components/DiscoveryProgress.test.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/src/components/AgentList.tsxweb-ui/src/components/AgentCard.tsxweb-ui/src/components/AgentAssignmentCard.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/components/TaskReview.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 Nova color palette variables (bg-card, text-foreground, etc.) instead of hardcoded color values
Applied to files:
web-ui/src/components/AgentCard.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/components/checkpoints/CheckpointRestore.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 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__/lib/qualityGateUtils.test.tsx
🧬 Code graph analysis (12)
web-ui/__tests__/components/ReviewSummary.test.tsx (2)
web-ui/__mocks__/@hugeicons/react.js (1)
React(6-6)web-ui/src/components/reviews/ReviewSummary.tsx (1)
ReviewSummary(153-442)
web-ui/src/types/reviews.tsx (1)
codeframe/core/models.py (1)
ReviewCategory(148-155)
web-ui/src/lib/qualityGateUtils.tsx (1)
web-ui/src/types/qualityGates.ts (2)
GateTypeE2E(74-74)QualityGateStatusValue(22-22)
web-ui/src/components/AgentCard.tsx (2)
web-ui/jest.setup.js (1)
React(41-41)web-ui/__mocks__/@hugeicons/react.js (1)
React(6-6)
web-ui/src/components/review/ReviewFindingsList.tsx (1)
web-ui/src/types/reviews.tsx (1)
getCategoryIcon(129-145)
web-ui/src/components/BlockerBadge.tsx (2)
web-ui/jest.setup.js (1)
React(41-41)web-ui/__mocks__/@hugeicons/react.js (1)
React(6-6)
web-ui/__tests__/components/review/ReviewFindingsList.test.tsx (2)
web-ui/__mocks__/@hugeicons/react.js (1)
React(6-6)web-ui/src/components/review/ReviewFindingsList.tsx (1)
ReviewFindingsList(66-129)
web-ui/src/components/reviews/ReviewSummary.tsx (2)
web-ui/src/types/reviews.tsx (1)
getCategoryIcon(129-145)codeframe/core/models.py (1)
ReviewCategory(148-155)
web-ui/__tests__/components/ErrorBoundary.test.tsx (2)
web-ui/jest.setup.js (1)
React(41-41)web-ui/__mocks__/@hugeicons/react.js (1)
React(6-6)
web-ui/src/components/AgentAssignmentCard.tsx (2)
web-ui/jest.setup.js (1)
React(41-41)web-ui/__mocks__/@hugeicons/react.js (1)
React(6-6)
web-ui/__tests__/lib/qualityGateUtils.test.tsx (2)
web-ui/src/lib/qualityGateUtils.tsx (3)
getGateIcon(29-48)getStatusIcon(110-124)getGateName(58-76)web-ui/src/types/qualityGates.ts (1)
GateTypeE2E(74-74)
web-ui/src/components/reviews/ReviewFindings.tsx (1)
web-ui/src/types/reviews.tsx (1)
getCategoryIcon(129-145)
⏰ 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: opencode-review
🔇 Additional comments (92)
web-ui/src/components/DiscoveryProgress.tsx (2)
16-16: LGTM! Clean import addition following the established pattern.The
Idea01Iconis correctly added to the existing Hugeicons import, maintaining consistency with other icon imports in the file. This aligns with the coding guidelines requiring use of@hugeicons/reactfor all icons.
788-791: LGTM! Well-implemented icon integration with proper accessibility.The keyboard shortcut hint correctly:
- Uses flex layout for icon/text alignment
- Marks the decorative icon with
aria-hidden="true"so screen readers focus on the text content- Maintains semantic
<kbd>element for the keyboard shortcut- Uses consistent Tailwind utility classes
web-ui/src/components/ProjectCreationForm.tsx (2)
15-15: LGTM!The import correctly uses
@hugeicons/reactas required by the coding guidelines. Based on learnings, Hugeicons should be used for all icons without mixing with lucide-react, and this import follows that pattern.
230-237: Well-implemented icon migration with proper accessibility.The error display correctly:
- Uses
aria-hidden="true"since the icon is decorative and "Error:" text provides meaning- Applies
flex-shrink-0to prevent icon distortion on long error messages- Maintains consistent sizing (
h-4 w-4) with proper flex alignmentThis aligns with the PR's goal of consistent Hugeicons usage across the codebase.
web-ui/src/components/pr/PRList.tsx (2)
283-290: LGTM!The
data-testid="create-pr-empty-state"attribute is a clean addition that improves testability without affecting functionality. The naming clearly distinguishes this button from the header's "Create PR" button.
428-435: LGTM!The
data-testid="create-pr-button"attribute enables reliable test selection for the header's primary action button. Consistent with the PR's objective of aligning test assertions with icon-based UI via test IDs.web-ui/src/components/TaskReview.tsx (1)
13-13: LGTM!Import aligns with the Hugeicons convention per coding guidelines. Based on learnings, all icons should use
@hugeicons/react.web-ui/src/components/BlockerPanel.tsx (1)
12-12: LGTM! Icon migration implemented correctly.The Hugeicons imports and usage follow best practices:
- Correct import from
@hugeicons/reactper coding guidelines- Proper
aria-hidden="true"for decorative icons- Consistent sizing with Tailwind classes
- Uses Nova design system color variables (
text-secondary)Also applies to: 122-124, 191-191
web-ui/src/components/TaskList.tsx (1)
18-18: LGTM! Clean icon integration for blocked task indicator.The
Cancel01Iconis appropriately used:
- Correct import source
- Proper
aria-hidden="true"for decorative use- Good layout with flex container and gap for icon-text alignment
- Appropriate sizing (
h-4 w-4) for inline contextAlso applies to: 140-143
web-ui/src/components/review/ReviewResultsPanel.tsx (1)
12-12: LGTM! Error and empty state icons migrated correctly.Good implementation with proper
aria-hidden="true"attributes and Nova color variables.Also applies to: 63-65, 78-80
web-ui/__tests__/components/ReviewSummary.test.tsx (1)
112-116: LGTM! Test assertions correctly updated for icon migration.The test assertions properly query for icons using
data-testidattributes that match the mock structure, correctly verifying icon presence in banners and category cards.Also applies to: 161-165, 282-290
web-ui/jest.setup.js (1)
59-114: LGTM! Comprehensive icon mock expansion for the migration.The global mock approach is well-structured:
- Factory pattern (
createIconMock) ensures consistent mock behaviorforwardRefsupports ref forwarding where neededdata-testidmatching icon names enables reliable test assertions- Alphabetically sorted array aids maintainability
The expanded icon list covers the icons introduced during the emoji-to-Hugeicons migration.
web-ui/__tests__/integration/dashboard-realtime-updates.test.tsx (1)
70-83: LGTM!The expanded icon mocks follow the established
createMockIconfactory pattern and maintain consistent test ID naming conventions. This properly extends mock coverage for the quality gate and agent card icons used by dashboard components.web-ui/__tests__/integration/discovery-answer-flow.test.tsx (1)
11-18: LGTM!The mock additions for
AlertDiamondIconandIdea01Iconcomplete the icon coverage for the DiscoveryProgress component. The inline mock pattern is consistent with the existing mocks in this file.web-ui/__tests__/components/ReviewFindings.test.tsx (3)
25-43: LGTM!Well-structured mock implementation with the factory pattern, proper
displayNameassignment for debugging, andaria-hidden="true"for accessibility. The mock covers all category icons used by the ReviewFindings component.
92-95: LGTM!Test assertion properly migrated from emoji text matching to data-testid based verification for the checkmark icon.
152-156: LGTM!Test correctly verifies the security category icon via data-testid instead of emoji.
web-ui/__tests__/components/checkpoints/CheckpointRestore.test.tsx (2)
21-29: LGTM!The mock is properly implemented with
aria-hidden="true"for accessibility compliance.
152-156: LGTM!Test assertions correctly updated to verify both the warning text and the Alert02Icon presence via data-testid.
web-ui/src/components/TaskTreeView.tsx (3)
11-11: LGTM - Hugeicons import correctly configured.The icon imports align with the project's coding guidelines requiring
@hugeicons/reactfor all icons.
74-81: LGTM - Provenance icons properly implemented.The function correctly returns accessible icon components with consistent sizing and
aria-hidden="true"for screen reader compatibility.
238-247: LGTM - Blocked badge with icon and label.Good accessibility pattern: the icon is decorative (
aria-hidden="true") while the visible "Blocked" text provides the semantic meaning for all users.web-ui/src/types/reviews.tsx (3)
8-14: LGTM - Icon imports for category mapping.Correct usage of
@hugeicons/reactper coding guidelines. The selected icons appropriately represent each category semantically.
125-145: LGTM - Category icon mapping function.Clean implementation with exhaustive coverage of all
ReviewCategoryvalues and a sensible default fallback. The sharediconPropspattern reduces duplication.
147-157: Consider removing deprecated constant in a follow-up. The deprecation annotation is appropriate for the migration. Verify no consumers still referenceCATEGORY_ICONSbefore removal.web-ui/src/components/review/ReviewFindingsList.tsx (4)
10-19: LGTM - Icon imports properly configured.All icons are correctly imported from
@hugeicons/reactper coding guidelines.
67-76: LGTM - Empty state with celebratory icon.Good UX pattern using
CheckmarkCircle01Iconto reinforce the positive "no findings" message. Proper accessibility witharia-hidden="true".
116-124: LGTM - Suggestion block with icon.Clean flexbox layout with proper icon alignment (
mt-0.5for vertical centering with text). TheIdea01Iconeffectively highlights suggestions.
45-64: This represents intentional separation of two distinct review systems:ReviewFindingsList.tsxhandlesFindingCategorytypes (from the Review Agent feature in Sprint 9), whilereviews.tsxhandlesReviewCategoryenum types (from the legacy review system in Sprint 10). These use different category sets, different icon mappings, and are used with different data models (ReviewFindingvsCodeReview). No consolidation is needed—this is correct architectural separation.Likely an incorrect or invalid review comment.
web-ui/__tests__/components/review/ReviewResultsPanel.test.tsx (3)
13-27: LGTM - Clean Hugeicons mock pattern.The
createMockIconhelper produces testable mock components withdata-testidattributes and properdisplayNamefor debugging. This pattern enables reliable icon presence assertions.
114-122: LGTM - Error state icon test.Clean migration from emoji text assertion to
data-testidbased verification. The async pattern withwaitForcorrectly handles the state transition.
178-191: LGTM - No review state icon test.Consistent testing pattern for verifying
FileEditIconpresence in the no-review state.web-ui/__tests__/components/review/ReviewFindingsList.test.tsx (2)
12-32: LGTM - Comprehensive Hugeicons mock setup.All 8 icons used by
ReviewFindingsListare properly mocked with testabledata-testidattributes. The mock pattern is consistent with other test files in this PR.
190-228: LGTM - Category icon test coverage.Tests cover all category mappings including the default/unknown case, ensuring the
getCategoryIconfunction works correctly across all scenarios.web-ui/__tests__/components/TaskTreeView.test.tsx (3)
12-28: LGTM - Clean Hugeicons mock pattern.The
createMockIconhelper provides consistent mock SVGs withdata-testid,classNamesupport, andaria-hiddenattributes. This pattern aligns well with the broader migration and enables reliable icon assertions.
126-132: LGTM - Icon assertions properly migrated.The provenance indicator tests correctly use
getByTestIdto verifyBotIconandUserIconpresence instead of emoji text matching.
612-697: LGTM - Dependency and blocking indicator tests updated correctly.The tests properly verify:
Link01Iconfor dependency indicatorsCancel01Iconfor blocked status- Absence of blocked icon when dependencies are satisfied
web-ui/src/components/AgentCard.tsx (3)
2-11: LGTM - Icon imports align with Hugeicons standard.All icons are imported from
@hugeicons/reactas per coding guidelines. Each imported icon is used in the badge configurations.
55-72: LGTM - Badge configurations cleanly migrated to Icon components.The type signature
React.ComponentType<{ className?: string; 'data-testid'?: string }>properly accommodates both production and test usage. Icon choices are semantically appropriate for each agent type and maturity level.
129-144: LGTM - Icon rendering with proper accessibility.Icons are correctly rendered as components with:
- Consistent
h-3 w-3sizingaria-hidden="true"for decorative icons (text labels provide meaning)web-ui/src/components/BlockerBadge.tsx (2)
9-9: LGTM - Appropriate icon selection for blocker types.
Alert02Iconfor critical (SYNC) andIdea01Iconfor informational (ASYNC) blockers are semantically appropriate choices.
38-49: LGTM - Clean icon component rendering.The dynamic icon rendering pattern
<config.Icon className="..." aria-hidden="true" />is correct and maintains accessibility.web-ui/__tests__/components/BlockerBadge.test.tsx (2)
10-21: LGTM - Local Hugeicons mock is functional.The mock correctly provides
data-testid,className, andaria-hiddenattributes for testing. While there's a global mock available injest.setup.js, local mocks ensure test isolation and explicit dependencies.
118-130: LGTM - Icon size verification tests.Tests correctly verify that icons receive the expected
h-3.5 w-3.5size classes, ensuring visual consistency.web-ui/src/components/reviews/ReviewSummary.tsx (4)
14-15: LGTM - Proper icon imports and centralized category icon usage.Good approach using
getCategoryIconfor category icons (promoting consistency) while importing specific icons for unique contexts (recommendation, blocking/success banners).
46-48: LGTM - Category icon retrieval with defensive casting.Using
getCategoryIconwith type assertion handles the string-to-enum conversion appropriately, with the fallback ingetCategoryIconhandling unknown categories.
243-276: LGTM - Banner icons appropriately sized and accessible.The blocking and success banners use:
Alert02IconandCheckmarkCircle01Iconwith appropriate semantic meaning- Larger
h-6 w-6sizing suitable for prominent banner displaysaria-hidden="true"since adjacent text conveys the message
371-384: LGTM - Category breakdown uses centralized icon function.Consistent usage of
getCategoryIcon(category)maintains DRY principle and ensures icon consistency across the application.web-ui/src/components/reviews/ReviewFindings.tsx (2)
21-23: LGTM! Clean migration to Hugeicons.The imports are properly structured with
getCategoryIconfor category-based icons andCheckmarkCircle01Iconfor the specific empty-state success indicator. This aligns with the codebase guideline to use Hugeicons for all icons.
239-242: LGTM! Proper use of getCategoryIcon.The
getCategoryIconfunction correctly returns the appropriate icon component based on the finding's category, with accessibility attributes already included in the returned JSX.web-ui/__tests__/components/DiscoveryProgress.testutils.tsx (1)
57-81: Well-structured Hugeicons mock with factory pattern.The
createMockIconfactory provides consistent mock implementations with properdisplayName,data-testid, andaria-hiddenattributes. This pattern enables reliable icon assertions in tests while maintaining good debugging experience.web-ui/__tests__/components/QualityGateStatus.test.tsx (2)
12-39: LGTM! Comprehensive Hugeicons mock for QualityGateStatus tests.The mock covers all icons used by the QualityGateStatus component and its dependencies. The factory pattern ensures consistent mock behavior with proper accessibility attributes.
93-101: LGTM! Test assertion correctly updated for Hugeicons migration.The test now properly verifies the
Alert02Iconpresence viadata-testidinstead of emoji text matching, aligning with the migration strategy.web-ui/src/components/ErrorBoundary.tsx (2)
11-11: LGTM! Proper Hugeicons import.The import of
Alert02Iconfrom@hugeicons/reactfollows the codebase guidelines for icon usage.
82-89: LGTM! Well-styled error icon with proper accessibility.The
Alert02Iconis appropriately sized (h-6 w-6), uses semantic destructive coloring, and includesaria-hidden="true"since the accompanying text conveys the error state to screen readers.web-ui/__tests__/components/ErrorBoundary.test.tsx (2)
16-24: LGTM! Minimal and focused Hugeicons mock.The mock correctly includes only
Alert02Icon, which is the only Hugeicons component used by ErrorBoundary. This keeps the test focused and avoids unnecessary mock complexity.
210-219: LGTM! Test correctly verifies Alert02Icon presence.The test assertion is updated to use
getByTestId('Alert02Icon'), properly validating the emoji-to-Hugeicons migration in the error UI.web-ui/__tests__/components/DiscoveryProgress.test.tsx (1)
9-35: LGTM! Well-structured Hugeicons mock with proper ordering.The mock factory pattern creates consistent icon components with appropriate accessibility attributes (
aria-hidden="true"), test IDs, anddisplayNamefor debugging. Placing the mock before component imports ensures Jest hoisting works correctly.web-ui/__tests__/components/BlockerPanel.test.tsx (2)
51-51: Assertion updated to use test ID - aligns with icon migration.The test now correctly queries by
data-testidinstead of emoji text matching.
269-272: Test updated to verify BotIcon presence.The renamed test and updated assertion correctly verify the icon component is rendered instead of the robot emoji.
web-ui/src/components/AgentList.tsx (3)
24-24: LGTM! Correct Hugeicons imports.All icon imports are from
@hugeicons/reactas required by the coding guidelines, with no mixing of other icon libraries.
133-133: LGTM! Proper accessibility attribute on decorative icon.The
aria-hidden="true"attribute correctly marks this icon as decorative since the adjacent text conveys the error meaning.
206-213: LGTM! Well-structured refresh button with icon and text label.The button correctly combines:
inline-flex items-center gap-1for proper icon-text alignmentaria-hidden="true"on the decorative icon- Visible "Refresh" text for accessibility
web-ui/src/components/quality-gates/QualityGatesPanelFallback.tsx (3)
12-12: LGTM! Correct Hugeicons imports for fallback component.Icons are imported from the designated icon library per coding guidelines.
58-65: LGTM! Accessible error icon in header.The Alert02Icon has
aria-hidden="true"since the adjacent heading text "Quality Gates Panel Unavailable" provides the semantic meaning.
96-103: LGTM! Retry button with proper icon-label pattern.The button correctly uses
inline-flex items-center gap-2for layout,aria-hidden="true"on the decorative icon, and maintains an accessiblearia-labelon the button itself.web-ui/__tests__/components/Dashboard.test.tsx (1)
69-82: LGTM! Comprehensive icon mock expansion for Dashboard dependencies.The added mocks cover icons used by Quality Gates components and AgentCard, with helpful grouping comments. The kebab-case test ID convention is consistent within this file.
web-ui/src/components/quality-gates/QualityGatesPanel.tsx (4)
28-28: LGTM!The Hugeicons imports are correctly added from
@hugeicons/react, aligning with the coding guidelines to use Hugeicons for all icons. The selected icons appropriately match their semantic contexts (info, alert, checklist, checkmark).
211-230: LGTM!The planning phase message correctly uses Hugeicons with proper accessibility attributes (
aria-hidden="true") and appropriate sizing. The icons serve as visual reinforcement alongside meaningful text labels.
241-248: LGTM!The
InformationCircleIconappropriately replaces the previous emoji for the empty state message, with correct accessibility attributes and sizing.
280-288: LGTM!The
Alert02Iconcorrectly replaces the error emoji with proper destructive color styling andaria-hidden="true"for accessibility.web-ui/__tests__/components/AgentCard.test.tsx (2)
12-34: LGTM!The Hugeicons mock pattern is well-structured:
- Placed before component import to ensure proper Jest hoisting
- Uses a clean
createMockIconfactory withdisplayNamefor debugging- Includes
aria-hidden="true"to mirror production behavior- Test IDs follow a consistent kebab-case naming convention
238-286: LGTM!The test assertions are correctly migrated to use
getByTestIdqueries, which are more robust than emoji text matching. Each agent type correctly maps to its expected icon test ID.web-ui/__tests__/components/AgentList.test.tsx (1)
15-40: LGTM!The mock correctly covers icons for both
AgentListand its child componentAgentAssignmentCard. The helpful comments document which icons belong to which component, improving maintainability.web-ui/src/components/AgentAssignmentCard.tsx (4)
14-22: LGTM!The Hugeicons imports are correctly added, covering all agent types and the active assignment indicator. This aligns with the coding guidelines to use
@hugeicons/reactfor all icons.
67-93: LGTM!The
agentTypeBadgesstructure is well-designed with proper TypeScript typing for theIconcomponent. The fallback toBotIconfor unknown agent types ensures graceful handling of edge cases.
190-195: LGTM!The dynamic icon rendering using
<agentTypeBadge.Icon />is a clean pattern that allows type-safe icon selection. Thearia-hidden="true"attribute correctly marks the icon as decorative.
254-259: LGTM!The active assignment indicator correctly uses
CheckmarkCircle01Iconwith proper accessibility attributes and Nova design token coloring (text-secondary).web-ui/src/components/quality-gates/QualityGateStatus.tsx (8)
22-29: LGTM!The Hugeicons imports are comprehensive, covering all UI states: error, no-status, approval, refresh, warnings, failures, and success. This aligns with the coding guidelines.
134-144: LGTM!The error state correctly uses
Alert02Iconwith destructive styling and proper accessibility attributes.
148-164: LGTM!The no-status state appropriately uses
Settings01Iconto indicate quality gates haven't run yet, with proper accessibility handling.
183-188: LGTM!The "Requires Approval" badge is well-structured using
UserIconwith an inline-flex layout combining the icon and text.
208-214: LGTM!The re-run button correctly uses
RefreshIconwith proper accessibility attributes.
221-239: LGTM!The warning banner appropriately reuses
Alert02Iconwith yellow/warning color styling.
258-262: LGTM!The failures header uses
Cancel01Iconwith destructive coloring to visually indicate failed quality gates.
299-308: LGTM!The success message correctly uses
CheckmarkCircle01Iconwith green styling to indicate all gates passed.web-ui/src/lib/qualityGateUtils.tsx (2)
7-19: LGTM!Hugeicons imports are correctly used, aligning with the coding guidelines to use
@hugeicons/reactfor all icons. The imported icon set covers all the quality gate types and status values.
29-47: LGTM!The
getGateIconfunction correctly maps gate types to their corresponding Hugeicons components with consistent props. The switch handles both E2E (type-check,lint,review) and backend (type_check,linting,code_review) naming conventions appropriately.web-ui/__tests__/components/quality-gates/QualityGatesPanelFallback.test.tsx (2)
14-27: LGTM!Clean mock implementation with a reusable factory function. The mock icons include
data-testidfor test assertions andaria-hiddento match the production component's accessibility attributes. Only the icons actually used by this component are mocked, keeping the test focused.
51-60: LGTM!Test correctly updated from checking emoji text to querying the icon by
data-testid. This approach is more robust and accurately tests the component's icon rendering behavior.web-ui/__tests__/lib/qualityGateUtils.test.tsx (2)
18-66: LGTM!Tests correctly migrated to render the returned JSX elements and assert icon presence via
data-testid. Coverage includes all gate type variants (E2E and backend naming conventions) plus fallback behavior for unknown/empty inputs.
156-189: LGTM!Status icon tests comprehensively cover all valid status values (
passed,failed,running,pending) plus edge cases (null,undefined, unknown strings). The render-and-query pattern is consistent with thegetGateIcontests.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Review SummaryThis is a well-executed migration from emojis to Hugeicons that aligns with the Nova design system. The implementation is comprehensive and consistent across all 42 affected files. ✅ Strengths
🔍 Code Quality IssueLocation: The function has redundant className props that create confusion: The className from gets completely overridden by the explicit className in each return statement (last className wins in React). While this works functionally, it's misleading. Suggested Fix: 📝 Minor Notes
✅ RecommendationApprove with optional fix for the className duplication issue. The change is non-breaking and improves code clarity. |
|
Review comment posted successfully to PR #286: #286 (comment) |
- Remove duplicate warning icon in CheckpointRestore (inline SVG + emoji) - Update stale .bg-blue-50 test selectors to .bg-muted (Nova palette) - Correct misleading comment about Hugeicons data-testid in AgentList test - Update CheckpointRestore test to match separated emoji/text structure
Review FeedbackOverall, this is a well-executed migration to Hugeicons that improves consistency and accessibility across the codebase. However, there are a few issues that should be addressed: 🔴 Issues to Address1. Incomplete migration in (lines 117-130) 2. Incorrect icon sizing in (line 170-173) 3. Stale test selector in (lines 100, 392) 4. Redundant className in (lines 110-123) ✅ Strengths
These changes should be made before merging to complete the migration and ensure code clarity. |
|
Review comment posted successfully to PR #286: #286 (comment) |
| }; | ||
|
|
||
| const agentTypeBadge = agentTypeBadges[agent.type] || { bg: 'bg-muted', text: 'text-foreground', icon: '🤖' }; | ||
| const agentTypeBadge = agentTypeBadges[agent.type] || { bg: 'bg-muted', text: 'text-foreground', Icon: BotIcon }; |
There was a problem hiding this comment.
agent.type lookup can hit prototype keys (e.g., "constructor"), so the fallback never applies and agentTypeBadge.Icon becomes undefined, causing an invalid React element error. Consider guarding with hasOwnProperty (or using a Map/null-prototype object) before using the result.
| const agentTypeBadge = agentTypeBadges[agent.type] || { bg: 'bg-muted', text: 'text-foreground', Icon: BotIcon }; | |
| const agentTypeBadge = Object.prototype.hasOwnProperty.call(agentTypeBadges, agent.type) | |
| ? agentTypeBadges[agent.type] | |
| : { bg: 'bg-muted', text: 'text-foreground', Icon: BotIcon }; |
🚀 Want me to fix this? Reply ex: "fix it for me".
- Replace success message inline SVG with CheckmarkCircle01Icon - Remove redundant text-2xl wrapper (icons already sized via h-5 w-5) - Fix redundant iconProps className in getStatusIcon utility - Update test mock to include CheckmarkCircle01Icon


Summary
Changes
Components updated:
Utilities refactored:
qualityGateUtils.ts→qualityGateUtils.tsx(JSX icon functions)reviews.ts→reviews.tsx(getCategoryIcon returns JSX)Test updates:
Test plan
Closes Issue #219
Summary by CodeRabbit
Style
New Features
Tests
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.