feat(ui): filter timeline calls#22
Conversation
There was a problem hiding this comment.
Pull request overview
Implements client-side filtering controls for the UI timeline so users can narrow large call lists by tool/method substring and optionally show only errors, with an empty-state message when filters produce no rows.
Changes:
- Added reusable timeline filtering helpers (
filterTimelineCalls) and corresponding unit tests. - Updated the timeline UI to include a search box, an “errors only” toggle, and a “no matching calls” empty state.
- Introduced a small typed
readJson<T>helper for the UI API client to improve TypeScript inference aroundresponse.json().
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/timeline-filter.ts | Adds filtering helpers used by the timeline UI. |
| ui/src/style.css | Styles new timeline filter controls and empty-state row. |
| ui/src/app.tsx | Wires filtering UI into the timeline table and renders empty state. |
| ui/src/api.ts | Adds typed JSON helper for fetch responses. |
| tests/timeline-filter.test.ts | Adds Vitest coverage for timeline filtering behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import type { Call } from "./api.js"; | ||
|
|
||
| export function callLabel(call: Call): string { | ||
| return call.toolName ?? call.method; |
| if (!normalizedQuery) { | ||
| return true; | ||
| } | ||
| return callLabel(call).toLowerCase().includes(normalizedQuery); |
| it("matches tool or method names case-insensitively", () => { | ||
| expect(filterTimelineCalls(calls, "GITHUB", false).map((entry) => entry.id)).toEqual(["1"]); | ||
| expect(filterTimelineCalls(calls, "resources", false).map((entry) => entry.id)).toEqual(["2"]); | ||
| }); |
|
The hasText guard resolves all three review points cleanly, and your new tests — tools/call matching tool rows, plus the empty-toolName fallback, lock the behavior in. The filter logic reads correctly. One gap remains before this merges: you fixed the label logic inside the helper but left the two places that actually render it, so filtering and display now disagree. app.tsx:136 (the timeline cell) and app.tsx:185 (the CallDetail heading) still call c.toolName ?? c.method directly. You already export callLabel for exactly this case, so import it and route both sites through callLabel(c). That closes the display edge case and removes the duplicated ?? logic in one move. One nit you can ignore: !filteredCalls.length shows "no matching calls" even for a genuinely empty session, not just a filtered-out one. |
What & why
Adds the timeline filtering requested in #10:
no matching callswhen filters hide every rowI also added a small typed JSON helper in the UI API client so the current TypeScript gate treats
response.json()results as the expected response types.Closes #10.
Checklist
npm run checkpasses (same gate as CI)feat(scope): ...)