Data products: seven follow-ups from the #5203 pre-merge review

[QuiltSimon]

Filed from the pre-merge /code-review on #5203. The three blocking findings were fixed on that branch (b690c2cb); these seven are real but independent, and #5203 was already 91 commits, so they are recorded here rather than grown into it.

Line numbers are against b690c2cb. Severity is the reviewer's, and I have re-checked each claim below.

---

1. The .. parent row is inert — Contents.tsx:226

inert = item.name === '..' strips role, tabIndex, onClick and onKeyDown. The justifying comment says "Listing already renders it as its own affordance" — it does not. Every row's name column goes through CellComponent (Listing.tsx:1167), and the '..' references there are sorting (929, 941), suppression (507, 1236) and row construction (103) — never a handler.

Effect: after entering a subdirectory, clicking .. does nothing and it is not keyboard-reachable, while format still computes a real to. Accessibility as well as usability.

2. DirGroup.fileCount counts directory markers — contents.ts:149

groupForPath counts an explicit raw/ marker as a file; totals, totalsForPath and refusedForPath all exclude markers via isDirMarker. This is the exact disagreement the file's comments claim to have unified.

Reproduced: ['raw/', 'raw/a.txt'] → fileCount: 2 vs totals() → 1. And ['raw/'] alone renders a raw folder row beside a "0 files" header; navigating in reports "nothing at this path" for a path the revision contains.

3. Duplicate filter terms share a React key — Buckets.jsx:471

Terms are neither deduped nor uniquely keyed. ?q=xyz+xyz renders two chips with key="xyz", and clicking either drops both (.filter((t) => t !== term)), clearing the whole filter — contradicting the per-term-drop rationale directly above it. Reachable exactly when NoMatch renders.

4. 33KB of fixtures ship to anonymous visitors — Buckets.jsx:20 / index.ts:49

export * as fixtures is not tree-shakeable against a namespace import. Buckets.jsx imports the barrel for one symbol (DP.useProducts), and Landing.jsx, Landing/BucketList.jsx and OpenLanding.jsx all import containers/Home/Buckets eagerly — so 33,393 bytes of fixture data reach every anonymous visitor to /.

Note the trap: the obvious fix (import from ./hooks directly) is what the barrel's own docstring discourages. Probably wants a fixtures entry point separate from the barrel.

5. DP card omits treatments its sibling gained — DataProductCard.tsx:38

BucketCard gained a prefers-reduced-motion transition kill, :active, and the whole-card ::after overlay (lines 46/58/61/83). DataProductCard has none. A reduced-motion user gets the preference honoured on bucket cards and ignored on DP cards in the same row, and DP body whitespace is dead where a neighbour's navigates. .impeccable/surfaces/home.md:88 asserts "The whole card is the target now" surface-wide.

6. Products match on a field nothing displays — Buckets.jsx:587

Products match on labels, which appears in containers/Home/ only at that line — no renderer shows it. A label-only hit produces a card containing no instance of the typed term. Compounding it, NoMatch says it searched "name, description, and tags" while counting products, which have labels and no tags.

7. PLATFORM_LABEL duplicated 6× and already diverged — BucketRows.tsx:203

Copy-pasted into six files. The admin screen distinguishes "Databricks Unity" from "Databricks Unity (Delta Sharing)"; the five user-facing copies collapse both to "Databricks Unity". The fixtures contain both kinds, and that line is a DP row's only platform signal (the slot a bucket uses for s3://) — so two rows read byte-identically. AUTH_METHODS gives unity-schema three auth methods and unity-share two, so they are genuinely different things.

Record<PlatformKind, string> catches a missing key, never a divergent value. Also unguarded: an unrecognized kind from a real adapter renders a blank line.

---

Below the reviewer's severity cap, recorded for completeness

• Buckets.jsx:542 — useFeature adds a settings-fetch suspension to the landing grid, which previously suspended only on the buckets query. Per features.ts's own comment, ResourceCache suspends on Init too, so this fires even in LOCAL mode where nothing is fetched. Anonymous OPEN visitors hold the skeleton for a settings round-trip on a page that may never enable DP.
• Contents.tsx:287 — refusedForPath is unmemoized and rescans the full manifest per render; its own comment concedes the cause ("a hook there would change hook order"). totalsForPath walks the identical prefix, memoized, 120 lines above. Design target is 100k–1M entries.
• Detail.tsx:156,229,282 — three M.Paper at MUI default elevation 1 against DESIGN.md:281 "do not add new resting shadows"; no MuiPaper override exists. Contents.tsx:41 does it right. Plus fontSize: '0.75rem' (:56) and style={{marginTop: 24}} (:282) where classes.section already resolves to 24px.
• BucketRows.tsx:215 — accessSummary is byte-identical to DataProductCard.tsx:120, each with its own 8-line docblock; two views of one grid with nothing enforcing identical text. And Buckets.spec.tsx never enables dataProductsEnabled in the view-toggle block, so row-dp: — a testid the spec itself defines at line 141 — is asserted nowhere. DataProductRow's body is untested.
• capabilities.ts:43 — DATAZONE.changeEvents: true contradicts the comment directly above it ("there is NO data-product event"). Inert today, but supportingPlatformCount returns 1 rather than 0, so a second platform flipping true would silently pass mayBranchOn.

---

Not covered by that review — worth a separate pass

• Generated artifacts excluded: schema.generated.ts (+1,153), types.generated.ts (+405). I separately verified gql:generate yields no diff and quilt3's Python client is byte-identical to master's, but the reviewer did not read them.
• shared/graphql/schema.graphql (+390) not reviewed for contract correctness. Noted: types.ts hand-maintains parallel types already diverging from the generated enums — ContentsSource has PACKAGE, DataProductContentsSource does not.
• fixtures.ts (862 lines) not audited for internal consistency.
• internals/dev/legacy-registry-shim.js (+214) skimmed only — referenced nowhere outside its own header, wired into no npm script, dead as committed. It re-implements ORDERING_TO_RESULT_ORDER from Search/model.ts:229. Worth deleting or documenting.
• No build and no browser run. Findings 2, 3 and the two fixed sort/crash bugs were reproduced by executing the logic in Node; the rest are code-path verified by reading.