feat(web-ui): Glitch Capture REQ detail view and sidebar entry point (#569)#578
feat(web-ui): Glitch Capture REQ detail view and sidebar entry point (#569)#578
Conversation
…569) - Backend: expose scope field in RequirementResponse via new ScopeOut model populated from RequirementScope dataclass in _req_to_response() - Types: add ProofScope interface and scope field to ProofRequirement - REQ detail page (/proof/[req_id]): - Render description with ReactMarkdown (prose prose-sm classes) - Show 'Where found' from req.scope (comma-joined non-empty scope arrays) - Show 'source' and 'source_issue' in metadata row - Obligations table: add 'Latest Run' column deriving status from most-recent gate run evidence (latestRunByGate map) - Evidence empty state: replace plain text with styled CTA linking to /review - Sidebar: add 'Capture Glitch' button that opens CaptureGlitchModal (implemented in #568) and navigates to new REQ on success - Tests: full coverage for all new features across 3 test files
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds serialized requirement scope to the backend API and types, renders requirement descriptions as Markdown, shows scope on the proof detail page, derives obligation status from latest run evidence, and adds a "Capture Glitch" sidebar entry with modal and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Client (browser)
participant UI as Frontend (Next.js)
participant API as Backend API
participant DB as Database
rect rgba(0,128,0,0.5)
User->>UI: Click "Capture Glitch" (sidebar)
UI->>UI: Open CaptureGlitchModal
User->>UI: Submit capture (description, scope, obligations)
UI->>API: POST /api/proof (capture payload)
API->>DB: Persist requirement + scope
DB-->>API: Stored requirement (id)
API-->>UI: 201 Created (requirement id)
UI->>UI: Close modal, navigate to /proof/{id}
UI->>API: GET /api/proof/{id} (requirement detail)
API->>DB: Fetch requirement + runs/evidence
DB-->>API: Requirement + evidence
API-->>UI: RequirementResponse (includes scope)
UI->>User: Render detail page (Markdown, Where found, obligations, latest runs)
end
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. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Review: Phase 3.5C — Glitch Capture REQ Detail & Sidebar Entry PointGood implementation overall — this cleanly closes Phase 3.5C. The scope field flows end-to-end (backend → types → UI), the `latestRunByGate` useMemo correctly sources live status from evidence rather than stale obligation state, and the 28-test addition is well-structured with clear describe blocks. A few things worth addressing: 1.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
codeframe/ui/routers/proof_v2.py (1)
106-113: ConsiderField(default_factory=list)forScopeOutarrays for Pydantic best practice.While the current code passes Ruff checks in this repository's configuration, using
Field(default_factory=list)is the recommended Pydantic v2 pattern for mutable defaults and avoids potential linting issues if Ruff rules are later tightened.Fieldis already imported, so the change is straightforward.♻️ Suggested change
class ScopeOut(BaseModel): """Serialized requirement scope.""" - routes: list[str] = [] - components: list[str] = [] - apis: list[str] = [] - files: list[str] = [] - tags: list[str] = [] + routes: list[str] = Field(default_factory=list) + components: list[str] = Field(default_factory=list) + apis: list[str] = Field(default_factory=list) + files: list[str] = Field(default_factory=list) + tags: list[str] = Field(default_factory=list)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@codeframe/ui/routers/proof_v2.py` around lines 106 - 113, The ScopeOut model uses mutable default lists; change each field declaration in class ScopeOut (routes, components, apis, files, tags) to use Field(default_factory=list) instead of literal [] so Pydantic v2 handles mutable defaults correctly—update the field definitions in the ScopeOut class (which inherits BaseModel) to call Field(default_factory=list) for each list-typed attribute.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web-ui/src/app/proof/`[req_id]/page.tsx:
- Around line 315-320: The Latest Run cell currently renders latestEv.run_id as
plain text; make it actionable by converting it into a link or button that
navigates to the run details page (use the same route used by your run
detail/history view). Update the JSX where latestEv is rendered (the <td>
showing latestEv.run_id) to wrap the id in a Link or clickable element and wire
its onClick or href to the run detail route using latestEv.run_id (and preserve
the satisfied coloring logic: className={latestEv.satisfied ? 'text-green-600' :
'text-red-600'}). Ensure accessibility (aria-label) and that navigation uses the
app's client-side routing so the drill-down to the run history/detail works from
this cell.
In `@web-ui/src/components/layout/AppSidebar.tsx`:
- Around line 147-154: The collapsed "Capture Glitch" button in AppSidebar.tsx
lacks an accessible name; update the button element (the one that calls
setShowCaptureModal(true) and renders Add01Icon) to include an accessible label
such as aria-label="Capture Glitch" or aria-labelledby pointing to a visually
hidden text node so screen readers can discover it; ensure the label text
matches the visible "Capture Glitch" string and keep the existing hidden
lg:inline span for visual behavior.
---
Nitpick comments:
In `@codeframe/ui/routers/proof_v2.py`:
- Around line 106-113: The ScopeOut model uses mutable default lists; change
each field declaration in class ScopeOut (routes, components, apis, files, tags)
to use Field(default_factory=list) instead of literal [] so Pydantic v2 handles
mutable defaults correctly—update the field definitions in the ScopeOut class
(which inherits BaseModel) to call Field(default_factory=list) for each
list-typed attribute.
🪄 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: 2311a733-fc90-4bdb-8608-e0b56951a556
📒 Files selected for processing (8)
codeframe/ui/routers/proof_v2.pyweb-ui/__mocks__/@hugeicons/react.jsweb-ui/__tests__/app/proof/req_id/page.test.tsxweb-ui/__tests__/components/layout/AppSidebar.test.tsxweb-ui/src/__tests__/components/proof/ProofDetailPage.test.tsxweb-ui/src/app/proof/[req_id]/page.tsxweb-ui/src/components/layout/AppSidebar.tsxweb-ui/src/types/index.ts
…#578) - Make obligations Latest Run run_id a clickable button that sets the evidence history search filter to show that run's evidence - Add aria-label and aria-hidden to Capture Glitch sidebar button for accessibility in collapsed sidebar (below lg breakpoint) - Use Field(default_factory=list) for ScopeOut mutable defaults per Pydantic v2 best practice
Follow-up ReviewChecking back after the latest push. Retracting a previous false positive: My concern about Addressed from my prior review:
Still outstanding (lower priority, not blockers):
The two items I flagged as "worth fixing before merge" (typography plugin and aria-label) are both resolved. The remaining items are style/robustness preferences. No blockers. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web-ui/src/app/proof/`[req_id]/page.tsx:
- Around line 317-320: The "Latest Run" button only calls
updateSearch(latestEv.run_id), which can leave existing gate/result filters
active and hide the run; change the click handler to also clear incompatible
filters by resetting the gate and result filter state before persisting the run
filter — e.g., call the existing setters (or a single search-update helper) to
set gateFilter and resultFilter to their default/empty values and then call
updateSearch(latestEv.run_id) (or extend updateSearch to accept a full filter
object) so that drilling into a run always clears gate/result filters and
reliably shows the run.
🪄 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: 9571fbcc-9293-4ebc-9c08-ccda01c616df
📒 Files selected for processing (3)
codeframe/ui/routers/proof_v2.pyweb-ui/src/app/proof/[req_id]/page.tsxweb-ui/src/components/layout/AppSidebar.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- codeframe/ui/routers/proof_v2.py
…obligations table
Follow-up Review (latest commit:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web-ui/src/app/proof/`[req_id]/page.tsx:
- Around line 118-129: latestRunByGate currently compares ev.timestamp and
existing.timestamp as raw strings which can produce incorrect "latest" picks if
timestamps aren't lexically sortable; update the comparison in the useMemo so it
parses timestamps to a comparable numeric form (e.g., Date.parse or new
Date(...).getTime()) before comparing, and handle invalid/missing timestamps by
falling back to 0 or another safe sentinel; ensure you update references to
ev.timestamp and existing.timestamp in the map population logic (in the
latestRunByGate computation over evidence / ProofEvidence) so the comparison
uses the parsed numeric values.
- Around line 254-256: ReactMarkdown is rendering images from user-provided
req.description which can trigger remote fetches; prevent that by disabling
image/embed elements when rendering descriptions: update the ReactMarkdown usage
(the JSX that renders req.description) to supply a components prop that
overrides img/image/iframe (and other embed tags you deem risky) to render
nothing or a safe placeholder, and also enable HTML-skipping/disable raw HTML if
your ReactMarkdown version supports skipHtml or disallowedElements to avoid
embedded HTML; target the ReactMarkdown element that consumes req.description in
page.tsx to apply these changes.
🪄 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: a2df4f05-329e-4fcc-8304-3b6cdfafa2e7
📒 Files selected for processing (1)
web-ui/src/app/proof/[req_id]/page.tsx
- Use Date.parse() for timestamp comparison in latestRunByGate to handle varying ISO 8601 offset formats correctly - Disable img element in ReactMarkdown to prevent remote resource fetches from user-authored requirement descriptions (security fix)
Follow-up Review (3rd pass)Reviewing the latest commit ( Resolved since last reviewFilter-clearing on Latest Run click — CodeRabbit raised that the previous version left gate/result filters active when clicking a run ID, which could hide the run. The new Timestamp comparison — now uses Still outstanding (style/robustness, not blockers)
No blockers. All items flagged as "worth fixing before merge" in the initial review have been resolved. Ready to merge. |
Follow-up Review (4th pass — docs-only commit)The latest commit updates only documentation: Docs accuracy check:
No discrepancies between the doc updates and the code that shipped in earlier commits. Status of all prior items:
Nothing new to flag. Ready to merge. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/PHASE_3_UI_ARCHITECTURE.md`:
- Line 137: Update the documentation entry that currently reads "PROOF9" to
match the implementation by changing the navigation label to "Proof"; the
implemented label is defined in AppSidebar.tsx as label: 'Proof', so replace the
"PROOF9" string in the docs with "Proof" to keep docs and code consistent.
- Around line 373-388: The docs claim named exports (RequirementHeader,
ObligationsTable, EvidenceHistory, GateEvidencePanel, WaiveDialog) exist for
/proof/[req_id], but the actual implementation uses inline JSX sections in
web-ui/src/app/proof/[req_id]/page.tsx (commented as {/* Header */}, {/*
Obligations */}, {/* Evidence history */} etc. Fix by either: 1) updating
PHASE_3_UI_ARCHITECTURE.md to describe the inline implementation and list the
documented sections as inline JSX blocks found in page.tsx, or 2) refactor
page.tsx by extracting the commented sections into discrete exported components
named RequirementHeader, ObligationsTable, EvidenceHistory, GateEvidencePanel,
and WaiveDialog, preserving existing props/hooks/state and replacing the inline
JSX with imports/usages so the documented component hierarchy matches the code.
🪄 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: c73b78e2-b5c9-41b8-afba-84dec83bff44
📒 Files selected for processing (3)
CLAUDE.mddocs/PHASE_3_UI_ARCHITECTURE.mddocs/PRODUCT_ROADMAP.md
✅ Files skipped from review due to trivial changes (2)
- CLAUDE.md
- docs/PRODUCT_ROADMAP.md
| 4. **Execution** (play/monitor icon) - only visible when runs are active | ||
| 5. **Blockers** (alert icon) - badge count for open blockers | ||
| 6. **Review** (git branch icon) | ||
| 7. **PROOF9** (checkmark icon) |
There was a problem hiding this comment.
Correct the navigation label from "PROOF9" to "Proof".
The documentation claims the navigation item is labeled "PROOF9", but the actual implementation in AppSidebar.tsx uses "Proof" as the label (line 37: label: 'Proof'). Update the documentation to match the implemented label.
📝 Proposed fix
-7. **PROOF9** (checkmark icon)
+7. **Proof** (checkmark icon)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 7. **PROOF9** (checkmark icon) | |
| 7. **Proof** (checkmark icon) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/PHASE_3_UI_ARCHITECTURE.md` at line 137, Update the documentation entry
that currently reads "PROOF9" to match the implementation by changing the
navigation label to "Proof"; the implemented label is defined in AppSidebar.tsx
as label: 'Proof', so replace the "PROOF9" string in the docs with "Proof" to
keep docs and code consistent.
| ProofRequirementPage (/proof/[req_id]) | ||
| ├── RequirementDetail | ||
| │ ├── ObligationsList | ||
| │ └── EvidenceHistory | ||
| └── WaiveForm | ||
| ├── RequirementHeader | ||
| │ ├── Title, severity badge, ProofStatusBadge | ||
| │ ├── MarkdownDescription (ReactMarkdown, images disallowed) | ||
| │ ├── MetadataRow (created_at, source, source_issue, created_by, waiver expiry) | ||
| │ └── ScopeChips (files, routes, components, APIs, tags from ProofScope) | ||
| ├── ObligationsTable ← new (Phase 3.5C) | ||
| │ └── ObligationRow[] (gate name, Latest Run pass/fail badge, link to evidence) | ||
| │ └── Latest Run column: shows most-recent run result per gate | ||
| ├── EvidenceHistory | ||
| │ ├── FilterBar (gate select, result select, search input, Reset Filters) | ||
| │ ├── EvidenceTable (sortable: gate, result, run_id, timestamp, artifact) | ||
| │ │ └── EvidenceRow[] (click run_id → focusRun filter) | ||
| │ └── EmptyState CTA: "Capture a Glitch" link when no evidence exists | ||
| ├── GateEvidencePanel (loads artifact content for latest run) | ||
| └── WaiveDialog (modal, opens via Waive button in header) |
There was a problem hiding this comment.
Architectural mismatch: Documented components don't exist as discrete exports.
The documentation describes the /proof/[req_id] page as using named components (RequirementHeader, ObligationsTable, EvidenceHistory, GateEvidencePanel, WaiveDialog), but the actual implementation in web-ui/src/app/proof/[req_id]/page.tsx uses inline JSX sections (marked with comments like {/* Header */}, {/* Obligations */}, {/* Evidence history */}) without separate component exports.
While the described functionality is correctly implemented, developers looking for these named components will not find them in the codebase. Update the documentation to accurately reflect the inline implementation pattern or refactor the implementation to match the documented component hierarchy.
📝 Proposed documentation fix to match actual implementation
ProofRequirementPage (/proof/[req_id])
-├── RequirementHeader
-│ ├── Title, severity badge, ProofStatusBadge
-│ ├── MarkdownDescription (ReactMarkdown, images disallowed)
-│ ├── MetadataRow (created_at, source, source_issue, created_by, waiver expiry)
-│ └── ScopeChips (files, routes, components, APIs, tags from ProofScope)
-├── ObligationsTable ← new (Phase 3.5C)
-│ └── ObligationRow[] (gate name, Latest Run pass/fail badge, link to evidence)
-│ └── Latest Run column: shows most-recent run result per gate
-├── EvidenceHistory
-│ ├── FilterBar (gate select, result select, search input, Reset Filters)
-│ ├── EvidenceTable (sortable: gate, result, run_id, timestamp, artifact)
-│ │ └── EvidenceRow[] (click run_id → focusRun filter)
-│ └── EmptyState CTA: "Capture a Glitch" link when no evidence exists
-├── GateEvidencePanel (loads artifact content for latest run)
-└── WaiveDialog (modal, opens via Waive button in header)
+├── Header section (inline JSX)
+│ ├── Title, severity badge, ProofStatusBadge
+│ ├── Description (ReactMarkdown, images disallowed)
+│ ├── Metadata row (created_at, source, source_issue, created_by, waiver expiry)
+│ └── Scope chips (files, routes, components, APIs, tags from ProofScope)
+├── Obligations section (inline JSX table)
+│ └── Obligation rows (gate name, status, Latest Run link)
+│ └── Latest Run column: clickable run_id that filters evidence history
+├── Evidence History section (inline JSX)
+│ ├── Filter controls (gate select, result select, search input, Reset Filters)
+│ ├── Evidence table (gate, result, run_id, timestamp, artifact)
+│ │ └── Evidence rows (click run_id → focusRun filter)
+│ └── Empty state with "Run Gates →" CTA linking to /review
+└── Waiver section (inline JSX with modal dialog)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ProofRequirementPage (/proof/[req_id]) | |
| ├── RequirementDetail | |
| │ ├── ObligationsList | |
| │ └── EvidenceHistory | |
| └── WaiveForm | |
| ├── RequirementHeader | |
| │ ├── Title, severity badge, ProofStatusBadge | |
| │ ├── MarkdownDescription (ReactMarkdown, images disallowed) | |
| │ ├── MetadataRow (created_at, source, source_issue, created_by, waiver expiry) | |
| │ └── ScopeChips (files, routes, components, APIs, tags from ProofScope) | |
| ├── ObligationsTable ← new (Phase 3.5C) | |
| │ └── ObligationRow[] (gate name, Latest Run pass/fail badge, link to evidence) | |
| │ └── Latest Run column: shows most-recent run result per gate | |
| ├── EvidenceHistory | |
| │ ├── FilterBar (gate select, result select, search input, Reset Filters) | |
| │ ├── EvidenceTable (sortable: gate, result, run_id, timestamp, artifact) | |
| │ │ └── EvidenceRow[] (click run_id → focusRun filter) | |
| │ └── EmptyState CTA: "Capture a Glitch" link when no evidence exists | |
| ├── GateEvidencePanel (loads artifact content for latest run) | |
| └── WaiveDialog (modal, opens via Waive button in header) | |
| ProofRequirementPage (/proof/[req_id]) | |
| ├── Header section (inline JSX) | |
| │ ├── Title, severity badge, ProofStatusBadge | |
| │ ├── Description (ReactMarkdown, images disallowed) | |
| │ ├── Metadata row (created_at, source, source_issue, created_by, waiver expiry) | |
| │ └── Scope chips (files, routes, components, APIs, tags from ProofScope) | |
| ├── Obligations section (inline JSX table) | |
| │ └── Obligation rows (gate name, status, Latest Run link) | |
| │ └── Latest Run column: clickable run_id that filters evidence history | |
| ├── Evidence History section (inline JSX) | |
| │ ├── Filter controls (gate select, result select, search input, Reset Filters) | |
| │ ├── Evidence table (gate, result, run_id, timestamp, artifact) | |
| │ │ └── Evidence rows (click run_id → focusRun filter) | |
| │ └── Empty state with "Run Gates →" CTA linking to /review | |
| └── Waiver section (inline JSX with modal dialog) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/PHASE_3_UI_ARCHITECTURE.md` around lines 373 - 388, The docs claim named
exports (RequirementHeader, ObligationsTable, EvidenceHistory,
GateEvidencePanel, WaiveDialog) exist for /proof/[req_id], but the actual
implementation uses inline JSX sections in
web-ui/src/app/proof/[req_id]/page.tsx (commented as {/* Header */}, {/*
Obligations */}, {/* Evidence history */} etc. Fix by either: 1) updating
PHASE_3_UI_ARCHITECTURE.md to describe the inline implementation and list the
documented sections as inline JSX blocks found in page.tsx, or 2) refactor
page.tsx by extracting the commented sections into discrete exported components
named RequirementHeader, ObligationsTable, EvidenceHistory, GateEvidencePanel,
and WaiveDialog, preserving existing props/hooks/state and replacing the inline
JSX with imports/usages so the documented component hierarchy matches the code.
1 participant
Summary
Closes #569
scopefield inRequirementResponsevia newScopeOutPydantic model, populated from theRequirementScopedataclass in_req_to_response()ProofScopeinterface andscope: ProofScope | nullfield toProofRequirement/proof/[req_id]):ReactMarkdown(prose classes)req.scope(comma-joined non-empty scope arrays)latestRunByGatemap), not storedob.status/reviewCaptureGlitchModal(from [Phase 3.5C] Glitch Capture: entry point and capture form #568), navigates to the new REQ on successTest plan
npm test— 728 tests pass (28 new tests added across 3 files)npm run build— TypeScript and build cleanuv run pytest tests/ui/test_proof_v2.py— 47 backend tests passuv run ruff check .— no issuesSummary by CodeRabbit
New Features
Improvements
Tests
Documentation