Catalog: honor a bucket's Athena preferences in the workspace console (5217 stack 9/9)

[nl0]

Description

ui.athena.defaultWorkgroup is a per-bucket preference, and the Athena console
is workspace-global — so since the re-home the console had no bucket whose
preferences to read, and the setting silently stopped working. This is the
consumer half of the pair: PR 8 makes every legacy /b/:bucket/queries/athena...
URL carry the bucket as ?bucket=, and this PR makes the console read it, mount
that bucket's preferences, and hand ui.athena to the model.

The scope has to survive navigation, or it is worse than nothing. A reader
who lands scoped and then clicks anything in-console would drop the scope and
land in a different console than the one they left. Three link sites now carry
the query string forward: the breadcrumb back to Query Executions, each execution
row in History, and the Queries tab strip.

Two defects the scoped mount introduced, fixed here rather than shipped:

• It crashed the catalog. The unconditional <Model.Provider> this replaces
  kept the console outside any BucketPreferences.Provider, and that absence is
  what made the create-package Files panel inert there. Scoping re-establishes
  the provider; nothing re-established the inertness. So on
  /queries/athena/<wg>?bucket=<b>, Create package → "Add files from bucket"
  mounted BucketSelect, which read a :bucket route segment that route does
  not have and asserted it — and the nearest error boundary is the app root, so
  the throw replaced the whole catalog screen rather than failing the panel.
• It threw away the query you were typing. Branching the tree on ?bucket=
  made the console's mount identity depend on the param, so losing the scope
  remounted Model.Provider and discarded queryBody, the selected catalog and
  the selected database with it.

⚠️ A direction decision this PR does not take

Re-scoping a workspace-level console with ?bucket= is a direction call, and
no artifact in this repository makes it. Both readings are live and neither is
refutable from the checkout: this PR's (a per-bucket preference needs a bucket in
scope), and the alternative the original description never weighs (a
workspace-level default, leaving the console bucket-free). queryRedirects.jsx
states the re-home's premise in a comment, not a decision record; the console's
own state spec modelled no ?bucket= state before this PR; there are no linked
issues. A maintainer should rule before this merges. Everything below it in
the stack stands on its own either way — which is why it is last.

Review findings addressed

• f30 (thread)
  — the crash, above. The route param was only ever a fallback for
  BucketPreferences' own handle; it is treated as one now, with the picker's
  current selection as a last resort that cannot be absent. BucketSelect is
  exported so the mount can be driven from a route without the segment, and the
  test fails with the original assertion in place.
• f31 (thread)
  — the remount, above. BucketPreferences.Provider takes
  bucket: string | null and with no bucket serves the state a consumer sees
  outside any provider — no document, no handle, no update, no fetch — instead of
  the subtree being unmounted. The console mounts it either way, so there is one
  Model.Provider element at one position for both cases. A scoped console still
  waits for its document before mounting the model (the workgroup is seeded once,
  and a default arriving later would not displace it), so acquiring a scope is
  a mount, as it has to be; losing one is a re-render. The test counts mounts
  rather than renders.
• f2 — the same class as f31's scenario, at the navigation site the
  scope-carrying fix stopped short of: the tab strip linked to bare pathnames.
• f21 — the breadcrumb half of the scope fix had no test. Both in-console
  link sites are pinned now, in both directions.
• f17 — the new precondition was absent from both documents that describe
  the behaviour. The preferences reference now says when a bucket is in scope and
  when it is not; the console's state spec gets a section on the scope as a
  precondition rather than as state, and the optional param on each URL.
• f41 — the ?bucket= parse was duplicated byte-for-byte in the console
  wrapper and the tabulator table list, and a repo-wide search for
  get('bucket') found exactly those two. The earlier dismissal said "extract a
  useBucketScope() when a third appears"; this PR needed the scope in a second
  place in the wrapper, which is the third.

Recorded, not acted on:

• f15 — checked and refuted: the _: () => <Placeholder/> arm is not a
  terminal-error trap. The pre-existing wrapper matched _ → Placeholder
  identically.
• The review recorded its own depth limit, and one of the areas it did not read
  is the ?bucket= × ?table= tabulator deep-link interaction. Both params
  ride the same query string and both are now load-bearing; that interaction is
  worth a reviewer's attention here specifically.

Dependencies

Hardening review of master
(6167dd82) against the
26.7.4 pin (eda3016f) —
linked rather than named, so the cross-version seam can actually be read.

Beyond the original diff

Three files here are not in #5217 and arrive with the fold-ins:
Queries.tsx/Queries.spec.tsx (f2), utils/BucketPreferences/Provider.tsx
plus a spec (f31), PackageDialog/Inputs/Files/S3FilePicker.tsx plus a spec
(f30), and Athena/scope.ts (f41).

Verification

cd catalog && npx vitest run app/containers/Queries app/utils/BucketPreferences app/containers/Bucket/PackageDialog/Inputs/Files/S3FilePicker.spec.tsx

12 files, 141 tests. The whole catalog suite (183 files, 1720 tests) is green,
tsc --noEmit is clean, and oxlint app reports nothing.

Position in the stack

PR 9 of 9, based on
stack/5217-8-queries-legacy-redirect-scope.

Part of the split of #5217 asked for in
f27.
Last, for two reasons: it is the consumer of PR 8's producer, and it is the only
unit blocked on the direction ruling above. It also carries the crash, so it is
the one unit that should not be waved through.

TODO

• Unit tests
• Security: Confirm that this change meets security best practices and does not violate the security model
• Open: Confirm that this change doesn't break the Open variant
• Documentation
  • Markdown somewhere in docs/**/*.md that explains the feature to end users
  • Markdown docs for developers
• Changelog entry

🤖 Generated with Claude Code

Greptile Summary

The PR restores bucket-scoped Athena preferences in the workspace query console while preserving console state when scope changes.

• Reads the optional bucket query parameter through a shared scope hook and mounts bucket preferences around the Athena model.
• Preserves query parameters across Athena history, breadcrumbs, and query-console tabs.
• Makes the S3 file picker safe on routes without a bucket path segment.
• Adds focused tests, user documentation, developer documentation, and a changelog entry.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete changed-code failure remaining after reviewing scoped loading, navigation, and provider transitions.

The scoped provider avoids unscoped fetches, navigation retains the required bucket parameter, and the file picker now has a valid fallback when no bucket route segment exists.

Important Files Changed

Filename	Overview
catalog/app/containers/Queries/Athena/Athena.tsx	Adds stable bucket-preference scoping around the Athena model and delays scoped model initialization until preferences resolve.
catalog/app/utils/BucketPreferences/Provider.tsx	Supports an explicit unscoped mode without fetching or exposing an update target while retaining existing scoped behavior.
catalog/app/containers/Queries/Queries.tsx	Preserves the current query string across query-console tab navigation.
catalog/app/containers/Queries/Athena/History.tsx	Retains bucket scope when opening successful query executions.
catalog/app/containers/Bucket/PackageDialog/Inputs/Files/S3FilePicker.tsx	Removes the route-bucket assertion and safely falls back to the picker’s current bucket.

Sequence Diagram

sequenceDiagram
  participant U as User
  participant R as Queries Router
  participant B as BucketPreferences Provider
  participant A as Athena Model
  U->>R: "Open /queries/athena?bucket=my-bucket"
  R->>B: Mount with bucket scope
  B->>B: Load bucket preferences
  B->>A: Pass ui.athena preferences
  A-->>U: Render scoped Athena console
  U->>R: Follow execution, breadcrumb, or tab
  R->>R: Preserve query string and bucket scope

Loading

_{Reviews (1): Last reviewed commit: "docs(changelog): entry for the bucket-sc..." | Re-trigger Greptile}

[codecov]

Codecov Report

❌ Patch coverage is 80.55556% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 36.69%. Comparing base (388ded0) to head (5cbf066).

Files with missing lines	Patch %	Lines
catalog/app/utils/BucketPreferences/Provider.tsx	60.00%	6 Missing ⚠️
catalog/app/containers/Queries/Athena/Athena.tsx	88.88%	1 Missing ⚠️

Additional details and impacted files

@@                              Coverage Diff                               @@
##           stack/5217-8-queries-legacy-redirect-scope    #5267      +/-   ##
==============================================================================
+ Coverage                                       36.02%   36.69%   +0.66%     
==============================================================================
  Files                                             741      742       +1     
  Lines                                           23841    23858      +17     
  Branches                                         6438     6443       +5     
==============================================================================
+ Hits                                             8589     8754     +165     
+ Misses                                          13520    13383     -137     
+ Partials                                         1732     1721      -11     

Flag	Coverage Δ
catalog	36.69% <80.55%> (+0.66%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nl0]

Local review pass

Reviewed this layer on its own diff (stack/5217-8-queries-legacy-redirect-scope..stack/5217-9-athena-bucket-scope), not against master. Two findings — one accepted, one declined — plus the things I checked and deliberately left alone.

Accepted: the single-mount guarantee only held in one direction

ScopedConsole gated on scoped && preferences === undefined on every render, not just the first. Gaining a scope refetches the preference document, so prefs read unresolved again for that window, and the gate swapped Model.Provider for the placeholder — remounting the console and discarding the query being typed along with the selected catalog and database. That is exactly the loss the single mount exists to prevent, and it was reachable in two steps: the sidebar's Queries item links to a bare /queries, which drops the scope, and Back then restores it.

Fixed by gating only the first render, which is the one that matters — the workgroup resolved then is pinned into the URL by Model.Provider's own redirect, so a default arriving later cannot displace it in either direction. Athena.spec.tsx covered only the losing direction; it now covers gaining as well, and that new case fails against the previous code.

Declined: "Add bucket" in the source-bucket picker

Raised as: on the Athena console handle || { bucket: routeBucket ?? bucket } falls back to the picker's selected source bucket rather than the ?bucket= scope, so "Add bucket" would open the config for a bucket whose ui.sourceBuckets does not govern the list.

The two cannot diverge. handle is null exactly when the scoped bucket has no preference document, and in that case the parsed preferences fall back to createSourceBuckets([scopedBucket]) — so the list is [scopedBucket] and the selection is already that bucket, with nothing else selectable. Whenever the list holds more than one entry the document exists, handle is non-null, and the fallback is never consulted. Left as is.

Checked, unchanged

• The ?table= param does not leak through the forwarded query string. TabulatorTables' autofill effect consumes it (params.delete('table') + history.replace) on first application, so by the time any in-console link is clicked the search is just ?bucket=. Forwarding the whole search is consistent with AthenaRootRedirect, which also keeps { bucket, table }.
• The placeholder gate cannot hang. ui.athena is a required field of the parsed preferences and defaults to {}; the Err arm resolves to Ok(parse('', scope)), and local mode serves Ok too. So preferences === undefined only ever means the document has not resolved yet.
• Every in-console link forwards the scope. Beyond the ones changed here, the workgroup select and both redirects in model/state.tsx already carried location.search from an earlier layer.
• The CHANGELOG claim is accurate against the code. Precedence really is URL > localStorage > ui.athena default > first listed, and the workgroup list really is sorted.
• Not fixed, pre-existing: on an unscoped console the create-package dialog's file input still renders nothing, because Files.tsx maps Result.Init() to null. This layer improves the scoped case rather than regressing the unscoped one — with no bucket the provider serves precisely the state a consumer saw outside any provider before.

Also swept this layer's comments and its CHANGELOG entry down to the constraints they carry, dropping the why-of-change narration.

Verification on the final commit, from catalog/: npm run typecheck clean, npm run lint:app clean, npx vitest run (full suite) 181 files / 1728 passed / 1 skipped / 0 failed.