Skip to content

fix(datagrid): stop a header click made during a run from replacing the next Run's statement - #3118

Merged
datlechin merged 1 commit into
mainfrom
fix/sort-rerun-outlives-run
Sep 24, 2026
Merged

datlechin merged 1 commit into
mainfrom
fix/sort-rerun-outlives-run

Conversation

@datlechin

Copy link
Copy Markdown
Member

What was wrong

A header click on a query result handed its re-run to runQuery through a field on the tab, pagination.sortExecutionOverride, and runQuery read that field before it looked at the caret. The field was only cleared when runQuery consumed it, or by resetLoadMore().

While a run is in flight, the grid keeps showing the previous result, and its headers stay clickable. So a click during a run did this:

  1. handleSortStateChanged wrote the re-run to the field and changed the tab's sort state.
  2. runQuery returned at its isExecuting guard and left the field set.
  3. Nothing cleared it when that run ended without applying a new result: Stop, a failure, and a result over the row cap all leave pagination alone.
  4. The next Run sent the stored re-run in place of the statement at the caret.

The field is older than #3114. On main before #3114, that stale re-run was the old editor text plus ORDER BY. #3114 made a parameterized re-run carry the values its result ran with and skip the parameter panel. So after #3114 the leftover also froze the old values: with :m set to bye in the panel, Run sent ... WHERE msg = ? ORDER BY msg ASC bound to hello, with nothing on screen to say so. A reviewer found this on #3114 after it merged.

What changed

  • sortExecutionOverride is gone. handleSortStateChanged hands the re-run straight to executeResolvedSQL, which is the same path it reached through runQuery before: Safe Mode gate, missing-value check, driver placeholders, history. runQuery now only runs the selection or the statement at the caret.
  • A header click on a query result while its tab is running is dropped. The sort state stays as it was, and the header is drawn from the sort state, so the chevron does not move. This check runs before the "Discard Unsaved Changes?" prompt, so a click that cannot run no longer asks the user to throw away edits first.
  • The check runs again when the prompt returns, together with a check that the clicked tab is still the selected one. The run itself goes to the selected tab, so a sort must not be sent to another tab.

The first test below fails on main without these changes.

Tests

MainContentCoordinatorSortTests.headerClickDuringRunLeavesNothingForTheNextRun, with two arguments: the in-flight run is stopped (stopExecution), or its claim settles without a result (how a failure or a truncated result ends). It uses the recording-driver harness from #3114:

  1. Run SELECT id, msg FROM audit WHERE msg = :m with m = hello.
  2. Claim the tab with beginTabExecution, then click the msg header.
  3. Assert that nothing was sent and the sort state is unchanged.
  4. End the claim, set the panel to bye, then press Run.
  5. Assert that the run sent the statement with no ORDER BY, bound to ["bye"].

On main (ea0caa3), both cases fail. The sort state becomes msg ASC, and Run sends SELECT id, msg FROM audit WHERE msg = ? ORDER BY `msg` ASC bound to ["hello"]. I got that by running the new test against main's source before changing it.

PaginationStateTests: the test for resetLoadMore() clearing the override now covers the query and its values, since the override no longer exists. Three in-place sort tests lose their sortExecutionOverride == nil line. Their rows-sorted-in-place assertions are what show that no re-run was sent.

Verification

  • verify.sh test MainContentCoordinatorSortTests PaginationStateTests TabQueryIsolationTests FetchAllQueryTaskTests PaginationCoordinatorTests: PASS, 93/93 (before the rebase)
  • After rebasing onto 78a9c73: verify.sh generate PASS, verify.sh build PASS, and the same suites plus TabQueryContentEqualityTests: PASS, 100/100
  • verify.sh lint on the four changed Swift files: 0 violations. The step reports FAIL only for a stale path in .claude/skills/fix-issue/references/verification.md (/Applications/Xcode-beta.app), which is older than this change.

Not covered

  • No UI test. A deterministic test would need a run still in flight when the header is clicked, and on the bundled SQLite sample that comes down to timing. UI automation also cannot start on this Mac: it needs a password prompt answered.
  • No SQL Server run. The defect is in app state, not in anything the driver does.
  • A table tab has the same shape and is left alone here. A header click during a table load changes the sort state and rebuilds the table query, then runQuery bails. The load in flight returns rows in the old order under the new chevron, until the next load runs the rebuilt query. Nothing stale is replayed there, because the rebuilt query matches the sort state on screen. It is older than fix(datagrid): sort a query result by re-running its own statement with the values it ran with #3114.

Found while fixing #3078 (#3105). Follow-up to #3114.

@datlechin
datlechin merged commit a8fd27e into main Sep 24, 2026
6 of 8 checks passed
@datlechin
datlechin deleted the fix/sort-rerun-outlives-run branch September 24, 2026 14:13
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.

1 participant