Skip to content

feat(web-ui): PROOF9 requirements list, detail, and dashboard widget (#458)#461

Merged
frankbria merged 2 commits intomainfrom
feat/proof9-web-ui
Mar 20, 2026
Merged

feat(web-ui): PROOF9 requirements list, detail, and dashboard widget (#458)#461
frankbria merged 2 commits intomainfrom
feat/proof9-web-ui

Conversation

@frankbria
Copy link
Owner

@frankbria frankbria commented Mar 20, 2026

Summary

Closes #458. Adds all PROOF9 web UI views, consuming the /api/v2/proof/* endpoints from PR #459.

  • /proof — Requirements list table with ID, title, glitch type, severity, gate count, status badge, and per-row Waive button
  • /proof/[req_id] — Detail page: header, gate obligations, evidence history table, waiver section, inline waive dialog
  • Dashboard widgetProofStatusWidget card injected after task stats, shows open/satisfied/waived counts with auto-refresh (30s)
  • Sidebar — "Proof" nav item added after "Review" with CheckmarkCircle01Icon
  • Shared componentsProofStatusBadge (color-coded for all three statuses), proofApi client, 10 TypeScript interfaces

Files changed

File Change
web-ui/src/types/index.ts Add 10 PROOF9 TS interfaces
web-ui/src/lib/api.ts Add proofApi (5 methods)
web-ui/src/components/layout/AppSidebar.tsx Add Proof nav item
web-ui/src/components/proof/ProofStatusBadge.tsx New shared badge
web-ui/src/components/proof/ProofStatusWidget.tsx New dashboard card
web-ui/src/components/proof/index.ts Barrel exports
web-ui/src/app/proof/page.tsx New requirements list page
web-ui/src/app/proof/[req_id]/page.tsx New detail page
web-ui/src/app/page.tsx Inject ProofStatusWidget

Test plan

  • npm run build passes (✓ verified locally)
  • Unit tests pass: 347/347 (✓ verified locally; 4 e2e failures are pre-existing Playwright env issue)
  • /proof renders with empty state when no requirements exist
  • /proof renders table when requirements exist; waive dialog submits correctly
  • /proof/[req_id] shows obligations, evidence, and waiver sections
  • Dashboard shows PROOF9 widget after task stats card
  • Sidebar shows Proof item, active when on /proof routes

Summary by CodeRabbit

  • New Features
    • Proof status widget added to workspace pages showing open/satisfied/waived counts and a link to view all.
    • New Proof overview page listing requirements with filters, status badges, and in-place waive actions.
    • Requirement detail pages showing requirement metadata, evidence history, obligations, and waiver management (including a waive dialog).
    • Proof entry added to the sidebar for quick navigation.

…458)

- Add /proof requirements list page with status filter summary and waive dialog
- Add /proof/[req_id] detail page with evidence history, obligations, and waiver section
- Add ProofStatusWidget card on main dashboard (open/satisfied/waived counts, refreshes every 30s)
- Add ProofStatusBadge shared component (open=red, satisfied=green, waived=gray)
- Add proofApi client (getStatus, listRequirements, getRequirement, getEvidence, waive)
- Add 10 PROOF9 TypeScript interfaces mirroring proof_v2.py response models
- Add Proof nav item to sidebar (CheckmarkCircle01Icon)
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 20, 2026

Walkthrough

Adds a PROOF9 frontend: types and API client, dashboard widget and badge, a listing page and a detail page with evidence history and waive dialog, and a sidebar nav entry linking to the new proof pages.

Changes

Cohort / File(s) Summary
Types & API
web-ui/src/types/index.ts, web-ui/src/lib/api.ts
Added PROOF9 TypeScript types and a new proofApi client with methods: getStatus, listRequirements, getRequirement, getEvidence, and waive.
Pages (overview & proof)
web-ui/src/app/page.tsx, web-ui/src/app/proof/page.tsx, web-ui/src/app/proof/[req_id]/page.tsx
Inserted ProofStatusWidget into workspace overview; added /proof listing page with SWR-backed listing, status counts, and in-page waive dialog; added dynamic /proof/[req_id] detail page showing requirement metadata, evidence history, and a waive dialog.
Components (proof UI)
web-ui/src/components/proof/ProofStatusBadge.tsx, web-ui/src/components/proof/ProofStatusWidget.tsx, web-ui/src/components/proof/index.ts
New ProofStatusBadge and ProofStatusWidget components plus barrel exports; widget polls status and renders badges/summary with link to /proof.
Navigation
web-ui/src/components/layout/AppSidebar.tsx
Added a "Proof" nav item (/proof) and updated icon imports to include CheckmarkCircle01Icon.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant UI as Proof Page / WaiveDialog
  participant API as Server (/api/v2/proof)
  participant SWR as Client Cache

  User->>UI: Open Waive dialog & submit reason/expires/approved_by
  UI->>API: POST /api/v2/proof/requirements/{reqId}/waive (body)
  API-->>UI: 200 OK (updated requirement)
  UI->>SWR: mutate(requirement or requirements list)
  SWR-->>UI: updated data
  UI-->>User: close dialog, refresh view
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 I hopped through types and API paths bright,

Badges and widgets tucked in plain sight,
Tables and waivers, dialogs that gleam,
Evidence stories stitched into a stream,
A little proof trail—hoppy delight! 🎀

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: adding PROOF9 requirements list, detail page, and dashboard widget to the web UI—all core additions present in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/proof9-web-ui

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link

claude bot commented Mar 20, 2026

Code Review — PR #461: PROOF9 Web UI

Good addition overall — clean component structure and consistent use of SWR/axios patterns. A few items to address before merging.

SWR cache key vs. axios param mismatch (medium priority)

In proofApi, the axios calls pass workspace_path as a param but the SWR keys in the page components use path= in the query string. The HTTP requests will use the correct param name (workspace_path), but the SWR cache keys won't match actual request URLs — this could cause stale cache entries or missed invalidation. Either align the keys to use workspace_path= or use a tuple key like ['/api/v2/proof/requirements', workspacePath].

WaiveDialog defined twice

The same inline component with identical fields (reason, expiry, approved-by) appears in both proof/page.tsx and proof/[req_id]/page.tsx. Any future change needs to be made in two places. Recommend extracting to web-ui/src/components/proof/WaiveDialog.tsx and exporting from the barrel index.ts, which fits the pattern already established by ProofStatusBadge.

Test Coverage

No unit tests included for the new components. The PR relies on the existing 347 passing tests, but those do not cover ProofStatusBadge rendering for each status value, ProofStatusWidget rendering (counts, error state), or proofApi method signatures. At minimum, a smoke test for ProofStatusBadge status variants would be quick to add and protect against regression.

Minor observations

Backend dependency: This PR assumes PR #459 is merged first. Consider noting whether this should be blocked or can be merged speculatively with graceful degradation when the API returns 404.

Empty state copy: The hint to use cf proof capture — confirm the command name is final before it lands in the UI.

30s SWR refresh on dashboard widget: Reasonable for a card, but if the proof endpoints are expensive (scanning evidence), consider whether the interval should be configurable or paused when the user is not on the dashboard.

Summary

SWR cache key / axios param mismatch — Medium — recommend fix before merge. WaiveDialog duplication — Low — nice-to-have. No new tests — Low — nice-to-have. Backend PR dependency — coordinate merge order.

The core implementation looks solid. Fix the cache key naming and consider extracting WaiveDialog, then this is good to go.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (5)
web-ui/src/types/index.ts (1)

309-313: Narrow by_status to proof statuses.

Record<string, number> lets typos in consumers compile. Partial<Record<ProofReqStatus, number>> keeps the missing-key behavior but preserves the three valid status keys.

♻️ Proposed fix
 export interface ProofRequirementListResponse {
   requirements: ProofRequirement[];
   total: number;
-  by_status: Record<string, number>;
+  by_status: Partial<Record<ProofReqStatus, number>>;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-ui/src/types/index.ts` around lines 309 - 313, The by_status property on
interface ProofRequirementListResponse is too loose (Record<string, number>) and
should be narrowed to the known proof status keys; update the type of
ProofRequirementListResponse.by_status to use Partial<Record<ProofReqStatus,
number>> (import/reference the ProofReqStatus enum/type if needed) so consumers
get compile-time safety while preserving optional/missing keys behavior.
web-ui/src/lib/api.ts (1)

40-45: Type listRequirements with ProofReqStatus.

The shared union already exists, but this method still accepts any string. Tightening it now prevents invalid filters from leaking into future callers.

♻️ Proposed fix
   ProofRequirement,
   ProofRequirementListResponse,
   ProofEvidence,
   ProofStatusResponse,
+  ProofReqStatus,
   WaiveRequest,
 } from '@/types';
   listRequirements: async (
     workspacePath: string,
-    status?: string
+    status?: ProofReqStatus
   ): Promise<ProofRequirementListResponse> => {

Also applies to: 573-576

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-ui/src/lib/api.ts` around lines 40 - 45, Change the untyped status
parameter on the listRequirements API helper to use the existing ProofReqStatus
union instead of a plain string: update the signature of
listRequirements(status: string | undefined) (and the other occurrence around
the second listRequirements definition) to accept status?: ProofReqStatus,
adjust any internal typings that build the query/filter to use that typed value,
and run/adjust call sites to satisfy the new type so invalid filter strings
cannot be passed.
web-ui/src/app/proof/[req_id]/page.tsx (3)

135-139: Consider handling evidence fetch errors explicitly.

The evidence SWR call doesn't destructure the error state. Currently, if the evidence fetch fails, users see "No evidence recorded yet" which may be misleading. Consider distinguishing between "no evidence" and "failed to load evidence."

♻️ Suggested approach
-  const { data: evidence, isLoading: evidenceLoading } =
+  const { data: evidence, error: evidenceError, isLoading: evidenceLoading } =
     useSWR<ProofEvidence[]>(
       workspacePath && reqId ? `/api/v2/proof/requirements/${reqId}/evidence?path=${workspacePath}` : null,
       () => proofApi.getEvidence(workspacePath!, reqId)
     );

Then in the render section (around line 236):

-              {!evidenceLoading && (!evidence || evidence.length === 0) && (
-                <p className="text-sm text-muted-foreground">No evidence recorded yet.</p>
+              {!evidenceLoading && evidenceError && (
+                <p className="text-sm text-destructive">Failed to load evidence.</p>
+              )}
+              {!evidenceLoading && !evidenceError && (!evidence || evidence.length === 0) && (
+                <p className="text-sm text-muted-foreground">No evidence recorded yet.</p>
               )}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-ui/src/app/proof/`[req_id]/page.tsx around lines 135 - 139, The useSWR
call for evidence (useSWR<ProofEvidence[]>) currently only destructures data and
isLoading; update it to also destructure error (e.g., const { data: evidence,
isLoading: evidenceLoading, error: evidenceError } = useSWR(...)) and then
change the render logic where evidence is displayed to distinguish three states:
loading (evidenceLoading), fetch error (evidenceError — show a “Failed to load
evidence” message and optionally the error message), and empty data (evidence is
empty — show “No evidence recorded yet”). Ensure the fetcher still uses
proofApi.getEvidence(workspacePath!, reqId) and keep the same SWR key check
(workspacePath && reqId ? ... : null).

292-297: Use Shadcn/UI Button component here as well.

This inline button should use the Button component for consistency with the rest of the UI, as noted in the earlier comment about form elements.

♻️ Suggested change
-                <button
-                  onClick={() => setShowWaiveDialog(true)}
-                  className="mt-3 rounded-md border px-3 py-1.5 text-sm hover:bg-muted"
-                >
+                <Button
+                  variant="outline"
+                  size="sm"
+                  className="mt-3"
+                  onClick={() => setShowWaiveDialog(true)}
+                >
                   Waive this requirement
-                </button>
+                </Button>

As per coding guidelines: "Web UI must use Shadcn/UI (Nova preset)".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-ui/src/app/proof/`[req_id]/page.tsx around lines 292 - 297, Replace the
native <button> used to trigger the waive dialog with the Shadcn/UI Button
component: call Button with the same onClick handler (setShowWaiveDialog(true)),
preserve the styling (e.g., className "mt-3 rounded-md border px-3 py-1.5
text-sm hover:bg-muted" or use an appropriate Button variant such as "ghost" to
match visuals), keep the inner text "Waive this requirement", and add/import the
Button symbol from the UI components module so the component compiles (ensure
Button is referenced where the current element was rendered and remove the old
<button> element).

64-108: Consider using Shadcn/UI form components for consistency.

The form uses plain HTML elements (<textarea>, <input>, <button>) with manual styling. Per project guidelines, the web UI should use Shadcn/UI components. This would ensure consistent styling and behavior with the rest of the application.

Additionally, form labels should use htmlFor with matching id attributes on inputs for accessibility.

♻️ Suggested refactor using Shadcn/UI components
+import { Button } from '@/components/ui/button';
+import { Input } from '@/components/ui/input';
+import { Textarea } from '@/components/ui/textarea';
+import { Label } from '@/components/ui/label';

Then replace form elements:

           <div>
-            <label className="mb-1 block text-sm font-medium">Reason *</label>
-            <textarea
-              className="w-full rounded-md border bg-background px-3 py-2 text-sm"
+            <Label htmlFor="waive-reason">Reason *</Label>
+            <Textarea
+              id="waive-reason"
               rows={3}
               value={reason}
               onChange={(e) => setReason(e.target.value)}
               placeholder="Why is this requirement being waived?"
             />
           </div>

Similarly for buttons:

-            <button
-              type="button"
-              onClick={onClose}
-              className="rounded-md px-4 py-2 text-sm text-muted-foreground hover:text-foreground"
-            >
+            <Button type="button" variant="ghost" onClick={onClose}>
               Cancel
-            </button>
-            <button
-              type="submit"
-              disabled={submitting}
-              className="rounded-md bg-primary px-4 py-2 text-sm text-primary-foreground hover:bg-primary/90 disabled:opacity-50"
-            >
+            </Button>
+            <Button type="submit" disabled={submitting}>
               {submitting ? 'Waiving…' : 'Waive requirement'}
-            </button>
+            </Button>

As per coding guidelines: "Web UI must use Shadcn/UI (Nova preset) with gray color scheme".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-ui/src/app/proof/`[req_id]/page.tsx around lines 64 - 108, Replace the
raw HTML inputs with the project's Shadcn/UI components (e.g., import and use
Label, Textarea, Input, Button from your ui components) for the fields bound to
state variables reason, expires, and approvedBy and for the actions; give each
control a stable id (e.g., id="reason", id="expires", id="approvedBy") and set
the matching Label htmlFor to improve accessibility; keep the existing
value/onChange bindings (value={reason}
onChange={(e)=>setReason(e.target.value)} etc.), preserve error rendering and
DialogFooter, and ensure the submit and cancel buttons use the Button component
with the same disabled/submitting logic and classes replaced by the Shadcn
Button props.
🤖 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/page.tsx`:
- Around line 210-212: The Link href builds a path with an unescaped requirement
id (Link href={`/proof/${req.id}`}) which can break when req.id contains
characters like / ? # %; update the Link to URL-encode the id (use
encodeURIComponent(req.id)) when constructing the href so the route becomes safe
for any req.id value and matches how the API expects encoded req IDs.
- Around line 192-243: The table wrapper currently uses overflow-hidden which
clips columns on small viewports; update the wrapper div (the one wrapping the
table) to use horizontal scrolling (e.g., overflow-x-auto) and give the table
element a minimum width (add a min-w utility like min-w-[...]) so the
eight-column layout can scroll horizontally; adjust classes on the table (the
element rendering data.requirements rows and using ProofStatusBadge /
setWaivedReq) to include the min-width while keeping existing styling.
- Around line 65-111: Replace the raw HTML inputs in the waiver form with
Shadcn/UI Form primitives and properly associate labels to controls: wrap the
form with Form (using the existing handleSubmit), replace each field block with
FormField/FormItem/FormLabel/FormControl tied to the same state variables
(reason/setReason, expires/setExpires, approvedBy/setApprovedBy) and ensure each
FormLabel uses htmlFor and the corresponding FormControl provides an id; keep
the DialogFooter, onClose cancel button, submit button (respecting submitting
state and error rendering) but use the shadcn Button/FormControl variants where
appropriate so labels are programmatically associated for WCAG accessibility and
keyboard navigation.

In `@web-ui/src/components/proof/ProofStatusWidget.tsx`:
- Line 9: The component ProofStatusWidget currently treats fetch failures as an
empty state because it checks only !data || data.total === 0; change the render
logic to first detect a fetch error (e.g., the error value returned by your data
hook/SWR/fetcher) and render an explicit error state when error is truthy and
there is no cached data, before falling back to the empty-state branch that
checks data.total === 0; modify the component's conditional order (check error
&& !data first) and use the existing identifiers (ProofStatusWidget, data,
error, data.total) so outages are shown as errors rather than "No requirements
captured yet."

---

Nitpick comments:
In `@web-ui/src/app/proof/`[req_id]/page.tsx:
- Around line 135-139: The useSWR call for evidence (useSWR<ProofEvidence[]>)
currently only destructures data and isLoading; update it to also destructure
error (e.g., const { data: evidence, isLoading: evidenceLoading, error:
evidenceError } = useSWR(...)) and then change the render logic where evidence
is displayed to distinguish three states: loading (evidenceLoading), fetch error
(evidenceError — show a “Failed to load evidence” message and optionally the
error message), and empty data (evidence is empty — show “No evidence recorded
yet”). Ensure the fetcher still uses proofApi.getEvidence(workspacePath!, reqId)
and keep the same SWR key check (workspacePath && reqId ? ... : null).
- Around line 292-297: Replace the native <button> used to trigger the waive
dialog with the Shadcn/UI Button component: call Button with the same onClick
handler (setShowWaiveDialog(true)), preserve the styling (e.g., className "mt-3
rounded-md border px-3 py-1.5 text-sm hover:bg-muted" or use an appropriate
Button variant such as "ghost" to match visuals), keep the inner text "Waive
this requirement", and add/import the Button symbol from the UI components
module so the component compiles (ensure Button is referenced where the current
element was rendered and remove the old <button> element).
- Around line 64-108: Replace the raw HTML inputs with the project's Shadcn/UI
components (e.g., import and use Label, Textarea, Input, Button from your ui
components) for the fields bound to state variables reason, expires, and
approvedBy and for the actions; give each control a stable id (e.g.,
id="reason", id="expires", id="approvedBy") and set the matching Label htmlFor
to improve accessibility; keep the existing value/onChange bindings
(value={reason} onChange={(e)=>setReason(e.target.value)} etc.), preserve error
rendering and DialogFooter, and ensure the submit and cancel buttons use the
Button component with the same disabled/submitting logic and classes replaced by
the Shadcn Button props.

In `@web-ui/src/lib/api.ts`:
- Around line 40-45: Change the untyped status parameter on the listRequirements
API helper to use the existing ProofReqStatus union instead of a plain string:
update the signature of listRequirements(status: string | undefined) (and the
other occurrence around the second listRequirements definition) to accept
status?: ProofReqStatus, adjust any internal typings that build the query/filter
to use that typed value, and run/adjust call sites to satisfy the new type so
invalid filter strings cannot be passed.

In `@web-ui/src/types/index.ts`:
- Around line 309-313: The by_status property on interface
ProofRequirementListResponse is too loose (Record<string, number>) and should be
narrowed to the known proof status keys; update the type of
ProofRequirementListResponse.by_status to use Partial<Record<ProofReqStatus,
number>> (import/reference the ProofReqStatus enum/type if needed) so consumers
get compile-time safety while preserving optional/missing keys behavior.
🪄 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: 8ba63948-0eec-45ee-bb98-9f640cec637b

📥 Commits

Reviewing files that changed from the base of the PR and between 695340f and f346d59.

📒 Files selected for processing (9)
  • web-ui/src/app/page.tsx
  • web-ui/src/app/proof/[req_id]/page.tsx
  • web-ui/src/app/proof/page.tsx
  • web-ui/src/components/layout/AppSidebar.tsx
  • web-ui/src/components/proof/ProofStatusBadge.tsx
  • web-ui/src/components/proof/ProofStatusWidget.tsx
  • web-ui/src/components/proof/index.ts
  • web-ui/src/lib/api.ts
  • web-ui/src/types/index.ts

- Narrow ProofRequirementListResponse.by_status to Partial<Record<ProofReqStatus, number>>
- Type listRequirements status param as ProofReqStatus instead of string
- ProofStatusWidget: show error state when fetch fails (vs silent empty state)
- proof/page.tsx: URL-encode req.id in Link href; overflow-x-auto on table; use Shadcn Button/Input/Textarea in waive dialog; htmlFor/id on labels
- proof/[req_id]/page.tsx: destructure evidenceError from useSWR; show error state for evidence failures; use Shadcn Button/Input/Textarea in waive dialog; htmlFor/id on labels; Button for waive trigger
@claude
Copy link

claude bot commented Mar 20, 2026

Follow-up Review — PR #461: PROOF9 Web UI

This review focuses on issues not yet covered by CodeRabbit or my earlier comment. The SWR cache key mismatch, WaiveDialog duplication, by_status typing, and encodeURIComponent items from prior reviews stand and should still be addressed.

approved_by field has no validation (low security risk)

In both WaiveDialog implementations, the approved_by field is sent to the API as a raw, un-sanitized string with no format constraints — no length limit, no character validation, and no indication to the user whether it expects a username, email, or free text. If the backend stores this verbatim and surfaces it in audit logs or the UI, unexpected values could cause display issues or confuse the audit trail. At minimum, add a maxLength attribute on the Input and document the expected format in the placeholder (e.g., 'GitHub username or team name'). The placeholder currently says 'Your name or handle' which leaves the format ambiguous.

ProofStatusResponse.requirements is an over-fetch in the dashboard context

In web-ui/src/types/index.ts, ProofStatusResponse includes a full requirements: ProofRequirement[] array alongside the summary counts. ProofStatusWidget only uses data.total, data.open, data.satisfied, and data.waived — it never reads the requirements array. If the backend response includes all requirement objects (which it appears to from the type), the dashboard widget is pulling down the full dataset every 30 seconds and discarding most of it. This is worth confirming against the actual /api/v2/proof/status endpoint response shape in PR #459. If the endpoint does return all requirements, the widget should call /api/v2/proof/status only if it returns just the counts, or use listRequirements with a limit parameter instead. If the status endpoint already returns only aggregate counts, then the requirements field on the interface is misleading and should be removed.

workspaceReady pattern adds a flicker without real benefit

Both page components use a two-step initialization pattern:

const [workspaceReady, setWorkspaceReady] = useState(false);
useEffect(() => {
  setWorkspacePath(getSelectedWorkspacePath());
  setWorkspaceReady(true);
}, []);
if (!workspaceReady) return null;

The return null on the first render suppresses any skeleton/loading state, which causes a layout shift when the page mounts. Given that getSelectedWorkspacePath() is a synchronous localStorage read, the workspaceReady flag only guards against SSR hydration mismatches. If that is the intent, the pattern is reasonable but should be documented with a comment. If it is not the intent, the pattern can be simplified to initializing workspacePath directly (with a null default) and removing the extra boolean flag, which would eliminate the layout shift.

Date input value format not validated before submission

The 'Expiry date' Input uses type="date", which gives a browser date picker and returns values in YYYY-MM-DD format. This value is passed directly to WaiveRequest.expires without any format validation or conversion. If the backend expects an ISO 8601 datetime (e.g., 2026-12-31T00:00:00Z) rather than a plain date string, the submission will fail silently (the catch block swallows the error and shows only 'Failed to waive requirement'). Confirm the expected format with the proof_v2.py API contract in PR #459 and either convert the date picker value before submission or document that YYYY-MM-DD is accepted.

Waive button shown on satisfied requirements

In proof/page.tsx and proof/[req_id]/page.tsx, the Waive button is conditionally rendered with req.status !== 'waived'. This means a 'satisfied' requirement can be waived from the UI. Whether this is intentional product behavior should be confirmed — if satisfied requirements should not be waiveable (or should require a different flow), the condition should be req.status === 'open' instead.

Summary of new findings

Issue File Priority
approved_by has no length or format constraint Both WaiveDialog instances Low
ProofStatusResponse.requirements over-fetches on dashboard ProofStatusWidget + types/index.ts Medium
workspaceReady pattern causes layout shift / needs comment proof/page.tsx, proof/[req_id]/page.tsx Low
expires date format not validated against API contract Both WaiveDialog instances Medium
Waive button shown on 'satisfied' status — confirm intent proof/page.tsx, proof/[req_id]/page.tsx Low

The over-fetch concern and the expires format issue are the two worth resolving before merge; the others are cleanup or product decisions.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
web-ui/src/app/proof/[req_id]/page.tsx (2)

124-134: SWR cache keys don't encode reqId, but fetchers do.

The SWR keys at lines 126 and 132 use the raw reqId in the URL pattern, while proofApi.getRequirement and proofApi.getEvidence internally encode it. If reqId contains special characters (e.g., /, ?), the cache key won't match subsequent requests with the same logical ID, potentially causing unnecessary refetches or stale cache hits.

Consider encoding reqId in the SWR key for consistency:

-      workspacePath && reqId ? `/api/v2/proof/requirements/${reqId}?path=${workspacePath}` : null,
+      workspacePath && reqId ? `/api/v2/proof/requirements/${encodeURIComponent(reqId)}?path=${workspacePath}` : null,
-      workspacePath && reqId ? `/api/v2/proof/requirements/${reqId}/evidence?path=${workspacePath}` : null,
+      workspacePath && reqId ? `/api/v2/proof/requirements/${encodeURIComponent(reqId)}/evidence?path=${workspacePath}` : null,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-ui/src/app/proof/`[req_id]/page.tsx around lines 124 - 134, SWR keys use
the raw reqId while the fetchers (proofApi.getRequirement and
proofApi.getEvidence) encode it, causing cache key mismatches for IDs with
special chars; update the useSWR key expressions to encode reqId (e.g., use
encodeURIComponent(reqId)) so the string passed as the key matches the encoded
ID the fetchers use, keeping the same null-when-missing guard with workspacePath
&& reqId to avoid running when values are absent.

22-109: Consider extracting shared WaiveDialog to a reusable component.

This WaiveDialog is nearly identical to the one in web-ui/src/app/proof/page.tsx. The only difference is the prop interface (reqId: string vs requirement: ProofRequirement). Extracting to @/components/proof/WaiveDialog.tsx would reduce duplication and ensure consistent behavior across both pages.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-ui/src/app/proof/`[req_id]/page.tsx around lines 22 - 109, Duplicate
WaiveDialog implementations should be consolidated into a single reusable
component to remove duplication; create a new component (e.g., export default
function WaiveDialog) in "@/components/proof/WaiveDialog.tsx" that contains the
existing dialog UI and logic (useState hooks, handleSubmit, proofApi.waive call,
dialog markup), accept props { reqId: string; workspacePath: string; onClose: ()
=> void; onSuccess: () => void; } and reuse the same WaiveRequest construction
and error/submitting handling, then replace the two inline implementations in
web-ui/src/app/proof/[req_id]/page.tsx and web-ui/src/app/proof/page.tsx with
imports of this component; if the other page currently passes a ProofRequirement
object, wrap it at the call site by passing requirement.id as reqId (or extend
the component props to accept requirement?: ProofRequirement and derive reqId
internally) so both pages use the single shared WaiveDialog implementation.
web-ui/src/app/proof/page.tsx (1)

113-127: Consider adding SWR error handling for stale data scenarios.

The current implementation shows the error banner when error is truthy, but if there's cached data from a previous successful fetch, both the error banner and the stale data could render simultaneously (lines 163-167 check only error, while lines 178+ check only data).

This may be intentional (showing stale data with an error indicator), but if not, consider prioritizing error over stale data display.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-ui/src/app/proof/page.tsx` around lines 113 - 127, The ProofPage
currently can show an error banner and stale cached data simultaneously because
the render checks `error` and `data` independently; update the render logic in
ProofPage to prioritize error over stale data by gating the data/list rendering
on the absence of `error` (e.g., only render the requirements list when `data &&
!error`) or alternatively only show the error banner when `error && !data` and
use `isLoading`/`isValidating` from the useSWR hook to refine behavior; locate
the rendering that references `error` and `data` (and the useSWR call using
workspacePath → `/api/v2/proof/requirements?path=${workspacePath}` and
proofApi.listRequirements) and change the conditional checks so stale `data` is
not shown when an active `error` exists.
🤖 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/app/proof/`[req_id]/page.tsx:
- Around line 124-134: SWR keys use the raw reqId while the fetchers
(proofApi.getRequirement and proofApi.getEvidence) encode it, causing cache key
mismatches for IDs with special chars; update the useSWR key expressions to
encode reqId (e.g., use encodeURIComponent(reqId)) so the string passed as the
key matches the encoded ID the fetchers use, keeping the same null-when-missing
guard with workspacePath && reqId to avoid running when values are absent.
- Around line 22-109: Duplicate WaiveDialog implementations should be
consolidated into a single reusable component to remove duplication; create a
new component (e.g., export default function WaiveDialog) in
"@/components/proof/WaiveDialog.tsx" that contains the existing dialog UI and
logic (useState hooks, handleSubmit, proofApi.waive call, dialog markup), accept
props { reqId: string; workspacePath: string; onClose: () => void; onSuccess: ()
=> void; } and reuse the same WaiveRequest construction and error/submitting
handling, then replace the two inline implementations in
web-ui/src/app/proof/[req_id]/page.tsx and web-ui/src/app/proof/page.tsx with
imports of this component; if the other page currently passes a ProofRequirement
object, wrap it at the call site by passing requirement.id as reqId (or extend
the component props to accept requirement?: ProofRequirement and derive reqId
internally) so both pages use the single shared WaiveDialog implementation.

In `@web-ui/src/app/proof/page.tsx`:
- Around line 113-127: The ProofPage currently can show an error banner and
stale cached data simultaneously because the render checks `error` and `data`
independently; update the render logic in ProofPage to prioritize error over
stale data by gating the data/list rendering on the absence of `error` (e.g.,
only render the requirements list when `data && !error`) or alternatively only
show the error banner when `error && !data` and use `isLoading`/`isValidating`
from the useSWR hook to refine behavior; locate the rendering that references
`error` and `data` (and the useSWR call using workspacePath →
`/api/v2/proof/requirements?path=${workspacePath}` and
proofApi.listRequirements) and change the conditional checks so stale `data` is
not shown when an active `error` exists.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 55b730bb-b084-4311-bdaf-ebb0e57f3e43

📥 Commits

Reviewing files that changed from the base of the PR and between f346d59 and 3d91439.

📒 Files selected for processing (5)
  • web-ui/src/app/proof/[req_id]/page.tsx
  • web-ui/src/app/proof/page.tsx
  • web-ui/src/components/proof/ProofStatusWidget.tsx
  • web-ui/src/lib/api.ts
  • web-ui/src/types/index.ts
✅ Files skipped from review due to trivial changes (1)
  • web-ui/src/types/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • web-ui/src/lib/api.ts

@frankbria frankbria merged commit 936d588 into main Mar 20, 2026
11 checks passed
@frankbria frankbria deleted the feat/proof9-web-ui branch March 20, 2026 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PROOF9: Web UI has no proof views (requirements, run results, waivers)

1 participant