Skip to content

fix(sidebar): list favorite tables from every schema of the browsed database - #3200

Open
datlechin wants to merge 5 commits into
mainfrom
fix/favorites-tab-all-schemas
Open

datlechin wants to merge 5 commits into
mainfrom
fix/favorites-tab-all-schemas

Conversation

@datlechin

@datlechin datlechin commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

Found while implementing #3167 (PR #3189).

What was wrong

The Favorites tab looked each starred table up in one list: SchemaService.tables(for:), the flat table list of the loaded scope, handed down by SidebarView. A favorite that list did not hold was dropped by a compactMap.

  • On schema-grouped engines (PostgreSQL, Redshift, CockroachDB, SQL Server, DuckDB, SurrealDB) that list holds only the browsed schema, because the adapter's fetchTables() resolves to the driver's current schema. A favorite in any other schema of the same database never listed.
  • On hierarchical-schema engines (Oracle, Snowflake, BigQuery, Trino, Spanner, Dameng, R2 SQL, SAP HANA) the load commits an empty flat list and keeps every table in per-schema state, so every table favorite vanished. With nothing else visible, the tab fell through to the "No Favorites" onboarding view for someone who had starred tables.
  • The lookup could not tell "not loaded yet" or "refreshing" from "dropped", so the docs' promise (hide only a table that is missing) did not hold.

What changed

  • FavoriteTableResolver (pure) decides each favorite from every table list loaded for the browsed database:
    • a current list covering the favorite's schema decides: present means a verified row, absent means hidden;
    • a stale list only lends the type and icon to an unverified row;
    • a favorite no current list covers stays on the list, unverified, so a load or refresh never blanks it;
    • a favorite from another database is counted, not listed.
  • FavoriteTableCatalogReader builds that catalog from SchemaService (the flat list and per-schema lists) and DatabaseTreeMetadataService (tree table lists and the all-schema listing). It ignores SchemaService while its loaded database differs from the browsed one, so a database switch in flight cannot vouch for or hide anything.
  • Schemas no source has listed at all are loaded once: DatabaseTreeMetadataService.loadTables on schema-grouped engines, SchemaService.loadSchemaObjects on hierarchical ones, the same lists the tree expands into. Nothing reloads on staleness, so the per-COMMIT refetch SchemaService removed stays removed.
  • Opening a favorite calls openTableTab(name, schema:, database:) as Open Quickly does, so the browsed schema never moves. A favorite that no current list vouches for has its schema's list loaded first; if its kind still cannot be learned, the tab opens read-only.
  • A favorite outside the browsed schema shows its schema beside its name. View ER Diagram opens that schema's diagram, and is left out when the diagram key cannot name the schema (no browsed database, or a schema literally called default). The key is now built in one place, ERDiagramViewModel.schemaKey(databaseName:schema:).
  • Removal removes the stored entry exactly; it used to rebuild a key from TableInfo.schema plus the browsed database. Row ids and the persisted selection come from the entry.
  • Search goes through SidebarSearch.matchesObject, as Recents do: plain text matches names, sales. lists one schema, sales.ord narrows inside it.
  • New empty states for when every table favorite is in another database, or every one's table is gone, instead of the onboarding view.
  • Deleted the dead favoriteTableContextMenu(_:); the outline owns the menu through FavoritesMenuSpec.
  • Docs: docs/features/favorites.mdx, Table favorites.

Tests and verification

  • FavoriteTableResolverTests (new, 20): cross-schema verified row with caption, hierarchical per-schema list, unlisted schema kept unverified, current list hides, current list beats a stale listing that still holds the table, stale list keeps and lends its type, other database excluded, same name in two schemas, sort order, system schema absent from the listing versus a current per-schema list, flat engine, plain and qualified search, schemas needing load, only verified writable kinds open editable.
  • FavoriteTableCatalogReaderTests (new, 10) over a seeded SchemaService: a hierarchical favorite with an empty flat list and a loaded per-schema list is listed; missing is hidden; unloaded schema requested, loaded one not; nothing requested while disconnected; database switch in flight refused; schema-grouped flat list covers only the browsed schema; flat engine decides the whole database; open carries the verified kind; unlearnable and stale kinds open read-only. Four of these fail with the catalog reduced to today's flat-list lookup (hierarchicalFavoriteIsListed, unloadedSchemaIsRequested, databaseSwitchInFlightIsRefused, bySchemaFlatListCoversTheBrowsedSchema).
  • FavoritesEmptyStateTests, FavoritesMenuSpecTests, FavoritesOutlineSelectionTests, ERDiagramSchemaKeyTests updated and extended.
  • StringCatalogIntegrityTests, EllipsisConventionTests, SidebarScopeControlTests pass with the three new strings added to the catalog.
  • Build, SwiftLint (0 violations over 18 files) and the docs checks pass.

No UI test. A smoke test on the SQLite sample cannot fail before this fix (SQLite's flat list already holds every table), cross-schema needs a server, and starring a table writes through FavoriteTablesStorage, which defaults to UserDefaults.standard and so escapes the UI-test sandbox into the machine's real defaults. The screenshot on the page shows favorite databases only, so it still matches.

Screenshots

None. The change shows only on engines that group tables by schema, and staging favorites for a shot means starring tables in the app, which FavoriteTablesStorage writes to the real defaults domain even under the UI-test sandbox, so it would land in this machine's own favorites and sync. The SQLite sample lists the same rows before and after. The docs page's only screenshot shows favorite databases, so it still matches.

Review

Codex review (review-mumrl1bw-iszv7j), fixed:

  • [P1] An unlisted favorite opened with isView false and no kind, so a view could open as an editable table. Opening now loads the schema's list first and falls back to read-only.
  • [P2] View ER Diagram on a connection with no database name built a key like .SALES that the diagram cannot read back, so it drew the browsed schema. The item is left out wherever the key cannot name the schema, and the key has one builder.

Codex adversarial review (review-mumto9by-nozam5):

  • Fixed, [high]: a stale list's type could make a tab editable. Only a verified kind opens editable; opening an unverified favorite waits for a running refresh, then loads its schema's list, and stays read-only if that still does not vouch for it.
  • Dismissed, [high] "hierarchical favorites without an embedded schema": the only writer keys an entry on TableInfo.schema, and every per-schema fetch goes through PluginDriverAdapter.mapPluginTable, which fills schema: table.schema ?? schemaFallback with the schema it fetched. A nil-schema entry on those engines is not produced; if one existed it would show unverified and open by name, where today it is dropped.
  • Dismissed, [medium] "retained snapshots mislabeled as current": see Not fixed.

Second Codex review (review-mumuf9jo-lweojy):

  • Fixed, [P2]: a delayed open could land on a coordinator already torn down, or let an older click win over a newer one. The open task is held, cancelled by the next open, and checks isTearingDown on a weak coordinator after the await.
  • Dismissed, [P2] "track freshness before treating cached lists as authoritative": the same point as the adversarial medium finding, see Not fixed.
  • Dismissed, [P2] "load only table metadata for favorite schemas": SchemaService.loadSchemaObjects for hierarchical engines is deliberate. It fills the per-schema lists the tree expands into, so the work is shared rather than repeated; it runs once per schema no source has listed and never again on staleness; SchemaService has no tables-only per-schema load to share.

Not fixed

  • Favorites of other databases still list only when that database is browsed, as the docs say, while MCP list_favorite_tables lists every entry. openTableTab(database:) already opens across databases without a switch, so a grouped all-databases view is possible later; it needs table lists loaded for those databases.
  • A failed refresh keeps the previous snapshot in SchemaService and in the tree lists with no signal that it is stale, so Favorites treats it as current, exactly as the Tables list and the tree display it. A durable freshness signal belongs in those services and would fix all three readers at once.

CI fixes

The failed Package Tests job was SyncRecordMapperTests.unknownWireValueFailsClosed: the sync mapper's duplicate decoder turned an unknown Safe Mode value into Off. It now uses SafeModeLevel(wireValue:isReadOnly:), requiring confirmation while retaining legacy read-only restrictions. Added regressions for unknown read-only values and renames that preserve an unknown wire value.

Integrated main, preserving both sets of changelog entries. Fixed the confirmed SurrealDB compile error introduced by the integration: text called a length-taking JSON helper that had become a property. Restored that helper, retaining valid display truncation and complete export text.

Validation for these fixes:

  • Full swift test --package-path Packages/TableProCore: passed (1,197 Swift Testing tests plus XCTest).
  • SwiftLint on the changed Swift files: zero violations. The helper's documentation check still flags four pre-existing stale paths: TableProApp/rust-dameng, Contents/MacOS, Contents/Helpers, and /Applications/Xcode-beta.app.
  • Project regeneration: passed.
  • The three updated branches have identical SurrealDB sources and driver tests. An isolated SwiftPM build of all those plugin sources and the actual driver test file passed 50 tests in 8 suites on ci: retry test-product downloads and fix shared CI failures #3216. The identical corrected helper also passed an AllPlugins Xcode build earlier on fix(plugin-surrealdb): BETWEEN, REGEX and IS EMPTY filters become equality checks #3218.

GitHub Actions rerun after pushing these fixes.

@mintlify

mintlify Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
TablePro 🟢 Ready View Preview Sep 30, 2026, 4:03 PM

💡 Tip: Enable Automations to automatically generate PRs for you.

This branch was successfully deployed

1 active deployment
staging - docs — ab7f7f6c Deployed Sep 30, 2026 by mintlify[bot]
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