Skip to content

fix(rag): deploy app-layer public owner sentinel for anonymous search - #291

Merged
BigSimmo merged 4 commits into
mainfrom
cursor/reconcile-retrieval-sentinel-5c94
Jul 5, 2026
Merged

fix(rag): deploy app-layer public owner sentinel for anonymous search#291
BigSimmo merged 4 commits into
mainfrom
cursor/reconcile-retrieval-sentinel-5c94

Conversation

@BigSimmo

Copy link
Copy Markdown
Owner

What this fixes

The Supabase migration for public-only retrieval (retrieval_owner_matches + sentinel UUID) is already live on Clinical KB Database. This PR adds the matching application-layer changes so production anonymous search actually sends that sentinel to hybrid RPCs.

Without this, production RAG either:

  • throws (no ownerId), or
  • would send nullowner_filter (searches all owners — unsafe)

With this PR, anonymous global search with allowGlobalSearch: true sends owner_filter = 00000000-0000-0000-0000-000000000000, which the DB interprets as public documents only (owner_id IS NULL).

Changes

  • src/lib/owner-scope.tsPUBLIC_OWNER_FILTER_SENTINEL + retrievalOwnerFilter()
  • src/lib/rag.ts — threads sentinel through all hybrid RPC calls
  • src/lib/deep-memory.ts, src/lib/document-enrichment.ts — same scoping
  • supabase/schema.sql + migration file — repo parity with live DB
  • scripts/check-retrieval-owner-migration.ts + npm run check:retrieval-owner
  • Tests updated in tests/owner-scope.test.ts and tests/public-access-deep.test.ts

Deploy steps

  1. Merge this PR to main
  2. Vercel (or your host) auto-deploys the new app build
  3. Verify: npm run check:retrieval-owner (already passing against live Supabase)

Live corpus status: 1,935 indexed public documents; sentinel search returns results.

Open in WebOpen in Cursor

…tinel
Wire retrievalOwnerFilter through RAG, deep-memory, and document-enrichment so production anonymous search sends the public owner sentinel. Adds live migration check script and updated access tests.
@supabase

supabaseBot commented Jul 5, 2026

Copy link
Copy Markdown

Updates to Preview Branch (cursor/reconcile-retrieval-sentinel-5c94) ↗︎

DeploymentsStatusUpdated
DatabaseSun, 05 Jul 2026 17:51:02 UTC
ServicesSun, 05 Jul 2026 17:51:02 UTC
APIsSun, 05 Jul 2026 17:51:02 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

TasksStatusUpdated
ConfigurationsSun, 05 Jul 2026 17:51:04 UTC
MigrationsSun, 05 Jul 2026 17:51:06 UTC
SeedingSun, 05 Jul 2026 17:51:07 UTC
Edge FunctionsSun, 05 Jul 2026 17:51:08 UTC

View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

…d_at
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@BigSimmo
BigSimmo marked this pull request as ready for review July 5, 2026 17:46
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

cursoragentand others added 2 commits July 5, 2026 17:48
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@BigSimmo
BigSimmo enabled auto-merge (squash) July 5, 2026 17:58
@BigSimmo
BigSimmo merged commit a2153f7 into mainJul 5, 2026
5 checks passed
@BigSimmo
BigSimmo deleted the cursor/reconcile-retrieval-sentinel-5c94 branch July 9, 2026 12:35
BigSimmo added a commit that referenced this pull request Jul 9, 2026
* fix(ui): center mobile hero search and harden composer portal
- Hide footer Evidence/Sources chips on phone hero composers; scope stays in + menu
- Suppress bottom-dock composer flash until hero portal slot is ready
- Increase composer action/send touch targets to 44px on phones
- Update Playwright tests for scope menu, Answer home geometry, and stress fallback
* chore: ignore medications snapshot gitleaks false positives for deploy
* fix(access): resolve outstanding P0/P1 security and UX gaps
* fix(rag): scope anonymous retrieval to public documents via owner sentinel
* fix(access): complete forms fallback, signed-url hardening, upload guard
* feat(db): promote locally reviewed documents to public corpus for anonymous RAG
* fix(access): complete forms fallback, signed-url hardening, upload guard
* test: stabilize scope-sources stress flow via answer options menu
* chore: ignore medications snapshot gitleaks false positives for main deploy
* chore: ignore medications snapshot gitleaks false positives for main deploy
* chore: format access rollout files and add batched public document promotion script
* fix: audit P0 RAG cache, synopsis parity, and safety hardening
* fix(access): repair image signed-url route and align access route tests
* Compact answer recommended questions into shared suggestion chips
Replace the answer-mode composer banner and post-answer card with a shared
compact chip row that matches the footer search setup styling.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* fix(access): add document_upload rate limit bucket and update retrieval scope tests
* Auto-hide answer support chips when content sits below on mobile
* fix: clear merge markers and align tests with local upload guard
* fix(db): repair dollar-quote delimiters in retrieval owner sentinel migration
* fix(db): align schema.sql retrieval_owner_matches dollar quotes
* docs(governance): record public and anonymous API access verification
* fix: unblock production anonymous search and setup-status project warning
* style: format access rollout files for CI
* Polish answer suggestion chips across empty home, mobile composer, and modes
Unify empty-state quick actions with compact chips, move mobile follow-ups
into the footer composer, apply example chips to all search modes, add chip
truncation, update mockups/tests, and trim unused ClinicalDashboard imports.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* checkpoint before checking out cursor/add-supabase-plugin-6f14
* fix: drop accidental worktree gitlinks blocking CI
Remove nested worktree references from the plugin branch and ignore
.worktrees/ so checkout and required status checks can pass.
Co-authored-by: Cursor <cursoragent@cursor.com>
* style: format CI-flagged sources after main sync
* Fix CI failures from compact suggestion chips rollout
- Run Prettier on new/changed dashboard components
- Update ui-tools tests for Examples chips instead of Searching banner
- Scroll mobile table expand clear of taller footer composer stack
- Increase main bottom margin when composer follow-ups are visible
- Relax touch-target measurement tolerance for subpixel CI variance
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* feat: add public corpus promotion and retrieval-owner verification scripts
Add check:retrieval-owner to validate the live sentinel migration and promote:public-documents with dry-run/--apply for locally reviewed docs.
* Relax touch-target tolerance for full-suite subpixel variance
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* fix(mobile): prevent integrated action menu clipping on mode homes
* Hide cross-mode chip prefix when query is empty
The Examples dropdown section uses chips layout without a query, so
rendering 'Search "" in' was misleading. Only show the prefix when
there is an active query (cross-mode search).
* fix(ui): center mobile hero search and harden composer portal
- Hide footer Evidence/Sources chips on phone hero composers; scope stays in + menu
- Suppress bottom-dock composer flash until hero portal slot is ready
- Increase composer action/send touch targets to 44px on phones
- Update Playwright tests for scope menu, Answer home geometry, and stress fallback
* fix(ui): resolve merge conflicts and restore /applications route
- Remove stray merge conflict markers from master-search-header, ui-stress, globals.css
- Restore standalone /applications page and remove redirect to /?mode=tools
- Re-export ApplicationsLauncherPage for the restored route
- Regenerate docs/site-map.md
* fix(ci): format sources and stabilize scope-menu Playwright helpers
* style: format ui-stress spec for prettier check
* fix(mobile): restore favourites composer slot and stabilize scope smoke tests
* test(ui): fallback to answer options menu for desktop scope stress
* test(ui): open scope via answer options in desktop stress path
* test(ui): relax scope summary copy matcher for stress desktop path
* style: format mode-action-popup for prettier check
* Fix: close evidence sheet on follow-up quote to unblock focus trap
* test(ui): use openDailyActions for mobile stress scope path
* test(ui): force-click scope menuitem in mobile stress path
* fix(ui): clear integrated menu layout on close without effect setState
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(rag): deploy app-layer public owner sentinel for anonymous search (#291)
* fix(rag): scope anonymous retrieval to public documents via owner sentinel
Wire retrievalOwnerFilter through RAG, deep-memory, and document-enrichment so production anonymous search sends the public owner sentinel. Adds live migration check script and updated access tests.
* fix(ci): prettier migration check script and promote migration updated_at
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* fix: unblock PR CI — migration column guard and schema test sentinel
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* style: format supabase-schema sentinel assertions for prettier
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* Compact answer recommended questions into shared suggestion chips (#283)
* fix: resolve stale merge conflict markers from fix-all-db-issues integration
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* fix: run prettier on 6 files failing format check
* test(ui): relax mode-dependent selectors in smoke test
* Fix Prettier formatting violations
* test(ui): make mode-options smoke assertion mode-agnostic
* fix(ingestion): R24e — drop ingestion_job_stages.job_id FK (resolve drift #8)
Live evidence, not the backlog's add-the-FK plan: live never had this FK, the
edge agent writes an indexing_v3_agent_jobs id into job_id (never satisfies a FK
to ingestion_jobs), and live carries 253 orphan stage rows that would fail
VALIDATE. Removing the FK makes fresh/preview databases match live and unbreaks
the agent's artifact-repair path there.
- schema.sql: ingestion_job_stages.job_id is a plain not-null column (documented)
- migration 20260708140000: drop constraint if exists (no-op on live; removes it
on the branch/preview migration chain)
- drift-allowlist: remove the now-obsolete FK entry (schema.sql matches live)
- database-drift-detection.md: mark backlog item #8 resolved with rationale
- supabase-schema.test: assert the FK is absent from schema.sql
- drift manifest regenerated (clean scratch replay)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(ingestion): suppress agent stage ids from OCR retry targets
Only expose failed_ocr jobId values that resolve to a document ingestion_jobs row so the quality review Retry action does not 404 after dropping the ingestion_job_stages.job_id FK.
* fix: stale search-effect closure, missing effect dep, dead code cleanup
- ClinicalDashboard.tsx: route the URL-bootstrap search effect through refs
for executeSearch/scopeFilters so it always calls the latest versions
instead of the ones captured when the narrowly-scoped effect first ran
- DocumentViewer.tsx: add missing canUsePrivateApis effect dependency
- playwright-base-url.ts: accept 127.0.0.1 alongside localhost when
verifying an existing local server
- Remove the already-unused onPickSample prop from AnswerEmptyState, and
two now-unused type-only imports (MedicationRecordRow, RegistryRecordRow)
Verified: typecheck clean, lint clean, prettier clean, medications-route
and registry-records-route tests passing (17/17).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* style: format ingestion quality route for CI prettier gate
* chore: trigger CI after prettier fix
* chore: organize pending local changes
* ci: add image-generation remediation workflow
* feat: add image generation metadata re-stamp script
* fix(ci): stub RAG_QUERY_HASH_SECRET in deployment boot smoke
Production instrumentation requires RAG_QUERY_HASH_SECRET (PIA-2) when
next start boots in NODE_ENV=production. The boot smoke already stubs
Supabase and OpenAI keys for the child process; add the same pattern for
the query-hash secret so main CI deployment readiness passes.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* style: format deployment-boot-smoke for Prettier
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* fix(ci): require RAG_QUERY_HASH_SECRET in deployment boot smoke
Address Codex review: do not stub the query-hash secret with a known
placeholder. Add it to requiredProductionEnv and pass it from the CI
workflow secrets so main/release smoke fails closed when the secret is
absent, matching the Supabase/OpenAI gate pattern.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* fix(ci): supply RAG_QUERY_HASH_SECRET placeholder for deployment boot smoke
The production instrumentation hook requires a keyed query-hash secret so
logged clinical queries are not reversible. CI boot smoke only checks that
the server starts and reports the correct local project identity, so a
non-production placeholder is sufficient (same pattern as service-role and
OpenAI placeholders).
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* style: run Prettier on deployment-boot-smoke.mjs
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* ci: retrigger checks after Codex feedback fix
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* chore: reconcile ingestion RPC execute privileges, schema.sql and drift manifest
* ci: add db-reset-verify and dependency-review workflows
* ci: remove dependency-review workflow because repository is private without GHAS
* 📝 CodeRabbit Chat: Simplify code implementation (#434)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* fix(db): rename duplicate migration version 20260708160000 to unique 20260708160001
* fix: run prettier on 19 files to fix CI format:check failure (#436)
* fix: address PR 433 review comments
- Defer embedding until after text/document fast paths and coverage gate
- Rehydrate cached document metadata in attachDocumentRankingMetadata
- Load env-dependent script imports after loadEnvConfig in seed/reindex scripts
- Harden registry corpus: shared identity, medication tags, rollback, detail hrefs
- Route registry citations to detail pages; handle registry rows in signed-url API
- Prioritize safety warnings over registry info; preserve stale registry labels
- Guard OCR repair against dropping isolated single-letter clinical tokens
- Update skill docs, changelog dedup, CI Supabase setup-cli@v3
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* fix: address follow-up PR 433 review comments
- Backfill NULL document_images.index_generation_id during re-stamp
- Count globally forced embedding eval cases in retrieval summaries
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* docs: add Codex review throttling protocol
* fix: format docs/branch-review-ledger.md to pass Prettier check
* fix(docs): tighten ledger skip checks and pure-review exception
Require branch, HEAD, and scope to match before skipping cleanup reviews, and allow ledger appends as the sole edit during pure review runs.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: throttle Codex auto-resolve reviews
* fix: harden Codex review automation
* fix: prevent repeated Codex auto-resolve requests
* fix: keep Codex auto-resolve permission skips green
* fix: harden Codex auto-resolve GitHub API error handling
Wrap issues.listComments and issues.createComment in explicit try/catch paths that log a clear notice and fail the job deterministically on auth, rate-limit, and client API errors while preserving skip cases.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: harden Codex auto-resolve dedupe and review guards
Use SHA-scoped PR labels and concurrency to prevent duplicate resolve requests, skip non-actionable reviews, document the auto-resolve confirmation exception, and add CI workflow guard checks.
Co-authored-by: Cursor <cursoragent@cursor.com>
* docs: add Codex resolve follow-up guidance and issue_comment trigger
Document manual re-trigger, add resolve completion checklist, cover top-level PR comments in auto-resolve, and link the review protocol from README.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: format check-codex-autofix-workflow.mjs with Prettier
* fix: harden Codex auto-resolve label and retrigger guards
Queue PR/head workflow runs, create SHA labels before applying them, post the resolve comment before the label lock, and skip resolve completion summaries on issue_comment events.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: soft-skip Codex auto-resolve on comment read 403
Treat issues.get and listComments permission failures as warnings so the workflow stays green when the token cannot read PR metadata.
Co-authored-by: Cursor <cursoragent@cursor.com>
* archive: document obsolete branch preservation
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
BigSimmo added a commit that referenced this pull request Aug 9, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BigSimmo pushed a commit that referenced this pull request Aug 9, 2026
…check
Implements the smallest correct fix for the two recommendations that had one,
and corrects a recommendation that turned out to be wrong.
#291 — corrected, not implemented as written. The row claimed that pairing
`disabled` with an aria-describedby sr-only reason means the reason is never
announced, and that the docs and eslint rule blessed the pairing so a repo-wide
pass was needed. All three claims are refuted. A natively disabled button stays
in the accessibility tree with its accessible description intact;
tests/favourites-hub-unavailable-controls.dom.test.tsx pins toBeDisabled(),
not.toHaveAttribute("aria-disabled") and toHaveAccessibleDescription() together
under the title "keeps unavailable actions natively disabled and exposes their
reasons" — a deliberate, tested decision. require-button-wiring already accepts
both forms. An attempt to swap the attribute in favourites-hub and four other
sites was reverted for contradicting that decision.
The one real defect was documentation: docs/wiring-conventions.md named
favourites-hub.tsx as "the reference markup" while showing aria-disabled="true",
which is not what that file does. The canonical block now shows native
`disabled`, explains why the description survives it, cites the pinning test,
and says when aria-disabled + a no-op handler is correct instead. It also
records that `disabled` + `aria-disabled` together is a third shape pinned two
contradictory ways in one test file, which stays open as the residue of #291.
#292 — mitigation implemented. The open-PR check before starting a queued item
is now in newtask's "Before you start" (which already made that GitHub read for
PR bundling, so it asks the same list a second question at no extra cost), in
the issues skill after the read-only flow, and in the /issues section of
AGENTS.md so Codex and Cursor get it too rather than Claude Code only. All three
scan for the route/component/surface rather than the ledger id, and degrade to a
warning when GitHub is unreachable.
#290 — not implementable here: it needs two more same-SHA Playwright
reproductions, and this container has no node_modules or browsers.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Ex4A4xx3tNdZUADV9VaXK
BigSimmo added a commit that referenced this pull request Aug 9, 2026
…ree items (#1773)
* docs: pin PageSection ownership to the nav-header sibling, capture three items
Record the in-page-nav decision in docs/search-chrome-behaviour.md under
"Default in-page navigation template": a page's PageSection[] table is always
owned and exported by a colocated "use client" nav-header sibling, never
declared inline in the page and never in a separate per-route section-index
module.
The four Server Component pages need the sibling regardless, since neither
onSelectSection nor a LucideIcon crosses the RSC boundary. Applying the same
shape to Client Component pages gives one answer to where the section table
lives and one import path for tests/in-page-nav-route-sections.dom.test.tsx.
PR #1766 shipped both shapes; the closed PR #1767 proposed a third.
States explicitly that existing pages are not being migrated — the rule binds
new conversions only, and the grandfathered sites are listed so the
inconsistency is not read as a bug.
Ledger:
- #291 (P2 issue) disabled placeholder buttons pair `disabled` with an
aria-describedby sr-only reason; `disabled` removes the tab stop, so the
reason is never announced. Needs one repo-wide pass across the sites,
docs/wiring-conventions.md and eslint-rules/require-button-wiring.mjs.
- #292 (P2 rec) two sessions built the same queued item four hours apart
(PR #1766, closed PR #1767); check the open-PR list for the affected route
before starting a queued item, not just the ledger row.
- #290 extended with the quarantine arithmetic: tests/flake-ledger.json is
empty, so two more same-SHA reproductions are needed before a quarantine
entry is admissible.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Ex4A4xx3tNdZUADV9VaXK
* docs: fix the placeholder-pattern reference, wire the duplicate-work check
Implements the smallest correct fix for the two recommendations that had one,
and corrects a recommendation that turned out to be wrong.
#291 — corrected, not implemented as written. The row claimed that pairing
`disabled` with an aria-describedby sr-only reason means the reason is never
announced, and that the docs and eslint rule blessed the pairing so a repo-wide
pass was needed. All three claims are refuted. A natively disabled button stays
in the accessibility tree with its accessible description intact;
tests/favourites-hub-unavailable-controls.dom.test.tsx pins toBeDisabled(),
not.toHaveAttribute("aria-disabled") and toHaveAccessibleDescription() together
under the title "keeps unavailable actions natively disabled and exposes their
reasons" — a deliberate, tested decision. require-button-wiring already accepts
both forms. An attempt to swap the attribute in favourites-hub and four other
sites was reverted for contradicting that decision.
The one real defect was documentation: docs/wiring-conventions.md named
favourites-hub.tsx as "the reference markup" while showing aria-disabled="true",
which is not what that file does. The canonical block now shows native
`disabled`, explains why the description survives it, cites the pinning test,
and says when aria-disabled + a no-op handler is correct instead. It also
records that `disabled` + `aria-disabled` together is a third shape pinned two
contradictory ways in one test file, which stays open as the residue of #291.
#292 — mitigation implemented. The open-PR check before starting a queued item
is now in newtask's "Before you start" (which already made that GitHub read for
PR bundling, so it asks the same list a second question at no extra cost), in
the issues skill after the read-only flow, and in the /issues section of
AGENTS.md so Codex and Cursor get it too rather than Claude Code only. All three
scan for the route/component/surface rather than the ledger id, and degrade to a
warning when GitHub is unreachable.
#290 — not implementable here: it needs two more same-SHA Playwright
reproductions, and this container has no node_modules or browsers.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Ex4A4xx3tNdZUADV9VaXK
* docs: address PR review — fix the quarantine sequencing and the ledger-status claim
Two valid review findings on #1773, both verified against the repo before acting.
Codex (P2, docs/outstanding-issues.md #290): the "Next" step added in 16b6119
told the reader to append each of the next two reproductions to
tests/flake-ledger.json. Following it cannot produce a valid ledger state.
docs/testing.md requires three same-SHA reproductions before an entry is added
or retained, and scripts/flake-ledger.mjs:48 throws "exact title must include
@quarantine" while tests/ui-smoke.spec.ts:4470 is untagged — so an entry written
today fails validation outright, and appending per run treats the ledger as a
per-run log. The step now says to record the two observations in the row itself,
then after the third reproduction tag the test @quarantine and add one complete
entry with owner, repro command, tracking ref, first/last-seen and expiry.
CodeRabbit (docs/outstanding-issues.md #292): the claim that the ledger "has no
in-progress state" is wrong — IN PROGRESS appears on two rows and IMPLEMENTED in
PR #1766 on another. The accurate gap is narrower and worse: there is no
structured status field and no atomic claim, so a marker is written by whoever
did the work, usually after the fact, and nothing requires or checks one, which
means the absence of a marker carries no information. Corrected in all four
places that carried the overstatement, not only the flagged row — AGENTS.md, the
issues and newtask skills, and #292 itself — since the same sentence had been
copied into each.
CI on the previous head was green; no check failures were outstanding.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Ex4A4xx3tNdZUADV9VaXK
---------
Co-authored-by: Claude <noreply@anthropic.com>
BigSimmo added a commit that referenced this pull request Aug 9, 2026
…1770)
* feat(design-system): gate the ungated prohibitions and close gate 4
Closes#264 and the gate-4 half of #265. Measured against origin/main
8db1e53 rather than scoped from the handover, which mattered: four of
#264's six prohibitions were already gated while GATES.md §3 read
`planned`, and that understatement is what deferred this task twice.
Tighten two ratchets carrying stale slack to their measured values:
edgeOwnershipConflicts 28 -> 27 and legacyShadowAliases 231 -> 224. Seven
files had paid debt down without a baseline refresh, so up to seven new
violations would have passed. Every other metric and debtByPath entry was
asserted unchanged first. The remaining 224 aliases are #262's cleanup.
Add three checks to check:design-system-contract:
- statusColouredNumerals (ratcheted 2) — a text-* status hue on an element
whose children are all figures.
- colourOnlyStatusIndicators (ratcheted 4) — gate 4's repository-wide
enumeration. A status hue on a box with no children, no accessible name
on it or any ancestor, no sibling text, and not a StatusMark. Also flags
shared swatch recipes, since the analyzer is per-file and cannot follow
an imported statusDotReady to its call sites.
- imageInversions (pinned at zero) — CSS filter/backdrop-filter and the
Tailwind invert/hue-rotate utilities.
All three are mutation-verified, as is the tightening: reintroducing one
var(--shadow-tight) now fails with `legacyShadowAliases increased from 224
to 225` plus the per-path assertion, where the same edit passed at 231.
The two status metrics were re-measured three times before the baseline was
written. The first draft found 19 colour-only indicators and 3 numerals; 15
were false positives — a name test loose enough that `size` read as a
numeral, an ancestor walk checking for JsxOpeningElement when ancestors are
JsxElement, and no sibling-text rule, which condemned the ordinary legend
pattern. All six recorded entries were read in source and confirmed real.
Gate 2 is NOT closed. A rendered-interactive tap enumeration was written,
shown to find genuine defects, and reverted: six runs against one
production build returned 6, 5, 4, 3, 3 and 9 distinct sub-floor shapes
because the audit races the async render, and networkidle plus shape
deduplication did not settle it. ui-style-contract.spec.ts runs in the
required Production UI job, so an intermittent version would have blocked
every merge. Recorded with the defect it found as #289.
Correct GATES.md throughout, including the gate 2 row's claim that
test:e2e:style-contract needs wiring into verify:cheap — the spec already
runs in required CI via productionSpecPattern and playwright-pr-shards, and
adding it to verify:cheap:internal would trip check:gate-manifest, which
requires every gate in that chain to also run in the browser-less static-pr
job. Close#277, verified already satisfied on main.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(ledger): record the M2 design-system gates review
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(ledger): supersede the M2 row after renumbering #289 to #291
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix: expand NUMERAL_TEXT regex to accept clinical unit suffixes
Co-authored-by: BigSimmo <87357024+BigSimmo@users.noreply.github.com>
* fix(design-system): close seven gaps found in review of the new gates
All seven review findings reproduced against a probe before any change, and
every one was real. Two mattered:
- Neither status pattern accepted Tailwind's `/NN` opacity modifier, and 83
status-token utilities in `src` carry one. The anchored patterns rejected
every one before the semantic checks ran, so a colour-only indicator written
`bg-[color:var(--danger)]/90` walked past a ratchet described as
repository-wide.
- `NUMERAL_TEXT` rejected every letter while its own comment claimed units were
covered, so a dose painted in a status colour — the case the rule exists for
— passed silently. Replaced with `isNumeralTextFragment`, which accepts an
explicit unit list and still rejects prose. It carries no digit requirement,
because `{dose} mg` splits into an expression and a unit-only text child.
Also:
- `isNumeralExpression` now requires an arithmetic operator, rejects any
string/template operand anywhere in the tree, and follows only `+`/`-` prefix
unaries, so `{count + " errors"}` is no longer a numeral. The previous comment
claiming a concatenation "would have a string literal rather than a numeric
one" was wrong: one numeric side was enough.
- `INVERSION_FUNCTION` matches the `invert(`/`hue-rotate(` call itself, covering
`filter-[invert(1)]`, `[filter:invert(1)]` and both `backdrop-` forms.
- `IMAGE_INVERSION_UTILITY` excludes `-0`; `invert-0` disables inversion and a
hard-zero gate must not reject the reset.
- `rendersVisibleText` no longer counts `{null}`, `{false}`, `{undefined}` or a
bare self-closing element as a text channel, and judges markup-building
expressions by that markup.
- `hasNonEmptyAccessibleName` replaces the attribute-presence test, so
`aria-label=""` no longer exempts an indicator.
- The Gate 9 row said a layout-property lint was "planned" while
`layoutTransitionExceptions` already ships and ratchets per path — the exact
contradiction this branch exists to remove.
Fixing these surfaced two false positives of my own, both caught by
re-verifying every baseline entry in source rather than trusting the count: the
sibling walk stopped at the first non-JSX parent, so a conditional badge with a
label was reported colour-only; correcting that then let a sibling
`{started ? <div/> : null}` — another coloured div — count as text.
Baseline unchanged at 4 colour-only / 2 numerals / 0 inversions, and unchanged
by the unit widening, so nothing in the repo paints a dose in a status colour.
`legacyShadowAliases` 224 -> 220 from the main merge. Every new form is
mutation-verified.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(ledger): supersede the M2 row after the review round and main merge
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
cursorBot pushed a commit that referenced this pull request Aug 9, 2026
Keep main's shared #290-#293 queue. Drop duplicate pr-handoff #291 (already #284).
Archive this PR's #279 canvas-gate resolution alongside main's #264/#277 closures.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo added a commit that referenced this pull request Aug 9, 2026
…d reading mode, and the first canvas gate (#1772)
* test(viewer): gate the PDF canvas raster in a real browser
The viewer's raster surface had no browser proof: unit tests cover the raster
budget, DOM tests cover gestures, and a static contract covers the lazy
boundary, but none of them can see whether a clinical source page actually
paints. A blank canvas still reports correct dimensions, a correct aria-label,
and a resolved render promise.
tests/ui-document-canvas.spec.ts reads the raster back — ink pixels on page 1,
the real page count in the one toolbar readout, and a page flip whose FNV pixel
signature differs from page 1's. It also attaches an advisory page-flip cost
measurement (long tasks + time to paint) as the input the OffscreenCanvas
decision is conditioned on.
pdfjs-dist@6 calls Map.prototype.getOrInsertComputed, which ships in Chromium
151 but not in the 141 build some sandboxed containers pre-bake and pin via
PLAYWRIGHT_BROWSERS_PATH. The skip guard is therefore asymmetric: without CI it
skips with a reason naming the browser version; with CI set a missing engine
feature FAILS, because a gate that can skip itself green on the machine that
gates the merge is worse than no gate. Both directions were verified locally.
Spec collection is three hand-maintained lists that must agree, so all three are
updated together and tests/playwright-project-isolation.test.ts gains a
fail-closed assertion for this basename — "did not run" and "ran and skipped"
are indistinguishable in a log otherwise.
Closes#279 in docs/outstanding-issues.md. Its two refuted remedies (bump the
pinned Playwright build, pin pdfjs-dist down) were not actioned; the recorded
measurements were re-derived after install and match.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7
* perf(viewer): virtualize the PDF reader into a windowed page column
The reader rasterised exactly one page into one canvas, so every page flip on a
long guideline was a cold pdf.js render. That is the remaining felt slowness in
the document view.
The viewer now renders a column of page slots and keeps a small window of them
rastered. Each slot reserves its page's box whether or not a canvas is currently
in it, so disposing a far page does not move the scroll position a reader
navigates by, and pages outside the window drop their backing store instead of
holding it until collection.
Three constraints shaped this and are resolved explicitly rather than deferred:
The raster budget is now document-wide. resolveCanvasRasterPlan bounds ONE
canvas against WebKit's ~2^24 ceiling and says nothing about how many exist, so
N individually-legal canvases could still exhaust device memory.
resolveLiveCanvasWindow caps total retained raster instead. Its useful property
is the curve, not the constant: a fit-width phone page never binds against it,
while a page at maximum zoom costs the whole per-canvas ceiling and collapses
the window to one — render-ahead disappears exactly where retaining neighbours
would be most dangerous, with no special-casing of zoom. MAX_CANVAS_PIXELS is
unchanged.
Render-ahead is reconciled with disableAutoFetch rather than trading it away.
Those flags exist because a reader looks at one page and pdf.js would otherwise
pull a whole guideline over cellular; rendering neighbours pulls exactly those
bytes back. Both flags stay, and the policy is bounded on three independent
axes: one page either side, deferred to requestIdleCallback so a fast flip never
pays for pages it passes, and switched off entirely under Save-Data or 2g. Three
resident pages, never the document.
Page sync stays one-way. Intent scrolls the column, scroll position derives the
displayed page, and a derived page writes the route only when it did not come
from a programmatic scroll. Two real races surfaced while testing this: the
route effect re-runs when pdf.js reports its page count, which is always after
the reader can have scrolled, so it now acts only when the route asks for
somewhere the reader is not; and the in-flight gate is armed when intent is
registered rather than a frame later when the scroll executes, since
intersections landing in that gap read as reader input and cancel the jump.
Multi-page documents get a bounded reading pane so the column is the thing that
scrolls; single-page documents keep their existing geometry exactly. The fit
scale now derives from the holder's content box rather than clientWidth minus a
fixed 16px, which was 16px short at sm:p-4 — invisible with one canvas, a layout
shift once slots reserve boxes from the same number.
Preserved: the per-run pageToCleanup isolation (Sentry 15801413), canvas zeroing
on dispose, the renderZoom debounce with its interim transform, and the
isLikelyExpiredUrl recovery path — which now also fires for a neighbour's range
403 while refusing to blank the reader's good page over a failed prefetch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7
* perf(viewer): window the document rail's figure cards
The rail mapped every clinical figure and every audit figure into a
DocumentImage on mount. That row is not cheap: it parses table markdown, decides
whether a structured AccessibleTable can render at all, computes quality
warnings and evidence tags, and mounts a SignedImage frame. A guideline with
ninety indexed tables paid all of it during hydration, before the reader had
opened the section, and the audit list underneath paid it again.
DocumentImageList renders a window of six and grows it as a sentinel comes into
view, with an explicit control to reveal the rest. Short lists — the
overwhelming majority of indexed documents — render whole and get no extra
chrome at all. The window is derived during render rather than synchronised in
an effect, so a list that shrinks underneath an expanded reader clamps
immediately instead of pointing past the end of the array for a frame.
The filmstrip is left whole on purpose: it is one button per figure with no
image behind it, and it is the cheap way to reach any page.
One correction to the Phase 3 brief, recorded in the test rather than assumed
either way. The brief says collapsed audit rows "still mint signed URLs".
SignedImage already defers its fetch behind an IntersectionObserver and a closed
<details> is display:none, so that claim is at least doubtful — but it is a
claim about real browser layout, and jsdom does no layout, so nothing available
here settles it. What is certain, and is what this commit removes, is the
mounting cost, which applies whether the section is open or shut. The rail's
observer also uses a 320px root margin rather than SignedImage's 640px, so the
two do not both run far ahead of the viewport once the section does open.
No virtualization dependency: rows have data-dependent heights, a windowed list
needs no measurement to be correct, and check:bundle-budget totals every built
chunk, so a library would land straight on it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7
* perf(viewer): give deferred rail figures a lower fetch and decode priority
Two levers, both about the same thing: a secondary figure rail should not
contend with whatever the reader actually opened.
SignedImage now sets fetchPriority explicitly — high when the caller marked the
figure above-the-fold, low otherwise. next/image already emits decoding="async",
which governs when a decode blocks; fetch priority governs whether the image
competes for that budget at all, and it was the missing half of the pair.
The document rail passes a 240px IntersectionObserver root margin instead of the
shared 640px default. The wide default suits a surface whose images are the
point of the page; the rail's are not, and at 640px it minted signed URLs for
rows most of a viewport away, which land while the reader is looking at
something else. There is no cross-surface request scheduler, so this margin
differential is the ordering: surfaces on the wide default resolve first.
The 100-id batch signed-URL route stays unwired, deliberately. Beyond keeping a
privileged owner-scoped API route out of a component-only diff, the case for it
has actually weakened: windowing the rail to six rows means a figure-heavy
document no longer mounts N rows at once, which was the many-distinct-images
scenario the batch was meant to serve. Recorded on #283 with the measurement
that should decide it, rather than left as a standing assumption.
use-signed-image-url.ts is untouched. Its identity-in-the-dedupe-key and
cache-write-outside-the-shared-promise fixes were confirmed green before and
after (tests/auth-signed-url-cache.dom.test.tsx, 6 passed).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7
* feat(viewer): complete the PDF reader's keyboard reading mode
The holder handled arrow keys, +/-, and 0. Phase 3 adds Page Up / Page Down,
Home / End, F for fit-to-width, and R for rotate.
Rotation needed a route back out. `rotation` arrives as a controlled prop with
no callback, so the keyboard could reach every viewing control except that one.
Rather than give the viewer its own rotation state — a second source of truth
for a single toolbar button — R calls the same `handlePdfRotate` that
DocumentFrame's rotate control already calls, threaded down as `onRotate`. When
no handler is supplied, R stays inert rather than swallowed: the event is not
preventDefault'ed, so it still reaches whatever else wants it.
Modified keystrokes are now explicitly ignored. Ctrl/Cmd+0 is the browser's own
zoom reset and Cmd+Left is history back on macOS; a reader that lost either to
the viewer would be worse off than one with no bindings at all.
The holder's aria-label names the bindings, so a screen-reader user hears them
on focus instead of having to discover them. Contract documented in
docs/wiring-conventions.md and covered by tests/document-viewer-keyboard.dom.test.tsx,
including the two rules that are easy to regress silently: only keystrokes aimed
at the holder itself are handled, and rotation goes through the frame's callback.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7
* docs(viewer): record the Phase 3 outcome and the OffscreenCanvas decision
Phase 3's table said what to build; it now says what landed, including the two
items that deliberately did not.
Crop -> page overlay stays out: bbox is already SELECTed in document-detail.ts
but absent from DocumentDetailImage, so it is a contract change across
src/lib/**document** rather than a viewer change, and it is called out as the
one remaining Phase 3 capability with the shape of the work named.
OffscreenCanvas is not implemented, which is the plan's own instruction rather
than a shortcut — it conditions the work on "measured main-thread paint cost",
and no such measurement existed. Two things changed that. Virtualization keeps
the reader's page and a neighbour already rastered, so the cold-render-per-flip
cost that motivated a worker raster is largely gone before any threading work
starts; and the new canvas gate now attaches the number (flip-to-painted, long
task count and duration, backing pixels) on every Production UI run. #290
records how to read it and what result would close the question either way.
Nothing about this could be measured locally: pdfjs-dist@6 needs
Map.prototype.getOrInsertComputed, which this container's Chromium 141 lacks and
Node 24.13.0 lacks too, so no browser and no headless harness here can raster a
page at all.
Toolbar density is struck from the table — it shipped in Phase 2.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7
* test(viewer): keep the canvas gate out of the phone-chrome consumer list
The new spec imported blockExternalRequests from tests/helpers/phone-scroll,
which silently enrolled it in scripts/verify-phone-chrome.mjs's consumer list —
tests/verify-phone-chrome.test.ts asserts that list equals the set of specs
importing that helper, and went red. Enrolling it would have been wrong anyway:
this is a desktop raster gate and has nothing to do with phone chrome selection,
so it keeps a local copy of the request block instead, with a comment naming the
coupling so the next person does not re-import it.
Also records #291: tests/pr-handoff-stop.test.ts fails for any session running
as root, because it injects a write failure with chmod 0o555 and root ignores
directory write bits. Confirmed pre-existing on a clean origin/main worktree
with no local diff, so it is not from this branch — CI runs non-root and stays
green, and only container sessions ever see it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7
* docs(issues): record the measured bundle-budget headroom on #252
A clean production build for this branch reports 306 client chunks at 1538.4 KiB
gzip against the 1406.4 KiB baseline captured 2026-08-04 — +9.4% inside a 10%
tolerance, so roughly 8 KiB of gzip headroom remains.
The drift is pre-existing rather than from this branch: Phase 3 adds no
dependency and its code delta is small. But it means the next feature-sized PR
of any kind trips check:bundle-budget whatever it touches, which turns #252's
open question — whether counting mockup chunks makes that a real signal — from
theoretical into the thing that decides how the next red build is read.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7
* docs(ledger): record the Phase 3 review for PR #1772
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N496muQbJVbJW8XvkCgKc7
* fix(pdf-canvas-viewer): release canvas backing store on render=false and reset geometry on rotation
Co-authored-by: BigSimmo <87357024+BigSimmo@users.noreply.github.com>
* fix(viewer): key-repeat pageRef, live-canvas max budget, rail window reset
Relative keyboard moves read pageRef so key-repeat advances before React
re-renders. Budget retained canvases against the largest measured page cost.
Reset DocumentImageList on collectionKey. Soften #252 tip-only bundle wording;
tighten #294 OffscreenCanvas close criteria. Drop the PR-added legacy
shadow-tight on the empty page slot to keep the DS ratchet.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* fix(viewer): remount rail image list on document change via key
Avoid setState-in-effect for the expanded window reset; React remounts
DocumentImageList when the document-scoped key changes.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* style(tests): format document-rail image window test after prettier
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* docs(ledger): record PR 1772 review-and-fix at content tip 2cd72f1
Superseding heavy-scope row with decisive verify:cheap / verify:pr-local lines,
merge-tree clean, and the Phase 3 review dispositions.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo added a commit that referenced this pull request Aug 9, 2026
…#1778)
* fix(a11y): make unavailable placeholder buttons reachable by keyboard
The disabled-placeholder pattern put `disabled` on the button and attached
the reason it was unavailable via `aria-describedby` to an `sr-only` span,
plus a `title`. But `disabled` removes the tab stop, so a keyboard user —
and a screen-reader user moving by Tab rather than by virtual cursor — could
never land on the control. The explanation was written and unreachable; the
control simply vanished for them.
Convert the controls that are unavailable for a *stated* reason (feature not
built yet, or this record lacks the data) to `aria-disabled="true"` plus a
shared inert handler, keeping the title and the described-by reason. 24 sites
across 12 components, including the four that carried `disabled` and
`aria-disabled` together — belt and braces that never helped, because the
native attribute still won on focus.
Transiently inert controls keep native `disabled`, which is correct there: a
request in flight, a pager at its last page, a form action awaiting validity.
Those are listed in the wiring doc so the next pass does not "fix" them.
Styling had to move with the attribute: `disabled:` variants stop applying
once the native attribute is gone, and the control becomes hoverable. The
`controlDisabled` recipe grew its `aria-disabled:` half, and the therapy
recipes switched `hover:enabled:` to `hover:not-aria-disabled:enabled:` so a
converted control does not light up under the cursor.
Contract updated with the code, rather than left saying the opposite:
- docs/wiring-conventions.md replaces the "native disabled is the default
here" argument with the stated-reason vs transient split, and settles the
unreconciled pairing tracked as ledger #291.
- require-button-wiring gains `redundantDisabledPair`, failing on the two
attributes together on any <button>; a statically-off side still passes.
- tests/require-button-wiring.test.ts pins that the rule fires in both
directions, since a green lint run cannot distinguish a clean repo from a
rule that matches nothing.
- favourites-hub-unavailable-controls tabs onto a converted placeholder,
asserts it takes focus, asserts the description is what the reader gets,
and asserts keyboard and pointer activation do nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018hT4qToXT5nr3hV4ENqRgS
* docs(ledger): record the disabled-button accessibility handoff
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018hT4qToXT5nr3hV4ENqRgS
* fix(a11y): clear Static PR and Codex pair-check blockers
Update the therapy hover contract assertion to accept
hover:not-aria-disabled:enabled:, and run redundantDisabledPair before
the JSX spread escape so explicit disabled+aria-disabled pairs stay gated.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo pushed a commit that referenced this pull request Aug 12, 2026
…system rows
Archived as verified-delivered or duplicate: #303, #284 (dup of #296), #149,
#159, #286, #287, #288, #256, #261, #291.
Re-scoped with re-measured evidence: #210 (gate was never missing; the defect
is the .next/dev types include), #262 (aliases 228 -> 193), #266 (counts moved,
23 unadopted holds), #275 (premise 'only one implementation' is false — the
values leaked to five files).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017paT42ZVMf8jaLtkjFxdy5
BigSimmo added a commit that referenced this pull request Aug 13, 2026
Sign up for freeto 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.

2 participants

@BigSimmo@cursoragent