feat(web-ui): filter, search, and sort for proof evidence history table - #528
Conversation
…table (#485) - Gate dropdown filters evidence by gate name (dynamic options from data) - Result dropdown filters by pass/fail - Search input matches run_id and artifact_path substrings - All five columns sortable by clicking headers (aria-sort for a11y) - Default sort: timestamp descending (most recent first) - Reset button clears all filters (only visible when filters active) - 21 new unit tests covering all filter/sort/reset combinations
WalkthroughAdded client-side filter, search, and sort UX for the proof evidence history table with session-persisted filter state and a comprehensive Jest + React Testing Library suite validating UI and table behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Page as ProofDetailPage
participant Session as sessionStorage
participant SWR as SWR/DataFetcher
participant API as Backend/API
User->>Page: Open /proof/[req_id]
Page->>Session: loadSessionFilters(sessionKey)
Page->>SWR: fetchEvidence(req_id)
SWR->>API: GET /evidence?req_id
API-->>SWR: evidence[]
SWR-->>Page: evidence[]
Page->>Page: compute gateOptions, filteredEvidence (apply gate/result/search + sort)
Page-->>User: render filter controls + sorted table
User->>Page: change filter / search / sort
Page->>Page: update filter state, recompute filteredEvidence
Page->>Session: saveSessionFilters(sessionKey)
Page-->>User: update table view
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 docstrings
🧪 Generate unit tests (beta)
Comment |
Review: PR #528 — Proof Evidence Filter/Sort/SearchClean implementation. The feature is well-scoped, the Bug: Index key on sorted/filtered rows
{filteredEvidence.map((ev, i) => (
<tr key={i} ...>Using the array index as a key was harmless when the order was stable, but now that rows can be reordered or filtered, React will reuse DOM nodes incorrectly — cells may retain stale state (e.g., focus) when the list changes. Use a stable identifier: {filteredEvidence.map((ev) => (
<tr key={`${ev.run_id}-${ev.gate}-${ev.timestamp}`} ...>A composite of UX gap: empty filtered-results stateWhen evidence exists but all rows are filtered out, the table renders with an empty {filteredEvidence.length === 0 && (
<tr>
<td colSpan={5} className="px-4 py-6 text-center text-sm text-muted-foreground">
No evidence matches the current filters.
</td>
</tr>
)}Accessibility: search input missing
|
![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/app/proof/[req_id]/page.tsx (1)
316-317: Use a stable row key instead of the filtered index.This table is explicitly re-sorted and re-filtered, so
key={i}changes meaning on every interaction. A key derived from immutable evidence fields will keep row identity stable as the order changes.♻️ One possible fix
- {filteredEvidence.map((ev, i) => ( - <tr key={i} className="border-b last:border-0"> + {filteredEvidence.map((ev) => ( + <tr + key={`${ev.req_id}:${ev.run_id}:${ev.timestamp}:${ev.gate}:${ev.artifact_path}`} + className="border-b last:border-0" + >🤖 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 316 - 317, The row key currently uses the mapped index (key={i}) inside the filteredEvidence.map call which breaks identity when the list is re-sorted or filtered; replace it with a stable, immutable identifier from each evidence object (e.g., ev.id or a deterministic combination like `${ev.source}-${ev.offset}`) in the <tr> generated in the filteredEvidence.map so row identity remains stable across reorder/filter operations; if an immutable id doesn't exist on the evidence objects, add one when the evidence is created/loaded (e.g., populate an `id` field) and use that `id` as the key.
🤖 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 49-56: The filter and search state (filterGate, filterResult,
search and their setters setFilterGate, setFilterResult, setSearch) are only
stored in component state and are lost when navigating away; persist them to
session-level storage by syncing with next/router searchParams or
window.sessionStorage: on mount initialize the three values from
searchParams/sessionStorage and whenever setFilterGate/setFilterResult/setSearch
are called update the searchParams and/or sessionStorage accordingly so state is
restored when returning to the page.
- Around line 13-14: Move the new TypeScript types SortCol and SortDir out of
page.tsx and into the centralized types module: add their definitions to
web-ui/src/types/index.ts, export them, then import and use them in
web-ui/src/app/proof/[req_id]/page.tsx (replace the inline type declarations
with imports of SortCol and SortDir). Ensure the exported names match exactly
and update any references in page.tsx to use the imported types.
- Around line 16-39: Replace raw HTML controls with Shadcn/UI components: update
the SortButton component to render the Shadcn Button (use the Button import)
instead of a plain <button>, swap the plain <select> elements used for Gate and
Result filtering to Shadcn Select, and replace the raw search <input> with the
Shadcn Input component; ensure you wire their props to the existing handlers
(retain SortButton props col, label, current, dir, onSort and call onSort(col)),
keep the Gate/Result select change handlers and values, and keep search
value/handler but add aria-label="Search run ID or artifact" to the search Input
so it is programmatically labeled. Use the Nova gray color styling conventions
from the Shadcn preset and Hugeicons for any icons (do not use lucide-react).
Ensure imports for Button, Select, Input (and Hugeicons) are added and
remove/replace any direct HTML controls in SortButton and the select/input
locations.
---
Nitpick comments:
In `@web-ui/src/app/proof/`[req_id]/page.tsx:
- Around line 316-317: The row key currently uses the mapped index (key={i})
inside the filteredEvidence.map call which breaks identity when the list is
re-sorted or filtered; replace it with a stable, immutable identifier from each
evidence object (e.g., ev.id or a deterministic combination like
`${ev.source}-${ev.offset}`) in the <tr> generated in the filteredEvidence.map
so row identity remains stable across reorder/filter operations; if an immutable
id doesn't exist on the evidence objects, add one when the evidence is
created/loaded (e.g., populate an `id` field) and use that `id` as the key.
🪄 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: 5b15b085-f660-41ae-b1b3-aa4a19c86825
📒 Files selected for processing (2)
web-ui/src/__tests__/components/proof/ProofDetailPage.test.tsxweb-ui/src/app/proof/[req_id]/page.tsx
| function SortButton({ | ||
| col, | ||
| label, | ||
| current, | ||
| dir, | ||
| onSort, | ||
| }: { | ||
| col: SortCol; | ||
| label: string; | ||
| current: SortCol; | ||
| dir: SortDir; | ||
| onSort: (col: SortCol) => void; | ||
| }) { | ||
| return ( | ||
| <button | ||
| aria-label={`Sort by ${label}`} | ||
| onClick={() => onSort(col)} | ||
| className="flex items-center gap-1 font-medium hover:text-foreground" | ||
| > | ||
| {label} | ||
| {current === col && <span className="text-xs">{dir === 'asc' ? '↑' : '↓'}</span>} | ||
| </button> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full file to understand context and verify the code locations
wc -l web-ui/src/app/proof/\[req_id\]/page.tsxRepository: frankbria/codeframe
Length of output: 105
🏁 Script executed:
# Read the file to see the actual code at lines 16-39 and 223-259
cat -n web-ui/src/app/proof/\[req_id\]/page.tsx | sed -n '1,50p'Repository: frankbria/codeframe
Length of output: 1844
🏁 Script executed:
# Check lines around 223-259
cat -n web-ui/src/app/proof/\[req_id\]/page.tsx | sed -n '200,270p'Repository: frankbria/codeframe
Length of output: 3620
🏁 Script executed:
# Check if Shadcn/UI components are available in the project
ls -la web-ui/src/components/ui/ | head -20Repository: frankbria/codeframe
Length of output: 981
🏁 Script executed:
# Verify the dependencies in package.json to confirm Shadcn/UI setup
cat web-ui/package.json | grep -A 20 -B 5 "shadcn"Repository: frankbria/codeframe
Length of output: 45
🏁 Script executed:
# Check package.json more carefully
cat web-ui/package.json | grep -E "(shadcn|hugeicons|lucide)" | head -20Repository: frankbria/codeframe
Length of output: 96
🏁 Script executed:
# Let's also check if there are any imports of lucide-react in the file
rg "lucide-react" web-ui/src/app/proof/\[req_id\]/page.tsxRepository: frankbria/codeframe
Length of output: 45
🏁 Script executed:
# Check if the file imports hugeicons
rg "hugeicons" web-ui/src/app/proof/\[req_id\]/page.tsxRepository: frankbria/codeframe
Length of output: 45
🏁 Script executed:
# Look at the Shadcn Input and Select components to see how they should be used
cat web-ui/src/components/ui/input.tsxRepository: frankbria/codeframe
Length of output: 841
🏁 Script executed:
# Check the Select component implementation
head -50 web-ui/src/components/ui/select.tsxRepository: frankbria/codeframe
Length of output: 1817
🏁 Script executed:
# Check if there's a Label component available
ls -la web-ui/src/components/ui/label.tsxRepository: frankbria/codeframe
Length of output: 144
🏁 Script executed:
# Check if there's a label component anywhere in the project
rg -i "label" web-ui/src/components/ui/ --type tsxRepository: frankbria/codeframe
Length of output: 90
🏁 Script executed:
# Let's see how the Button component is used elsewhere in the file
rg "Button" web-ui/src/app/proof/\[req_id\]/page.tsx -A 3Repository: frankbria/codeframe
Length of output: 1934
🏁 Script executed:
# Let's check the full context around the search input to see if it's wrapped in a label
sed -n '250,265p' web-ui/src/app/proof/\[req_id\]/page.tsxRepository: frankbria/codeframe
Length of output: 644
🏁 Script executed:
# Check if there are other uses of Input component in the project to see best practices
rg "Input" web-ui/src --type tsx | head -10Repository: frankbria/codeframe
Length of output: 90
🏁 Script executed:
# Let's check another page that might use Shadcn Input and Select properly
find web-ui/src/app -name "*.tsx" | head -10Repository: frankbria/codeframe
Length of output: 403
🏁 Script executed:
# Check if there are other examples of Shadcn Select usage in the codebase
rg "from.*@/components/ui/select" web-ui/src --type ts -A 5 | head -40Repository: frankbria/codeframe
Length of output: 1018
🏁 Script executed:
# Check if Input is used elsewhere
rg "from.*@/components/ui/input" web-ui/src --type ts -A 10 | head -50Repository: frankbria/codeframe
Length of output: 3912
Replace raw HTML controls with Shadcn/UI components and add proper labeling for the search field.
The sort button (lines 16–39), filter selects (lines 223–251), and search input (lines 253–259) all use raw HTML elements instead of Shadcn/UI. Replace SortButton to use the Shadcn Button component, the <select> elements with Shadcn Select, and the <input> with Shadcn Input to maintain consistent styling and design system compliance.
Additionally, the search input currently has no programmatic label—add aria-label="Search run ID or artifact" to it so screen readers can identify it properly. The Gate and Result selects already have aria-labels, but the search field needs one.
Per coding guidelines: "Web UI must use Shadcn/UI (Nova preset) with gray color scheme and Hugeicons (@hugeicons/react); never use lucide-react."
🤖 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 16 - 39, Replace raw
HTML controls with Shadcn/UI components: update the SortButton component to
render the Shadcn Button (use the Button import) instead of a plain <button>,
swap the plain <select> elements used for Gate and Result filtering to Shadcn
Select, and replace the raw search <input> with the Shadcn Input component;
ensure you wire their props to the existing handlers (retain SortButton props
col, label, current, dir, onSort and call onSort(col)), keep the Gate/Result
select change handlers and values, and keep search value/handler but add
aria-label="Search run ID or artifact" to the search Input so it is
programmatically labeled. Use the Nova gray color styling conventions from the
Shadcn preset and Hugeicons for any icons (do not use lucide-react). Ensure
imports for Button, Select, Input (and Hugeicons) are added and remove/replace
any direct HTML controls in SortButton and the select/input locations.
…lters - Use stable composite row key (req_id:run_id:timestamp:gate:artifact) instead of index - Move ProofEvidenceSortCol and SortDir types to types/index.ts - Replace SortButton plain <button> with Shadcn Button (ghost variant) - Replace search <input> with Shadcn Input component - Persist filter state to per-requirement sessionStorage key (survives navigation) - Clear sessionStorage between tests to prevent filter state leakage Note: kept native <select> for Gate/Result filters to maintain consistency with the established codebase pattern (ProofPage #483 also uses native selects) and compatibility with the fireEvent.change test interaction model.
|
Follow-up Review Both correctness items from my prior review have been addressed in the latest commit:
One open item remains: Still open: empty filtered-results state When evidence exists but all rows are filtered out, filteredEvidence is empty while evidence.length > 0, so neither the loading/error messages nor the 'No evidence recorded yet' copy fires. The table renders with a silent empty tbody. A single row inside the body closes this: This is the only remaining item. Everything else (filter logic, sort logic, session persistence, test coverage) looks good. Happy to approve once the empty-state case is handled. |
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 80-87: The effect hooked by useEffect reads reqId (via
loadSessionFilters(reqId)) but has an empty dependency array, so it should
include reqId to re-run when the route param changes; update the dependency
array for the useEffect that calls setWorkspacePath(getSelectedWorkspacePath()),
setWorkspaceReady(true), loadSessionFilters(reqId), setFilterGate(...),
setFilterResult(...), and setSearch(...) to include reqId (and any other stable
values like getSelectedWorkspacePath if it's not stable) so the filters are
reloaded when reqId 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: 890052e7-da90-41b3-a28b-946c6af7fb3f
📒 Files selected for processing (3)
web-ui/src/__tests__/components/proof/ProofDetailPage.test.tsxweb-ui/src/app/proof/[req_id]/page.tsxweb-ui/src/types/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- web-ui/src/tests/components/proof/ProofDetailPage.test.tsx
| useEffect(() => { | ||
| setWorkspacePath(getSelectedWorkspacePath()); | ||
| setWorkspaceReady(true); | ||
| const saved = loadSessionFilters(reqId); | ||
| setFilterGate(saved.gate); | ||
| setFilterResult(saved.result); | ||
| setSearch(saved.search); | ||
| }, []); |
There was a problem hiding this comment.
Missing reqId in the dependency array.
The useEffect references reqId (via loadSessionFilters(reqId)) but the dependency array is empty. If the user navigates between different requirements via client-side routing without a full remount, the effect won't re-run and filters from the previous requirement will persist incorrectly.
🐛 Proposed fix
useEffect(() => {
setWorkspacePath(getSelectedWorkspacePath());
setWorkspaceReady(true);
const saved = loadSessionFilters(reqId);
setFilterGate(saved.gate);
setFilterResult(saved.result);
setSearch(saved.search);
- }, []);
+ }, [reqId]);📝 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.
| useEffect(() => { | |
| setWorkspacePath(getSelectedWorkspacePath()); | |
| setWorkspaceReady(true); | |
| const saved = loadSessionFilters(reqId); | |
| setFilterGate(saved.gate); | |
| setFilterResult(saved.result); | |
| setSearch(saved.search); | |
| }, []); | |
| useEffect(() => { | |
| setWorkspacePath(getSelectedWorkspacePath()); | |
| setWorkspaceReady(true); | |
| const saved = loadSessionFilters(reqId); | |
| setFilterGate(saved.gate); | |
| setFilterResult(saved.result); | |
| setSearch(saved.search); | |
| }, [reqId]); |
🤖 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 80 - 87, The effect
hooked by useEffect reads reqId (via loadSessionFilters(reqId)) but has an empty
dependency array, so it should include reqId to re-run when the route param
changes; update the dependency array for the useEffect that calls
setWorkspacePath(getSelectedWorkspacePath()), setWorkspaceReady(true),
loadSessionFilters(reqId), setFilterGate(...), setFilterResult(...), and
setSearch(...) to include reqId (and any other stable values like
getSelectedWorkspacePath if it's not stable) so the filters are reloaded when
reqId changes.
1 participant
Closes #485
Summary
run_idandartifact_pathsubstrings (case-insensitive)aria-sortfor accessibilityTest plan
ProofDetailPage.test.tsxcovering all filter/sort/reset combinations__tests__/app/proof/req_id/page.test.tsxstill passSummary by CodeRabbit
New Features
Tests