fix: complete public production access hardening for mobile and anonymous users - #270
Merged
BigSimmo merged 6 commits intoJul 5, 2026
Conversation
The golden retrieval set was 100% lexical fast-path (embedding_skipped_rate=1.0), so it could not measure whether a re-index changes vector/embedding retrieval quality. - forceEmbedding option on searchChunksWithTelemetry (SearchChunksArgs): bypasses every lexical text-fast-path so retrieval always exercises the embedding/vector stage. Diagnostic/eval-only; folded into the search cache key; never set on production paths. - eval-retrieval.ts: per-case `forceEmbedding` field + a global `--force-embedding` flag. - 10 `vector-*` cases (psychiatric monographs: PTSD, OCD, panic, anorexia, GAD, Tourette, postnatal, bipolar, ADHD, opioid) with forceEmbedding=true. Each is a clinical query that must be answered by vector retrieval of the right monograph — verified live at document_recall@5=1.0, content_recall@5=1.0, all via strategy=hybrid (embedding used). Rationale: forcing embedding is the correct instrument for re-index measurement — you want to measure the vector index directly, not have a lexical shortcut mask a regression. Wording alone can't reliably force the vector path (the fast-path is driven by emergent lexical-match strength), so the flag makes these probes deterministic. Live golden eval: 34/34 pass (24 existing + 10 new), no regression. verify:cheap green (980). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wire forceEmbedding through eval runners and retrieval cache keys, bypass coverage/lexical shortcuts when forced, and add golden-case failure metrics so vector regressions cannot hide behind text-fast-path or cache hits.
Updates to Preview Branch (cursor/fix-access-review-issues-5c94) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
BigSimmo
marked this pull request as ready for review
July 5, 2026 14:05
Uh oh!
There was an error while loading. Please reload this page.
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This was referenced Jul 5, 2026
BigSimmo added a commit
that referenced
this pull request
Aug 8, 2026
Same defect Codex flagged on PR #1719: the commits recording these measurements were authored 2026-08-08 (07:xx UTC), so a 2026-08-09 stamp places every re-measurement after the commit that recorded it. Four occurrences, in the #118, #269 and #270 rows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BigSimmo added a commit
that referenced
this pull request
Aug 8, 2026
BigSimmo added a commit
that referenced
this pull request
Aug 8, 2026
… disproved (#1738) `tap` was held out of CLINICAL_TWMERGE_THEME.spacing because declaring it would hand same-property conflicts to the later class -- "22 call sites, 18 of which would drop from 48px to 32/36/40/42px". #270 re-measured that premise and it does not hold: zero same-variant pairs exist, and the 84 survivors are cross-variant responsive step-downs that tailwind-merge cannot reach because it groups by variant. Several named call sites were stale outright -- DocumentManagerPanel and settings-dialog carry no tap token at all. The gap that kept #270 open was that a per-string-literal scan cannot see a conflict composed across cn() arguments: cn(recipe, "min-h-tap") pairs the recipe's min-h-7 with the token, and order decides the outcome -- recipe-then-tap raises to 48px, tap-then-recipe drops to 28px, the forbidden direction. So the sweep resolves constant recipe identifiers at each call site before grouping by variant AND property. Result across 700 files and 1418 cn() call sites (1410 with resolvable arguments, 802 resolvable class constants): ZERO same-variant pairs, in either direction. Two defects were found and fixed in the sweep itself before trusting that zero, because its first run reported three drops and all three were artefacts: - Ternary arms were concatenated, so `size === 44 ? "h-tap w-tap" : "h-[38px] w-[38px]"` looked like one element carrying both. Arms are mutually exclusive; arguments now expand to alternatives and compositions are enumerated. - Class constants were keyed by bare name globally, so `fieldControl` in one module resolved to a same-named constant in another and invented a conflict that does not exist at the call site. Resolution is now per file, with a global fallback only where a name is unambiguous repo-wide. The zero is mutation-tested rather than assumed: synthetic probes cn("h-tap", "h-4") and cn("min-h-tap", recipe) are both flagged as drops, cn(recipe, "min-h-tap") is correctly reported as a raise, and cn("min-h-tap", "sm:min-h-9") is correctly not flagged at all. The pinning test is replaced rather than deleted. It now asserts both halves: a same-variant pair merges to the last class, and cross-variant responsive step-downs pass through untouched. That second case is the load-bearing one -- if it ever merges, a control loses its breakpoint step. No production tap target is lowered, and no cross-variant numeric is deleted -- they are live responsive steps, not dead classes. Verified: tests/tailwind-merge-config.test.ts 34 passed (34); npm run test -- 5590 passed, 2 pre-existing environmental failures unchanged from the same run before this change (mode-nav-addon-slot absolute Windows paths, pr-handoff-stop); check:design-system-contract, check:icon-scale, check:type-scale all exit 0; format:check clean. Chromium look is delegated to this PR's Production UI jobs and is not claimed as run locally. Refs #270
BigSimmo added a commit
that referenced
this pull request
Aug 8, 2026
* docs(design-system): stamp the 7 August handover superseded Nine open ledger rows (#261, #262, #264-#270) cite docs/design-system/HANDOVER-2026-08-07.md as their Source, and docs/design-system/README.md sent anyone picking the work up cold straight to it as "measured state, the ordered plan". Four of its figures have since been disproved, and the corrections were written into the ledger rows rather than the document, so the document still asserts the originals. A session scoping from it re-derives work that is already known wrong. Recorded as #277. A banner rather than a rewrite, which is what #277 asks for: the corrections already live in the rows, and duplicating them re-creates the drift this fixes. The banner names the four measured errors -- the "229 --shadow-tight aliases" that is really a seven-token total mislabelled as one token (100 sites across 55 files, 228 total), the 24-vs-23 unadopted count, the "baselines cannot be generated on Windows" conclusion that overlooked the ubuntu CI job now used under #118, and #270's 22-site premise that has zero same-variant pairs -- then points at the rows. The README now sends a cold start to docs/outstanding-issues.md and marks the handover superseded in the link text itself, where the misdirection was. Deliberately not done: the document is not deleted, moved, or corrected in place. The nine Source citations, the PR and commit record, and its verification and gotcha sections are provenance the ledger is meant to preserve, and silently correcting it would leave those rows citing a document that no longer says what they were derived from. Verified: npm run docs:check-links -- "docs link check passed: 1667 repo path references resolve"; npm run docs:check-inventory -- current; npm run format:check -- "All matched files use Prettier code style!". Refs #277 * docs(design-system): drop false #118 baseline-adoption claim The superseded banner said all six visual baselines were adopted under #118, but the ledger still has #118 open and tests/__screenshots__/ holds only README.md. Keep the accurate Ubuntu-CI generation point without implying visual-regression protection exists yet. 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>
BigSimmo added a commit
that referenced
this pull request
Aug 9, 2026
…lection (#262 parts 2 and 3) (#1780) * docs(issues): close#218 and #270, both shipped before this session Both rows were still open in docs/outstanding-issues.md while their work was already live on main, which had scoped a third session from them. #218 (cn() lacks tailwind-merge) shipped in PR #1678, aeba5a2. src/components/ui-primitives.tsx:37 is twMergeClinical(...) rather than a plain join, package.json carries tailwind-merge ^3.6.0, and src/lib/tailwind-merge.ts declares the repo's @theme scales to twMerge. #270 (declare the tap spacing token) shipped in PR #1738, 80cf781, an ancestor of origin/main. "tap" is present in CLINICAL_TWMERGE_THEME.spacing, and tests/tailwind-merge-config.test.ts was inverted rather than deleted so the merge behaviour is now asserted rather than pinned out. Verified in source at origin/main 7aaf934, not inferred from the handover. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(design-system): ratchet raw padding, radius and line-height literals (#262 part 3) The design-system contract ratcheted colour, shadow, tap and tracking but not spacing, radius or line-height, so a value could bypass the scale as a bare literal in either a class or a stylesheet and nothing objected. Adds three per-path ratchets, covering both halves the way the colour and legacy-shadow metrics already do: rawPaddingLiterals 67 (17 CSS declarations, 50 class utilities) rawRadiusLiterals 24 (22 CSS declarations, 2 class utilities) rawLineHeightLiterals 3 (3 CSS declarations) The exemption is deliberately "contains no CSS function", not the narrower `(?!var\()` the tracking rule uses. Padding is not only ever a token or a literal: production ships pb-[env(safe-area-inset-bottom)], pt-[max(0.75rem,var(--safe-area-top))], pt-[clamp(1.5rem,5vh,3rem)] and pb-[calc(7rem+env(safe-area-inset-bottom))]. Those are computed from the viewport or the safe-area inset, cannot be spelled as a scale step, and a `var(`-only lookahead would have flagged every one of them. On the CSS side, zero in any unit, the CSS-wide keywords and custom-property declarations (the token definitions themselves) are exempt for the same reason. Every one of the 94 baseline entries was verified present at its cited line before pinning, and the baseline change is additive: all fifteen pre-existing metrics and every pre-existing debtByPath entry are byte-identical. Mutation-tested rather than assumed. Class side, in a file with no prior debt: - rawPaddingLiterals increased from 67 to 68 - rawPaddingLiterals at src/components/ui-primitives.tsx increased from 0 to 1 - rawRadiusLiterals increased from 24 to 25 - rawRadiusLiterals at src/components/ui-primitives.tsx increased from 0 to 1 - rawLineHeightLiterals increased from 3 to 4 - rawLineHeightLiterals at src/components/ui-primitives.tsx increased from 0 to 1 The CSS half fails the same way. Both probes also carried the sanctioned computed forms, and each count rose by exactly one, so the exemptions are proved by the same runs rather than argued. No new npm script: the metrics live inside check:design-system-contract, so docs:check-inventory and check:gate-manifest are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(design-system): gate type-step selection on the decidable half (#262 part 2) check:type-scale blocks arbitrary text-[12px] values. Nothing has stopped the scale itself growing a step no surface ever picks, which is the drift that makes a wrong selection possible in the first place. Whether a heading should have chosen text-sm over text-sm-minus is not mechanically decidable, and this does not pretend otherwise. A step that is declared and consumed by nobody is decidable, and there is one today: --text-2xl-compact (globals.css:112) has zero consumers -- no utility use, no var() use -- while the next-rarest step, text-hero, has one real consumer. The analyzer reports every bare text-<name> it sees and does not decide which names are steps; the checker intersects that against the @theme block it parses from globals.css. So the scale is never written down twice, and a step added to globals.css is covered without touching this gate. Retiring the dead step edits @theme, so it gets its own revertible PR rather than riding along here: it is carried in UNUSED_TYPE_STEP_EXEMPTIONS and tracked as docs/outstanding-issues.md #295. The exemption cannot rot silently -- the gate also fails if an exempted step stops being declared or gains a consumer. Mutation-tested, three ways: - type steps are declared in globals.css @theme but no production surface selects them: --text-2xl-compact (text-2xl-compact). Retire the step or use it; do not leave the scale carrying a step nobody picks. - (a newly added --text-probe-step fails identically, so this catches future drift rather than only today's known case) - --text-2xl-compact is exempted as unused but production now selects text-2xl-compact -- drop the exemption Measurement note, since three different figures were in circulation for this row: the "1318 sites" is a repo-wide grep INCLUDING mockups, which the gate excludes (1360 at this HEAD). Production consumers of the nine non-standard steps total 705 -- text-2xs 421, sm-minus 160, base-minus 57, 3xs 42, 2xl-minus 9, 3xl-minus 9, lg-minus 6, hero 1, 2xl-compact 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(design-system): correct GATES.md for the two new scale gates GATES.md's own §1 is the list of what actually runs, and this series' recurring failure is that list lagging the code: four of #264's six prohibitions were already gated while it said "planned". Records the padding/radius/line-height ratchets and the type-step selection rule in the contract row, and rewrites the type-scale callout, which claimed a step-selection lint "does not exist". The decidable half now ships; the half that asks whether text-sm-minus was the right pick over text-sm still does not, and cannot. Also corrects the "1 318 call sites" figure quoted there. It was a repo-wide grep including src/app/mockups/**, which every one of these gates excludes (1 360 at 7aaf934). Production consumers total 705, and there are nine non-standard steps, not eight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ledger): record the #262 parts 2/3 gate work (PR #1780) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(design-system): close scale-ratchet and unused-step review gaps Cover Tailwind arbitrary-property forms and modern CSS zero units in the raw scale ratchets, and validate unused-step exemptions against the same class-or-CSS consumer predicate used for ordinary steps. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses all 20 issues identified in the content-access review and deep testing pass.
Security & access control
document_readrate limiting to all anonymous/authenticated document read routes (list, detail, signed-url, in-document search, image signed-url)withOwnerReadScopeso authenticated users can read both owned and public (owner_id IS NULL) documentsincludeMetafor unauthenticated callerssearch_document_chunksRPC owner scoping via migration (no change to RAG retrieval algorithms)Auth
shouldResolvePublicCatalogAccess)UX
DocumentViewercanViewSourceDocuments)Data / release gates
npm run backfill:gold-labels -- --all-owners --write --confirm)check:document-label-governanceandcheck:production-readinessnow passTests
ui-tools.spec.tsnpm run verify:cheap)Test plan
npm run verify:cheap(1076 tests)npm run check:document-label-governancenpm run check:production-readinessnpm run verify:ui(E2E fixes included; full run recommended before merge)Notes
OPENAI_API_KEYmust remain configured in deployment env (now passing locally)