Skip to content

Fix start-up races behind the command palette snapshot flakes - #1191

Merged
tconbeer merged 1 commit into
mainfrom
claude/palette-snapshot-flakes
Oct 18, 2026
Merged

tconbeer merged 1 commit into
mainfrom
claude/palette-snapshot-flakes

Conversation

@tconbeer

Copy link
Copy Markdown
Owner

Refs #1190. This doesn't close it: one of the issue's failure modes is still unexplained. See Still open below.

What are the key elements of this solution?

Two app races, plus test waits that didn't cover the state their snapshots read.

  • The Limit checkbox could start checked with no limit configured (run_query_bar.py, app.py). This is a user-facing bug that shipped. Building the limit Input posts a Changed message, and handle_new_limit_value checks the box for any valid value. If that message arrived after Harlequin.on_mount → apply_configured_limit(), the box ended up checked and queries were cut to the default 500 rows.
    • The Input is now built under self.prevent(Input.Changed), and the Checkbox starts with value=query_limit is not None. apply_configured_limit() is gone.
    • Validation still runs, and later edits still post Changed; test_run_query_bar covers both.
  • The location prompt sometimes opened with the whole path selected (catalog_source_screen.py). Textual delivers the auto-focus Focus event as a later message, and Input.select_on_focus defaults to True. Whenever the value was filled in before that event was handled, all of it ended up selected. That happened both in the app, where on_mount fills in the current location on reopen, and in the test.
    • The location input now sets select_on_focus = False, matching the history screen's filter input. That also restores what the trailing separator in on_mount is for: the cursor sits after the path, so autocomplete offers what is inside it.
  • The tests now wait for the state they read (test_command_palette.py):
    • The Theme Picker wait used to pass after the first keystroke, because "n" already ranks nord first. It now also requires the input to read "nord" with exactly one option.
    • submit_location waits for the prompt's is_mounted, since on_mount is what creates location_input.
    • test_show_files_command waits for the Files tab to be shown and active as well as listing the directory, instead of asserting the tab state straight after a wait on the listing.

Why did you design your solution this way? Did you assess any alternatives? Are there tradeoffs?

  • How it was reproduced: CI's snapshot reports couldn't be downloaded from the investigating environment, and the tests pass 100/100 locally under normal conditions. Both the implementer and an independent reviewer reproduced the races by adding a random 0–10 ms delay before each message a pump handles. One probe also weakened wait_for_idle, which approximates Windows, where process_time ticks at about 15.6 ms.
  • Results, 40 runs each:
    • Limit box checked with no limit configured: 3/40 before, 0/40 after.
    • Reopened Show Files prompt with the path fully selected: 23/40 before, 0/40 after.
    • The hide-tabs assertion in test_show_files_command: 12 failures in 200 fuzzed runs before, 0 after.
    • With no delays, both trees are deterministic.
  • Tradeoff: tabbing back into the location field no longer selects its text.
  • Upstream: PathInput in textual-textarea doesn't forward Input keywords such as select_on_focus. Setting Textual's public attribute after construction works today, so an upstream change isn't needed for this fix.
  • No snapshots changed, and none were regenerated.

Still open

  • The Theme Picker snapshot failing on its own (CI's [True, False] on macOS 3.14 and Windows 3.12) isn't root-caused.
    • The tighter wait is correct, but the reviewer found that every failing Theme Picker frame in the fuzz runs already satisfied it.
    • One stale CommandList-height frame turned up once in about 400 fuzzed runs and couldn't be reproduced since.
  • Start-up focus race (not fixed here): the Data Catalog is focused at start-up through both Textual's AUTO_FOCUS and DataCatalog.focus_on_widget_in_active_pane. If the tree handles that focus before the editor mounts, HarlequinTree.on_focus sets cursor_line = 0, and the first catalog row stays highlighted.
    • It's rare and cosmetic, but it can fail these snapshots.
    • Fixing it needs a decision on how the main screen chooses initial focus. A naive AUTO_FOCUS = "" breaks test_history_screen. It belongs in its own PR.

Does this PR require a change to Harlequin's docs?

  • No.
  • Yes, and I have opened a PR at tconbeer/harlequin-web.
  • Yes; I haven't opened a PR, but the gist of the change is: ...

Did you add or update tests for this change?

  • Yes. The waits in test_command_palette.py now cover the state each snapshot and assertion reads. The existing limit tests (test_a_configured_limit_is_in_force_from_the_first_query, test_no_configured_limit_leaves_the_box_unchecked, test_run_query_bar) still pass. The races are timing-dependent, so they have no deterministic regression test.
  • No, I believe tests aren't necessary.
  • No, I need help with testing this change.

Please complete the following checklist:

  • I have added an entry to CHANGELOG.md, under the [Unreleased] section heading. That entry references the issue closed by this PR.
  • I acknowledge Harlequin's MIT license. I do not own my contribution.

Generated by Claude Code

The Limit checkbox could start checked when no limit was configured: the
limit Input's initial Changed reached its handler after the app had set the
box, so queries were cut to 500 rows. The Input is now built without posting
that message and the box starts in the configured state.

The Show Files and Show S3 prompts selected their whole location whenever the
input's focus landed after its value was filled in, so a reopened prompt
sometimes opened with the path selected. The location input no longer selects
on focus.

The palette tests now wait for the state their snapshots and assertions read:
the theme picker listing only "nord", the location prompt mounted, and the
Files tab shown and active.

Refs #1190

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M8ADtp77YK31uWbip8tFWd
@tconbeer
tconbeer merged commit 01f3565 into main Oct 18, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants