feat(web-ui): task status tooltips and valid action guidance (#481)#524
Conversation
- Add STATUS_INFO constants in taskStatusInfo.ts with meaning, entry condition, and next-steps for all 7 task statuses - Wrap status badge in TaskCard with Tooltip showing meaning + next steps - Wrap status badge in TaskDetailModal with Tooltip showing meaning + entry condition - Add 'What's next?' guidance panel in TaskDetailModal footer for DONE, BLOCKED, and MERGED statuses (which have no action buttons) - Show 'Last changed' date in TaskDetailModal metadata when updated_at is set - Add 20 unit tests covering tooltip content and guidance panel rendering
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as TaskDetailModal
participant API as tasksApi.getOne
participant Tooltip as TooltipProvider/Tooltip
User->>UI: open modal (taskId)
UI->>API: fetch task (taskId)
API-->>UI: return task data
UI->>Tooltip: render status Badge wrapped in Tooltip
Tooltip-->>User: show tooltip (meaning + nextSteps) on hover
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
test comment placeholder |
PR Review: feat(web-ui): task status tooltips and valid action guidanceGood UX addition overall. The single-source-of-truth approach with Issue 1 - Performance: Multiple TooltipProvider instances
Fix: Move Issue 2 - Inconsistent tooltip content between TaskCard and TaskDetailModal
Same status badge, different tooltip content depending on where you click. Users who open the modal expecting the same tooltip context they saw on the card will get different information. Decide on one consistent set of fields (probably Issue 3 - Guidance panel missing for FAILED status The guidance panel ( Fix: Add Issue 4 - Minor: toLocaleDateString locale handling Verify whether the "Last changed" date formatting passes an explicit locale (e.g. What's working well
Summary: The |
- Consolidate to single TooltipProvider per TaskCard (removes per-tooltip overhead) - Align modal tooltip content to match card (meaning + nextSteps in both) - Add data-testid="status-next-step" to FAILED alert panel for consistent testability - Use 'en-US' locale in toLocaleDateString for deterministic date formatting - Add 2 tests: FAILED guidance panel + READY has no guidance panel
Follow-up ReviewAll four issues from my previous review have been addressed: Fixed: TooltipProvider placement — Single Fixed: Tooltip content consistency — Both Fixed: FAILED guidance panel — The FAILED alert panel now carries Fixed: Locale handling — One minor observation: This is ready to merge. The implementation is clean, test coverage is thorough, and the feature is complete across all 7 statuses. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web-ui/src/components/tasks/TaskDetailModal.tsx (1)
158-170: Tooltip implementation is correct; consider addressing the type assertion.The tooltip correctly displays
meaningandnextStepsfromSTATUS_INFO. SinceSTATUS_INFOis typed asRecord<TaskStatus, StatusInfo>and covers all seven status values, the access pattern is type-safe.The
as nevertype assertion on line 161 for the Badge variant is a workaround for variant typing constraints. This pattern appears elsewhere in the codebase (e.g., dependency badges at line 226), so it's consistent, but it circumvents type checking.💡 Optional: Consider typing STATUS_BADGE_VARIANT more precisely
If the
Badgecomponent'svariantprop type were extended or ifSTATUS_BADGE_VARIANTwere typed to match the expected variant union, theas neverassertion could be eliminated for better type safety. This is a broader refactor that could be addressed in a follow-up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-ui/src/components/tasks/TaskDetailModal.tsx` around lines 158 - 170, The Badge variant is being forced with "as never"; instead derive the Badge variant type and type STATUS_BADGE_VARIANT to that so the cast can be removed: get the variant prop type from the Badge component (e.g., React.ComponentProps<typeof Badge>['variant'] or the BadgeProps union) and update the declaration of STATUS_BADGE_VARIANT to be Record<TaskStatus, ThatVariantType>, then update the usage in TaskDetailModal (the Badge inside Tooltip/TooltipTrigger) to pass STATUS_BADGE_VARIANT[task.status] directly without "as never"; this will remove the unsafe assertion while keeping the existing mapping and labels (STATUS_LABEL, STATUS_INFO) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@web-ui/src/components/tasks/TaskDetailModal.tsx`:
- Around line 158-170: The Badge variant is being forced with "as never";
instead derive the Badge variant type and type STATUS_BADGE_VARIANT to that so
the cast can be removed: get the variant prop type from the Badge component
(e.g., React.ComponentProps<typeof Badge>['variant'] or the BadgeProps union)
and update the declaration of STATUS_BADGE_VARIANT to be Record<TaskStatus,
ThatVariantType>, then update the usage in TaskDetailModal (the Badge inside
Tooltip/TooltipTrigger) to pass STATUS_BADGE_VARIANT[task.status] directly
without "as never"; this will remove the unsafe assertion while keeping the
existing mapping and labels (STATUS_LABEL, STATUS_INFO) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18df47dc-4b9e-4228-8123-ecb80250b25b
📒 Files selected for processing (3)
web-ui/src/__tests__/components/tasks/TaskDetailModal.test.tsxweb-ui/src/components/tasks/TaskCard.tsxweb-ui/src/components/tasks/TaskDetailModal.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web-ui/src/components/tasks/TaskCard.tsx
- web-ui/src/tests/components/tasks/TaskDetailModal.test.tsx
The frontend-tests CI job was disabled during the Phase 2 CLI-first refactor and never re-enabled when Phase 3 web UI became active. This allowed component changes to silently break tests across multiple PRs. CI change: - Uncomment frontend-tests job in test.yml (runs npm run test:coverage) - Add frontend-tests to test-summary needs and failure check - Exclude e2e/ (Playwright) and test-helpers.ts from Jest via jest.config.js Test fixes (component updated but test not): - QuickActions: rewrite tests to exercise all 5 pipeline states (broke in #493 — context-aware refactor changed button labels) - TaskCard: update dependency title to regex match Radix Tooltip text (broke in #524 — tooltip replaced title attribute) - TaskDetailModal: update dep count assertion to "Dependencies (N)" (broke in #524 — label changed from "2 dependencies") - BlockerCard: update success text to "answer recorded" (broke in #531 — success message reworded) - TaskBoardView: update empty state to "No tasks yet" (single block) (broke in #499 — empty state redesigned with CTA)
* feat(core): WorktreeRegistry, orphan cleanup, and get_base_branch (#533) Add atomic worktree registration and stale cleanup to complete the worktree-per-task isolation feature for parallel batch execution. Changes: - codeframe/core/worktrees.py: add WorktreeRegistry (register/unregister/ list_stale/cleanup_stale), get_base_branch(), list_worktrees() - codeframe/core/sandbox/context.py: use get_base_branch() instead of hardcoded "main" when creating worktrees - codeframe/core/conductor.py: call WorktreeRegistry.cleanup_stale() at _execute_parallel() start when isolation == worktree - codeframe/cli/env_commands.py: report stale worktrees in cf env doctor - tests/core/test_worktrees.py: 13 new tests for registry, get_base_branch, list_worktrees, and conductor orphan cleanup integration Closes #533 * fix(web-ui): re-enable frontend CI and fix 5 stale tests The frontend-tests CI job was disabled during the Phase 2 CLI-first refactor and never re-enabled when Phase 3 web UI became active. This allowed component changes to silently break tests across multiple PRs. CI change: - Uncomment frontend-tests job in test.yml (runs npm run test:coverage) - Add frontend-tests to test-summary needs and failure check - Exclude e2e/ (Playwright) and test-helpers.ts from Jest via jest.config.js Test fixes (component updated but test not): - QuickActions: rewrite tests to exercise all 5 pipeline states (broke in #493 — context-aware refactor changed button labels) - TaskCard: update dependency title to regex match Radix Tooltip text (broke in #524 — tooltip replaced title attribute) - TaskDetailModal: update dep count assertion to "Dependencies (N)" (broke in #524 — label changed from "2 dependencies") - BlockerCard: update success text to "answer recorded" (broke in #531 — success message reworded) - TaskBoardView: update empty state to "No tasks yet" (single block) (broke in #499 — empty state redesigned with CTA) * fix(core): address code review feedback on WorktreeRegistry (#537) - wire register()/unregister() into create_execution_context() — registry was defined but never populated, making cleanup_stale() a no-op - fix list_stale() to catch PermissionError separately (process alive, different owner is not stale); previously OSError caught both cases - guard list_worktrees() against non-list JSON (returns [] for dicts etc.) - fix get_base_branch() to return "main" in detached HEAD state (git returns literal "HEAD" — now treated as fallback) - fix conductor.py string comparison to use IsolationLevel.WORKTREE.value - replace misleading conductor test with two focused tests that patch WorktreeRegistry.cleanup_stale directly and assert call/no-call - add tests for detached HEAD fallback and non-list JSON guard --------- Co-authored-by: Test User <test@example.com>
1 participant
Summary
Closes #481
TaskCardandTaskDetailModalstatus badge now shows a Radix UI tooltip on hover with the status meaning, how the task entered this state, and what to do nextTaskDetailModalshows a "What's next?" info panel forDONE,BLOCKED, andMERGEDstatuses (which have no action buttons) — telling the user the next step to takeTaskDetailModalshows "Last changed: [date]" in metadata whenupdated_atis availabletaskStatusInfo.tsdefinesSTATUS_INFOfor all 7 statuses — single source of truth used by both componentsAcceptance Criteria
TaskDetailModalshows only valid next transitions (existing buttons already correct; guidance panel added for action-less states)updated_atshown as "Last changed")STATUS_INFOused everywhere)Test plan
src/__tests__/components/tasks/TaskCard.test.tsx: verifies tooltip content for all 7 statusesTaskDetailModal.test.tsx: verifies tooltip content, guidance panel presence (DONE/BLOCKED/MERGED), action buttons (BACKLOG/READY), and last-changed displaySummary by CodeRabbit
New Features
Tests