Repository navigation
fix(ui): keep model picker search current across refresh and Escape - #140846
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. Reviewed September 7, 2026, 2:00 AM ET / 06:00 UTC. ClawSweeper reviewWhat this changesThe PR keeps model searches filtered during catalog refreshes and makes Escape clear the search before closing the picker in Chat and New Session. Merge readiness✅ Ready for maintainer review The PR remains useful: current main retains both reported failure paths. The reviewed changes address them coherently, and no blocking correctness or security finding was identified. Priority: P2 Review scores
Verification
How this fits togetherThe Control UI model picker receives catalog updates and keyboard input in Chat and New Session. It filters available models and passes explicit selections to the existing session controls. flowchart TD
A[Model catalog updates] --> C[Shared model picker]
B[Search and keyboard input] --> C
C --> D[Filter and highlight current results]
D --> E[Explicit model selection]
B --> F[Shared Escape handling]
F --> G[Clear search or close and restore focus]
Before mergeNone. Agent review detailsSecurityNone. PR surfaceSource -9, Tests +147, Docs +2, Other -1. Total +139 across 10 files. View PR surface stats
Review metricsNone. Technical reviewBest possible solution: Keep search reconciliation and Escape precedence in the shared picker owner, preserving existing selection guards and focus behavior. Do we have a high-confidence way to reproduce the issue? Yes, from source: refresh an open filtered catalog with replacement rows, or press Escape in its nonempty search field. Current main exposes both mechanisms; this review did not execute them. Is this the best way to solve the issue? Yes. Reapplying the retained query after rendering and moving clearing into the existing dismissal owner directly repair the failures without adding a competing control path. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 05b0f349f4de. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
|
Merged via squash.
|
…penclaw#140846) * fix(ui): keep model picker search current across refresh and Escape * fix(ui): apply model picker ref after its open binding
Additional instructions
MUST: Keep Allow edits from maintainers enabled for this PR so maintainers can help update the branch when needed.
What Problem This Solves
Fixes an issue where users searching the model picker would see unrelated models when the catalog refreshed in the background. Escape also closed the entire picker before clearing the search in both Chat and New Session.
Why This Change Was Made
The shared picker interaction owner now reapplies the current query after model rows render and preserves an eligible highlighted choice. Shared Escape handling clears the search before dismissing the picker, replacing the duplicate New Session dismissal path while retaining tooltip and nested-menu precedence.
User Impact
Users can keep searching while model discovery finishes, then select the matching result with Enter. With the search field focused, the first Escape clears its query and keeps the picker open; the next Escape closes it and returns focus to the trigger. The behavior is documented alongside the composer controls.
Evidence
Both failures were reproduced independently against the original code. The repair passed all 19 tests in the affected model-refresh, model-controls, and New Session catalog browser suites, plus 377 focused unit tests, the owning E2E test type graph, production UI types, full-file lint, and the full changed-file gate. Independent P0–P2 review was scoped-clean. The verified change was rebased patch-identically onto current main; published-head CI is tracked below. Production code is nine lines smaller overall, with exact shrink-only assertion-baseline updates.
The initial CI run found the same control-ownership inventory mismatch in two jobs. The follow-up correction moves the existing ref binding after the class and open bindings while preserving the inventory guard. All three inventory cases and the three focused browser regressions pass, with a fresh scoped-clean P0–P2 review. CI now tracks corrected head
257ca7156935073a39f98d6e61ccc636c95a17b6.All five linked captures were visually inspected. They contain only synthetic test UI and show the retained catalog filter and clear-search-before-dismiss behavior, covering the image-inspection gap in the earlier automated review.
Synthetic screenshots show the actual browser flow before and after the repair:
Chat uses the same clear-search behavior