Skip to content

fix(ui): keep model picker search current across refresh and Escape - #140846

Merged
steipete merged 2 commits into
mainfrom
codex/qa2-19-model-picker-20260907
Sep 7, 2026
Merged

steipete merged 2 commits into
mainfrom
codex/qa2-19-model-picker-20260907

Conversation

@steipete

@steipete steipete commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor
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:

Catalog refresh before Catalog refresh after
An Anthropic search incorrectly shows a newly discovered GPT model The same search stays filtered after discovery
First Escape before First Escape after
New Session closes the picker on the first Escape New Session clears the query while retaining its picker
Chat uses the same clear-search behavior

Chat keeps its model picker open after clearing the query with Escape

@clawsweeper

clawsweeper Bot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation app: web-ui App: web-ui size: L maintainer Maintainer-authored PR labels Sep 7, 2026
@steipete
steipete marked this pull request as ready for review September 7, 2026 05:14
@clawsweeper clawsweeper Bot added P2 Normal backlog priority with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 7, 2026
@clawsweeper

clawsweeper Bot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed September 7, 2026, 2:00 AM ET / 06:00 UTC.

ClawSweeper review

What this changes

The 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
Reviewed head: 257ca7156935073a39f98d6e61ccc636c95a17b6

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused owner-level repair with meaningful browser regression coverage and no identified blocking defect.
Proof confidence 🌊 off-meta tidepool Not applicable: The maintainer-authored PR is exempt from the external-contributor proof gate. Its supplied browser evidence targets catalog refresh and Escape in the shared picker; linked screenshots could not be retrieved here, and reported test results were not independently rerun.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The maintainer-authored PR is exempt from the external-contributor proof gate. Its supplied browser evidence targets catalog refresh and Escape in the shared picker; linked screenshots could not be retrieved here, and reported test results were not independently rerun.
Evidence reviewed 7 items Current main still needs the repair: Current main updates search on input and opening, without the new render synchronization. Its window-capture Escape handler dismisses the picker before the component-level search-clear handler can run.
Latest release comparison: The supplied latest release, v2026.9.2, also contains the older search implementation without render synchronization; no shipped replacement was established.
Render synchronization and selection: The ref follows the open binding and schedules filtering after row rendering. Synchronization retains only an eligible highlight and updates the active descendant; existing disabled and locked selection guards remain intact.
Findings None None.
Security None None.

How this fits together

The 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]
Loading

Before merge

None.

Agent review details

Security

None.

PR surface

Source -9, Tests +147, Docs +2, Other -1. Total +139 across 10 files.

View PR surface stats
Area Files Added Removed Net
Source 6 240 249 -9
Tests 2 147 0 +147
Docs 1 2 0 +2
Config 0 0 0 0
Generated 0 0 0 0
Other 1 1 2 -1
Total 10 390 251 +139

Review metrics

None.

Technical review

Best 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.

Labels

Label justifications:

  • P2: This repairs bounded model-picker filtering and keyboard interaction defects in Chat and New Session.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The maintainer-authored PR is exempt from the external-contributor proof gate. Its supplied browser evidence targets catalog refresh and Escape in the shared picker; linked screenshots could not be retrieved here, and reported test results were not independently rerun.

Evidence

What I checked:

  • Current main still needs the repair: Current main updates search on input and opening, without the new render synchronization. Its window-capture Escape handler dismisses the picker before the component-level search-clear handler can run. (ui/src/pages/chat/components/chat-picker-overlay.ts:65, 05b0f349f4de)
  • Latest release comparison: The supplied latest release, v2026.9.2, also contains the older search implementation without render synchronization; no shipped replacement was established. (ui/src/pages/chat/components/chat-model-picker.ts:150, 3928bad9badf)
  • Render synchronization and selection: The ref follows the open binding and schedules filtering after row rendering. Synchronization retains only an eligible highlight and updates the active descendant; existing disabled and locked selection guards remain intact. (ui/src/pages/chat/components/chat-model-picker.ts:183, 257ca7156935)
  • Shared dismissal ownership: Search clearing now runs before picker dismissal, after tooltip and modal-navigation handling. Existing shared outside-click dismissal covers the removed New Session handler. (ui/src/pages/chat/components/chat-picker-overlay.ts:76, 257ca7156935)
  • Behavioral regression coverage: The added browser cases cover replacement catalog rows under an active query, accessible highlighting and Enter selection, plus two-stage Escape and focus restoration on both routes. Existing cases exercise nested account menus and tooltip precedence. Tests were inspected, not executed during this read-only review. (ui/src/e2e/chat-flow.model-picker-refresh.e2e.test.ts:310, 257ca7156935)
  • Captured proof and review continuity: The supplied PR snapshot reports before/after browser captures and focused validation, including the corrected ref ordering. Attempts to inspect the linked refresh and Escape screenshots failed with artifact retrieval errors. The previous review recorded no findings; its retrieval limitation is not a new contributor obligation. Comparing the earlier head locally was also limited by an unavailable blob, so the current implementation and verified introduced delta were used without inventing a historical comparison. (257ca7156935)

Likely related people:

  • Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Vyctor H. Brzezowski: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)
  • reviewed 2026-09-07T05:20:20.940Z sha a317465 :: blocked before merge. :: none

@steipete
steipete merged commit 6e91b35 into main Sep 7, 2026
136 checks passed
@steipete
steipete deleted the codex/qa2-19-model-picker-20260907 branch September 7, 2026 06:07
@steipete

steipete commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: web-ui App: web-ui docs Improvements or additions to documentation maintainer Maintainer-authored PR P2 Normal backlog priority with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: L status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant