feat: [ENG-3022] Render query results as a list in LocalWebUI#754
feat: [ENG-3022] Render query results as a list in LocalWebUI#754ncnthien wants to merge 3 commits into
Conversation
Parse the query-tool-mode task result into matched docs and render them as a list in the task-detail Result section: each row shows the title, a match-score badge with a "Match score" tooltip, the path, and the doc content with a Show more/less expander. Legacy or non-JSON results fall through to the existing markdown/HTML rendering. Adds parseQueryToolModeResult / isQueryToolModeType with unit tests.
Drop the EventLogSection from the LocalWebUI task-detail view for all task types and delete the now-unused task-detail-event-log component.
…input
- Decode the encoded {limit,query} content so the input box, panel title, and list-row title show the query instead of raw JSON (shared taskDisplayTitle).
- Make each query result path clickable: opens the file in the Context tab if it still exists, otherwise toasts (useNavigateToContextPath reuses findNodeByPath).
- Slim the result parser to return the matched-docs array; dedupe safeJsonParse and the stale-path toast message; drop the unused limit field.
- Remove the dead event-log helpers (build-event-timeline, task-detail-tool-call, EventDot/RAIL_BG/EventTone) orphaned by the EVENT LOG removal.
|
Claude finished @ncnthien's task in 5m 16s —— View job Code ReviewOverall score: 4 / 5 — solid, well-scoped refactor. Net –370 lines from removing the EVENT LOG + tool-call view, plus a clean structured-result panel for What works well
Issues / suggestionssuggestion — suggestion — nitpick — note — The local Correctness & safety
TestsPR description:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| return (path: string) => { | ||
| if (findNodeByPath(nodes, path)) { | ||
| navigate(`/contexts?path=${encodeURIComponent(path)}`) |
There was a problem hiding this comment.
suggestion (non-blocking): This hook always queries the default branch (no branch arg to useGetContextNodes()) and navigates to /contexts?path=… without a branch query param. Two side-effects worth thinking about:
- The existence check (
findNodeByPath(nodes, path)) runs against the default-branch tree, so a result whose path exists only on the user's currently-selected branch will toast as "stale" even though clicking through would have shown valid content (or vice versa). - If the user had previously switched branches in the Context tab, navigating from a task result silently drops them back to the default branch, since
/contexts?path=…is opened withoutbranch.
For the local-only query-tool-mode use case today this is fine, but mirroring the branch-resolution that useContextTree/useTopicViewerNavigation use would make the behavior consistent. Could read searchParams.get('branch') (or read it from a shared store) and forward it to both the fetch and the navigation URL.
| const el = bodyRef.current | ||
| if (!el) return | ||
| setOverflowing(el.scrollHeight > el.clientHeight + 1) | ||
| }, [doc.rendered_md]) |
There was a problem hiding this comment.
suggestion (non-blocking): The "Show more/Show less" toggle is only computed when doc.rendered_md changes — it won't re-evaluate if the panel/viewport resizes. A short markdown body that doesn't clip at 800px wide can clip at 500px (or vice versa) and the toggle won't appear/disappear correctly until the next mount.
A ResizeObserver on bodyRef.current (or on the parent panel) would keep overflowing accurate across resizes. Edge case for now, but the Tasks tab is in a resizable layout so it's reachable.
| import {SectionLabel, TerminalDot} from './task-detail-shared' | ||
|
|
||
| export function QueryResultsList({matchedDocs}: {matchedDocs: QueryToolModeMatchedDoc[]}) { | ||
| const label = `Result · ${matchedDocs.length} ${matchedDocs.length === 1 ? 'match' : 'matches'}` |
There was a problem hiding this comment.
nitpick (non-blocking): SectionLabel already accepts a count prop that renders the number on the right side of the divider line (used elsewhere in the file). Could lean on that instead of baking "matches" / "match" into the label string:
<SectionLabel count={matchedDocs.length}>Result</SectionLabel>If the inline "match/matches" pluralization was intentional (for readability), feel free to ignore.
| rendered_md?: string | ||
| score: number | ||
| title: string | ||
| } |
There was a problem hiding this comment.
note (non-blocking): The canonical envelope shape lives in src/server/core/interfaces/executor/i-query-executor.ts (QueryToolModeMatchedDoc) where format is 'html' | 'markdown' and rendered_md is required. The webui's local shape is intentionally weaker because it can't import from server/ (boundary rule), but the difference is worth being aware of: if the wire shape ever changes (e.g., a new format), only the parser test here will catch it — the server side won't fail typecheck against the consumer.
If a shared src/shared/types/ module ever picks up tool-mode envelopes, this is a candidate for de-duplication.
1 participant
Summary
Renders LocalWebUI
query-tool-modetask results as a scannable list instead of a raw JSON blob, and polishes the surrounding task-detail view.{limit,query}content so the INPUT box, panel title, and list-row title show the query instead of raw JSON.findNodeByPath+ stale-path convention.Testing
npm run lint && npm run typecheck && npm test— green (8920 passing, 0 failing).