Search UX tooltip and distinct /api/search error codes (#117)#126
Conversation
Add a search-window info tooltip, return structured 400/404/503/500 responses with machine-readable codes, validate query length and workspace hash, and show API error messages in the search UI.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds explicit Search API hardening and UI tooltip
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@api/search.py`:
- Around line 57-66: The _workspace_exists helper currently joins
client-controlled workspace_id values directly into os.path.isdir, which can be
abused with absolute paths or .. segments. Update _workspace_exists to
explicitly allow only known-safe workspace IDs before joining paths: keep the
existing "global" and "cli:" handling, and for regular IDs validate they match
the expected workspace hash/ID format before calling
os.path.join(workspace_path, workspace_id). Reject or return false for anything
outside that allowed pattern so workspace existence checks cannot be used as a
local path oracle.
- Around line 107-112: Move workspace discovery into the existing
structured-error guard in the search flow so failures are converted to the JSON
error contract. Specifically, in the request handling around workspace_filter,
resolve_workspace_path should be called inside the try block (the same guard
used by the search endpoint), and any discovery/storage exceptions should be
caught and routed through _search_error instead of escaping before the handler
can format a 500/503 response.
In `@templates/search.html`:
- Around line 17-24: The search help tooltip is not exposed to assistive
technologies because the trigger in search.html uses a focusable span with only
aria-label, while the guidance text is just visual content. Update the tooltip
trigger to a semantic control such as a button, and connect it to the help text
using aria-describedby with the tooltip content marked appropriately (for
example, role="tooltip") so screen readers can access the 30-day/full-history
guidance. Keep the existing search-info-tooltip, search-info-tooltip-text, and
search-info-icon structure as the anchor points while changing the accessibility
semantics.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fdfda060-7323-48ea-8cff-b4a8c4d5fc20
📒 Files selected for processing (4)
api/search.pystatic/css/style.csstemplates/search.htmltests/test_api_search.py
bradjin8
left a comment
There was a problem hiding this comment.
Please check the following issues.
[Optional] templates/search.html line 123-126:
result-count banner could mention undated chats when windowed ("last 30 days; undated chats included"). Tooltip covers discoverability; banner would reinforce after search.
windowNote when windowed now includes undated chats |
Summary
templates/search.htmlexplaining the default 30-day indexed window, undated-chat inclusion, and the all-history checkbox.except Exceptionhandler inapi/search.pywith narrowed handlers returning structured{"error", "code"}bodies for 400, 404, 503, and 500.errorfield from failed API responses.workspacehash validation (404 when unknown).Problem
Part A: Users did not understand why some search results were missing (30-day window, undated chats always included).
Part B:
/api/searchreturned generic 500 for all failures, preventing clients from distinguishing malformed input, missing workspace, index lock, and internal errors.Test plan
pytest tests/test_api_search.py -q(19 passed)pytest -q(533 passed, 21 skipped)mypy api/search.pysqlite3.OperationalErrorinternal_errorcodeTestSearchWindow/ happy-path behaviorOut of scope
Closes #117
Summary by CodeRabbit
New Features
Bug Fixes
/api/searchinput validation and returns structured, code-based error responses with clearer status handling.