Skip to content

fix: restore the redraws the macOS 13 port dropped and clear main's red test suite - #2940

Merged
datlechin merged 18 commits into
mainfrom
fix/observable-regressions-and-red-ci
Sep 17, 2026
Merged

datlechin merged 18 commits into
mainfrom
fix/observable-regressions-and-red-ci

Conversation

@datlechin

@datlechin datlechin commented Sep 17, 2026

Copy link
Copy Markdown
Member

macOS Tests has failed on every completed run of main since v0.74.0. This clears what those runs reported, and most of what they reported was a real regression in the app rather than a stale test.

One regression behind most of it

#2874 (macOS 13 support) converted every @Observable class to ObservableObject. Observation tracks any property a view body reads. ObservableObject publishes only @Published properties, and only to a view that holds the object as @ObservedObject or @StateObject. Wherever the conversion missed one half, the view stopped redrawing:

Surface What broke Why
Query history drawer Cmd+Y left the drawer shut until something else redrew the window; scope, date, outcome and source filters did the same six persisted HistoryPanelState properties had no @Published
History detail a clicked row stayed on "No Query Selected" HistoryPanelView keeps its model in @State, which subscribes to nothing, and read the selection from there
Query plan diagram zooming changed the canvas and left the percentage and the zoom buttons as they were DiagramZoomToolbar held its viewport as a plain property; the ER diagram only hid it because its own toolbar observes the same object
Highlight rules on a table tab Add Rule did nothing visible table rules live in HighlightRuleStorage, which nothing observed; its _ = revision read is an Observation idiom that does nothing here
Structure tab a staged column, index or foreign key did not appear and Save stayed disabled TableStructureView observes its session, but a change inside the session's changeManager never fires the session's publisher
Import from AWS region progress rows never left pending the progress step held the session as a plain property
Settings > General Last checked and Check for Updates kept the values they had when Settings opened updater held as a plain property

None of these shipped, since #2874 is unreleased, so they carry no CHANGELOG entries.

Also fixed

  • Cmd+Return with the autocomplete list open inserted the highlighted completion instead of running the query. SuggestionController.handleKeyDown runs from a local key monitor, which AppKit consults before the main menu, and it switched on the key code alone. A chord with a real modifier now passes through. This one shipped in 0.74.0 and has a Fixed entry.
  • TrinoStatementClientTests.testCancelStopsPolling raced its own stub. cancel() releases the statement with a DELETE fired from a detached task, and the stub served that DELETE from the queue of canned pages. When it won, it took the page the in-flight GET was waiting for; the GET got the empty default, read it as the last page and returned before the loop checked for cancellation. Reproduced by giving the DELETE a 50 ms head start: the old stub fails 3 of 3, the fixed one passes 3 of 3. ClientBox is also locked now, since it crosses threads.
  • Two storage suites asserted storeIsTrusted, which needs an integrity key from the system keychain the CI test host cannot reach. Both stores now take a ConnectionStoreIntegrity, defaulting to .shared, and the tests pass one backed by StoredIntegrityKeySource over an in-memory keychain. The sandboxed key path now uses the same type.
  • OffMainActorHandlerGuardTests: the SQLite agent heartbeat handler was missing @Sendable.

Tests that described old behaviour

  • Keyword case. The completion ranking tests expected uppercase labels, but completions now follow the case typed, so they passed on a Mac set to UPPERCASE and failed on CI. They pin the setting now.
  • Measured transports. .remoteDatabaseSession registers a byte counter, so it belongs in the measured set.
  • Menu fixture. capableContext() never set hasRowSelection or canDropSelectedTables.
  • Array fallback. A 33 character nested array literal falls back to the single-line editor, not the multi-line one.
  • UI tests:
    • query-execute-menu and "Execute All Statements" became query-run-menu and "Run All Statements" in the Run split button.
    • hasFocus raises NSInternalInconsistencyException on macOS, so focus is read from the element's own Keyboard Focused attribute instead.
    • Keystrokes sent right after a click could beat the editor's focus, so EXPLAIN QUERY PLAN arrived as IN QUERY PLAN. typeQuery now confirms the text arrived.
    • click() on a history row, which AppKit publishes as disabled, selects it without a mouse-down, so the list never took the keyboard. Those rows are clicked by coordinate now.
    • The AWS regions are checkboxes labelled with name and id, not separate static texts.

Verification

  • Unit and package suites: every suite main reported red passes locally, plus the full TableProEditor package.
  • UI suites:
    • These pass locally: QueryHistoryPanelUITests, WindowFocusUITests, QueryPlanResultUITests, DiagramKeyboardZoomUITests, HighlightRulesUITests, ImportFromAWSUITests, StructureForeignKeyEditUITests (twice), GridTimeZoneOffsetUITests and ResultMapModeUITests.
    • The six cases that reported "The sample database never finished opening" pass locally. They look like runner contention.
    • QueryHistoryFocusUITests and the Run in New Tab case need this run to confirm: a concurrent xcodebuild on the same Mac broke the automation session partway through.
  • Flaky before this change: testCommandEqualsZoomsAClickedERDiagram fails about one run in four on main too.

Audit of the rest of #2874

Every class the port converted was checked against its pre-port declaration, and every SwiftUI body that reads one of the shared objects was checked for observation.

  • Stored properties that were tracked before and have no @Published now. Of 37, four are read by a view:
    • PluginManager's plugin collections, so Settings > Plugins and the rejected-plugin banner did not follow an install, update or rejection. The banner also held the manager as a constant.
    • LicenseManager.isValidating, so Check Again was never disabled during a check.
    • MainContentCoordinator.inspectorRowContentRevision, whose onChange could never fire, so the inspector's JSON view went stale while a value window edited the row. It is now a PassthroughSubject with onReceive, not a published counter: publishing it would have redrawn every view observing the coordinator on each keystroke.
  • Bodies reading a shared object they never observe. Observation tracked a Foo.shared.bar read in a body on its own; ObservableObject does not. 79 observations across 71 files now cover what they read: ThemeEngine 32, AppSettingsManager 21, LicenseManager 8, DatabaseManager 8, PluginManager 5, MCPServerManager 2, and one each for SchemaService, CustomSlashCommandStorage and TeamLibrarySyncCoordinator. Only reads that run while building the body count: reads in initializers and in action closures (onChange, task, button actions) are left alone, and so is MainContentView, whose reads build a sheet at presentation time. Without this, changing the editor or data grid font, sidebar display options, keyboard shortcuts or the license left open views as they were, and the welcome window's connection status did not follow a connect or disconnect.
  • QueryCompletionProfileRegistry was designed so a body depended on one scope's revision box and nothing else. After the port nothing observed the box, so a schema invalidation reached the editor only on the next keystroke. The editor now subscribes to that scope's box with onReceive. It does not observe the registry, which publishes on every fetch.

Lint

MainContentCommandActions was at 1102 lines against the 1100 limit, which failed SwiftLint on main and would have failed the release tag's lint job. The switcher commands moved to MainContentCommandActions+Switchers.swift.

@datlechin
datlechin merged commit 1b907dd into main Sep 17, 2026
10 of 12 checks passed
@datlechin
datlechin deleted the fix/observable-regressions-and-red-ci branch September 17, 2026 10:16
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