feat(webui): turn empty findings and palette results into guidance - #421
Conversation
Priority 2 of dec.webui-design-direction (#305): - The clean findings drawer shows the server-owned time of the successful scan currently being served, plus a copyable cairn scan CTA. Cached-scan fallback retains the prior time; retries clear stale browser metadata. The HTTP UI wire moves to schema version 4. - Command palette results expose Enter affordances that match the active action, with combobox/listbox semantics and active-descendant state. - Pinned Biome 2.4.4 formatting is applied across ui assets so local verification matches CI. todo.webui-guidance-deadends is done; #305 remains open for priority 3.
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe WebUI now reports the cached scan reconciliation time, displays it in the findings empty state, and updates the metadata schema to version 4. The command palette adds ARIA result semantics and contextual Enter hints. Generated symbol metadata and several non-functional UI formatting changes were also updated. ChangesWebUI guidance
Generated metadata and cleanup
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
map.json (1)
4044-4121: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRemove duplicated symbols from the generated metadata.
The symbols for
cairn.rootare duplicated 7 times in total in this array (this block is a duplicate of the one starting at line 3746). This duplication also occurs in other modules with multiple entries in theirpathsarray, such ascairn.lsp(duplicated 2 times) andcairn.mcp(duplicated 2 times).This indicates a bug in the metadata generator, likely appending the module's symbols once for each path in the module's
pathsarray instead of deduplicating them. Please fix the underlying generator and regeneratemap.jsonto ensure data integrity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@map.json` around lines 4044 - 4121, Fix the metadata generator so each module’s symbols are emitted once regardless of how many entries appear in its paths array, deduplicating symbols before appending them to the generated output. Verify this for modules such as cairn.root, cairn.lsp, and cairn.mcp, then regenerate map.json with no duplicate symbol blocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/ui_assets/command-palette.js`:
- Around line 151-158: Update the command-palette hint template around shown and
activeIdx so it renders exactly one Enter hint: use the report-specific copy
when the active item’s __action is "report-issue", otherwise use the default
Enter copy. Remove the separate second hint row while preserving the existing
visibility condition.
In `@src/ui_assets/style.css`:
- Line 2464: Replace the hardcoded padding value near the style rule containing
`padding: 1px 6px` with the appropriate spacing variables from `tokens.css`,
using the closest matching tokens for both vertical and horizontal spacing while
preserving the current layout intent.
---
Outside diff comments:
In `@map.json`:
- Around line 4044-4121: Fix the metadata generator so each module’s symbols are
emitted once regardless of how many entries appear in its paths array,
deduplicating symbols before appending them to the generated output. Verify this
for modules such as cairn.root, cairn.lsp, and cairn.mcp, then regenerate
map.json with no duplicate symbol blocks.
🪄 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 Plus
Run ID: 2203c0df-4bce-4022-8703-f527d4a9fd37
⛔ Files ignored due to path filters (14)
tests/snapshots/wire_format_snapshots__api_blueprint.snapis excluded by!**/*.snaptests/snapshots/wire_format_snapshots__api_dependents_app_api.snapis excluded by!**/*.snaptests/snapshots/wire_format_snapshots__api_depends_app_api.snapis excluded by!**/*.snaptests/snapshots/wire_format_snapshots__api_graph.snapis excluded by!**/*.snaptests/snapshots/wire_format_snapshots__api_lint.snapis excluded by!**/*.snaptests/snapshots/wire_format_snapshots__api_meta.snapis excluded by!**/*.snaptests/snapshots/wire_format_snapshots__api_node_app_api.snapis excluded by!**/*.snaptests/snapshots/wire_format_snapshots__api_node_app_api_contract.snapis excluded by!**/*.snaptests/snapshots/wire_format_snapshots__api_node_app_api_decisions.snapis excluded by!**/*.snaptests/snapshots/wire_format_snapshots__api_node_app_api_rationale.snapis excluded by!**/*.snaptests/snapshots/wire_format_snapshots__api_node_app_api_research.snapis excluded by!**/*.snaptests/snapshots/wire_format_snapshots__api_node_app_api_sources.snapis excluded by!**/*.snaptests/snapshots/wire_format_snapshots__api_node_app_api_todos.snapis excluded by!**/*.snaptests/snapshots/wire_format_snapshots__api_status.snapis excluded by!**/*.snap
📒 Files selected for processing (15)
docs/design-system/copy.tomlmap.jsonmeta/todos/todo.webui-guidance-deadends.mdsrc/ui/mod.rssrc/ui/server.rssrc/ui/server/meta.rssrc/ui_assets/app.jssrc/ui_assets/canvas-chrome.jssrc/ui_assets/command-palette.jssrc/ui_assets/findings-panel.jssrc/ui_assets/graph-canvas.jssrc/ui_assets/style.csssrc/ui_assets/utils.jstests/graph_explorer.rstests/wire_format_snapshots.rs
| ${ | ||
| q !== "" && shown.length > 0 | ||
| ? html`<div class="cmd-palette-hint"> | ||
| <span class="hint-row"><kbd>Enter</kbd> ${copy("webui.palette-hints.enter")}</span> | ||
| ${shown[activeIdx]?.__action === "report-issue" ? html`<span class="hint-row"><kbd>Enter</kbd> ${copy("webui.palette-hints.report")}</span>` : null} | ||
| </div>` | ||
| : null | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the Enter keyboard hints mutually exclusive.
Currently, when the active item is the report-issue action, both the default Enter hint and the report-specific Enter hint are rendered simultaneously and stack vertically. Since pressing Enter performs exactly one action for the focused item, displaying two different instructions for the same key is confusing.
Conditionally render a single hint string based on the active item instead of rendering a second row.
💡 Proposed fix
- ${
- q !== "" && shown.length > 0
- ? html`<div class="cmd-palette-hint">
- <span class="hint-row"><kbd>Enter</kbd> ${copy("webui.palette-hints.enter")}</span>
- ${shown[activeIdx]?.__action === "report-issue" ? html`<span class="hint-row"><kbd>Enter</kbd> ${copy("webui.palette-hints.report")}</span>` : null}
- </div>`
- : null
- }
+ ${
+ q !== "" && shown.length > 0
+ ? html`<div class="cmd-palette-hint">
+ <span class="hint-row">
+ <kbd>Enter</kbd>
+ ${shown[activeIdx]?.__action === "report-issue" ? copy("webui.palette-hints.report") : copy("webui.palette-hints.enter")}
+ </span>
+ </div>`
+ : null
+ }📝 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.
| ${ | |
| q !== "" && shown.length > 0 | |
| ? html`<div class="cmd-palette-hint"> | |
| <span class="hint-row"><kbd>Enter</kbd> ${copy("webui.palette-hints.enter")}</span> | |
| ${shown[activeIdx]?.__action === "report-issue" ? html`<span class="hint-row"><kbd>Enter</kbd> ${copy("webui.palette-hints.report")}</span>` : null} | |
| </div>` | |
| : null | |
| } | |
| ${ | |
| q !== "" && shown.length > 0 | |
| ? html`<div class="cmd-palette-hint"> | |
| <span class="hint-row"> | |
| <kbd>Enter</kbd> | |
| ${shown[activeIdx]?.__action === "report-issue" ? copy("webui.palette-hints.report") : copy("webui.palette-hints.enter")} | |
| </span> | |
| </div>` | |
| : null | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ui_assets/command-palette.js` around lines 151 - 158, Update the
command-palette hint template around shown and activeIdx so it renders exactly
one Enter hint: use the report-specific copy when the active item’s __action is
"report-issue", otherwise use the default Enter copy. Remove the separate second
hint row while preserving the existing visibility condition.
| background: var(--stone-3); | ||
| border: 1px solid var(--seam-thin); | ||
| border-radius: var(--r-edge); | ||
| padding: 1px 6px; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use design system tokens for spacing.
As per coding guidelines, UI work must derive visual values from tokens.css rather than hardcoding them. This principle applies to spacing as well. Consider replacing the hardcoded pixel padding (1px 6px) with the appropriate spacing tokens (e.g., var(--s-0) or var(--s-1)) to ensure consistency with the design system.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ui_assets/style.css` at line 2464, Replace the hardcoded padding value
near the style rule containing `padding: 1px 6px` with the appropriate spacing
variables from `tokens.css`, using the closest matching tokens for both vertical
and horizontal spacing while preserving the current layout intent.
Source: Coding guidelines
Part of #305 (priority 2 of three; #305 remains open for the topbar unit).
Implements dec.webui-design-direction's "dead ends become guidance":
cairn scanCTA. Cached-scan fallback retains the prior timestamp; boot retries clear stale metadata./api/metaaddslast_reconciled(epoch milliseconds), so the private webui wire moves uniformly from schema v3 to v4; every endpoint snapshot is rebased. The shared Query API/CLI/MCP wire is unchanged.Verification: full pre-archive gate; pinned Biome; token + size gates; strict Cairn scan; webui harness
ux_defect_score=0, 11/11 scenarios. Sequential simplicity then adversarial review applied; final reviewer verdict SHIP.