test(eval): harden force-embedding flag and vector-path eval guards - #264
Closed
BigSimmo wants to merge 2 commits into
Closed
test(eval): harden force-embedding flag and vector-path eval guards#264BigSimmo wants to merge 2 commits into
BigSimmo wants to merge 2 commits into
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.
This pull request has been ignored for the connected project Preview Branches by Supabase. |
3 tasks
BigSimmo
commented
Jul 5, 2026
OwnerAuthor
Superseded by #269 (consolidated platform fixes branch). |
14 tasks
BigSimmo added a commit
that referenced
this pull request
Aug 7, 2026
…zero (#264) (#1702) GATES.md §3 listed "`dark:` colour override" as **planned** with three live instances. A prohibition with no gate row is a suggestion, which is the failure mode that document exists to prevent. This closes it. All three instances were the same shape — `ring-1 ring-<colour>/20 dark:ring-<colour>/10` on a lux surface (`panel` in ui-primitives, the mobile FAB menu in dashboard-nav, the PWA notice card). They are now `--ring-highlight` and `--ring-glass`: per-theme tokens that carry the value the override used to supply, so the utility no longer needs a variant. `legacyPaletteUtilities` (the `ring-white/35` on the same PWA line) went to zero with them. No visual change, and that is measured rather than assumed. Tailwind emits `color-mix(in oklab, <colour> N%, transparent)` for `ring-<colour>/N`, and mixing any colour with `transparent` yields that colour at alpha N regardless of colour space, so the token values are equivalent by construction. `--ring-glass` is deliberately literal white and NOT `--neutral-0`, which inverts to `#060708` in dark and would have turned a white glass ring near-black. Both metrics are pinned at 0 in the contract baseline rather than ratcheted. The check asserts `value <= baseline`, so zero is a hard floor. Mutation-verified: reintroducing one `dark:ring-white/10` fails with `darkColorOverrides increased from 0 to 1` plus the matching per-path assertion. Every other metric was asserted unchanged before the baseline was rewritten, so clearing one number could not silently move another. Deliberately not fixed here, having been triaged and rejected as unsafe for a minimal change: `onePixelShadowSpreads` (2) are `0 0 0 1px … inset` border-substitutes whose conversion changes the box model, and `rawColorLiterals` (2) are a medication accent DATA fallback under a documented colour contract, not markup paint. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
…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>
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
forceEmbeddingthrough eval runners, retrieval cache keys, and RAG search paths.forceEmbeddingis set.Verification
npm run test -- tests/eval-quality.test.ts tests/retrieval-query-variants.test.tsnpm run verify:cheapnpm run eval:retrieval:quality(retrieval/ranking/scoring behavior changed)npm run check:production-readinessClinical Governance Preflight
forceEmbeddingtest flag