Surface clinical answer failures the task ledger already records - #2498
Merged
Conversation
…rant timeout Both shell out to a Node child process. Measured on this worktree: the child finishes in ~7 s standalone and the tests take ~9 s and ~10 s, so the global 30 s budget looks generous in isolation. It is not under the full run — the node suite is CPU-bound on two workers across 900+ files, and on 2026-09-01 the parity test exceeded 30 000 ms inside `npm run test` and failed the gate for a diff that touched nothing under `.design-sync/**`. Deliberately not quarantined: it did not fail an assertion, and a wrong time budget is a different defect from an unstable test. `tests/flake-ledger.json` is untouched, and the repo's three-reproductions rule does not apply here. Proven rather than assumed: setting the same option to 1 ms makes the test fail with "Test timed out in 1ms", so the option is read rather than silently ignored. Committed with SKIP_DOCS_SYNC_HOOK=1 because the pre-commit generator refuses a partially staged tree; the sibling commit that follows runs it in full on the final tree, which is what gets pushed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…acts The strip shipped in Phase 1 with `demoMode`, `documentCount` and `email` all `null`, rendering "environment unknown · document count unavailable · account unknown". Phase 2's design deferred them to whichever phase owned each value's data source and reserved any Supabase read from the hub for its own approval; that approval was given on 2026-09-01. `resolveHubEnvironmentFacts` gathers all three in one Supabase round trip. Two of its rules are load-bearing, and each is pinned by a test proven to fail without it: - The count goes through the cookie-bound user client, so row-level security (`documents owner read`, `owner_id = auth.uid()`) scopes it to the caller's own documents. The service-role admin client bypasses RLS and would report every owner's total to whoever was signed in; a source assertion fails if that import ever appears. - Every failure path returns `null`, never `0`, and an unauthenticated request skips the query rather than reporting the zero rows RLS would correctly hand it. Zero is a true answer here, so a failed read must not be able to impersonate an empty corpus. Reading data makes the page an async Server Component, so the dom tests and the shared in-page-nav route table now await their fixtures. `next build` classifies `/mockups/development` as dynamic, which it already was via the layout's `headers()` call, so nothing regressed there. The Phase 2 handoff document is corrected here rather than left claiming the strip is unwired. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d rejects Raised in review of #2495. The helper handled a returned `{ error }` but not a rejected promise, and the installed client rejects rather than resolves when a request is aborted or exhausts its network retries. An unhandled rejection there fails the whole developer hub page instead of degrading one line of it — during exactly the Supabase outage that makes the page worth opening, and contrary to the module's own stated contract. Both awaits are now inside one guard, not just the count: an auth read can fail the same way. `demoMode` is resolved before either of them and survives both, because it never depended on the network, so an outage cannot make the strip claim it is unable to tell demo data from live. Proven, not assumed: two tests whose mocked `select` and `getUser` reject both fail without the guard and pass with it, and they assert the whole returned shape rather than merely that nothing threw. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…' into claude/dev-hub-environment-strip
…ady records
The task ledger already records which clinical questions the answer engine gets
wrong -- today, three open items naming five questions between them, including
antipsychotic metabolic monitoring returning a placeholder instead of an answer.
Those facts sit unread among eighty-one open items. This puts them on the hub.
`resolveClinicalAnswerFailures` intersects two records the repository already
maintains: open ledger items, and the eval case list that names each clinical
question. Derived rather than curated on purpose -- a hand-kept list of ledger
ids is exact the day it is written and wrong a month later, with nothing to
catch the drift.
Matching is whole-token, and that is the load-bearing part rather than a detail.
Case ids nest: `discharge-documentation` and `patient-safety-plan` are both real
cases AND substrings of other real ones, so a plain `includes` reported five
questions where three were real -- two clinical questions on screen that nothing
had reported as broken. Proven by mutation: reverting the matcher to `includes`
turns both the synthetic nesting test and the real-data property test red.
The panel names itself for its evidence ("Open problems recorded against a named
clinical question"), and states above the list that a recorded problem which
does not name a case is not shown. On a clinical surface the reassuring-looking
empty state is the dangerous one, so the empty branch says in words that a quiet
ledger is not a well answer engine. Both are pinned by tests.
A hub band links straight to the panel, in warning rather than danger colours:
these are recorded problems, not blocking work, and painting them the same red
as the blocking band would teach the reader to discount both.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…linical-answer-failures # Conflicts: # data/repo-awareness-snapshot.json
…al-answer-failures # Conflicts: # src/app/mockups/development/page.tsx # tests/developer-hub-page.dom.test.tsx
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_282c8ff9-4f8b-4d4e-a019-55b9cad8a1d9) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
BigSimmo
enabled auto-merge (squash)
September 1, 2026 11:14
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10c79bdd0f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…n that case Raised in review of #2498, and confirmed against the real ledger text rather than taken on trust. `#J8SJQ9` is about `quality-antipsychotic-metabolic-monitoring` and names `quality-discharge-documentation` only as the CONTRAST -- the case that "deliberately drops mustContainAny" because a source pointer is a legitimate answer there. The panel listed it among the questions and counted it, stating on a clinical surface the opposite of what the ledger says. Two tempting fixes were tried against the real data and rejected: - Matching only the `source` field fixes `#J8SJQ9` and HIDES both genuinely broken questions in `#S4R2W3`, which names them only in its detail prose. On this panel a false negative is worse than a loose one. - Guessing intent from surrounding words is the fragile heuristic the panel exists to avoid; it would be wrong quietly, later. So the assertion moves to the level the data supports. The item-level claim was always sound -- these are recorded problems about clinical answers -- and the named cases are now presented as references: the section reads "Questions this item names", the count is "questions referenced" (`affectedQuestionCount` becomes `referencedQuestionCount`), and a second caveat says in words that an item may name a question as the contrast that is behaving correctly. Tests: the reviewer's requested fixture reproduces the real `#J8SJQ9` detail and pins that BOTH cases come back as references, with a comment saying a later change may only narrow that via an explicit per-case association in the ledger, never a heuristic. A DOM test pins the wording, and fails when the caveat is removed -- proven by mutation, then restored byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ailures' into claude/dev-hub-clinical-answer-failures
Contributor
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #14683 (success). That run's conclusion is an aggregate and did not exercise Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
BigSimmo
added a commit
that referenced
this pull request
Sep 1, 2026
Resolve dirty/behind after #2498: keep main regenerated snapshots and deps; overlay PR intentional clinical copy + token-divergence gate hardening.
14 tasks
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 free
to 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
/mockups/development/clinical-answer-failureslists them, and a band on the hub links straight to it so the count is visible on every visit.resolveClinicalAnswerFailuresintersects two records the repository already maintains — open ledger items, and the eval case list that names each clinical question. It is derived rather than curated deliberately: a hand-kept list of ledger ids is exact the day it is written and wrong a month later, with nothing to catch the drift.Verification
npm run verify:pr-localRun in full on the pre-merge tree and green end to end: all fourteen static and documentation checks,
lint,typecheck, thenTest Files 936 passed | 3 skipped (939)/Tests 11737 passed | 75 skipped (11812), thennpm run buildexit 0 (Compiled successfully in 3.1min,Client bundle secret surface check passed), thencheck:rag:fixtures,check:medication-interactionsandcheck:medication-lexicon-report, all exit 0.Reported precisely: an earlier attempt of the same gate stopped at
buildwithBUILD_REFUSED_DEV_SERVER, because a PsychSift dev server was still running from the browser check. That is a refusal rather than a failure; the server was stopped and the build then passed.What has not been re-run since.
origin/mainmoved after that gate (PR #2495 merged), and merging it in produced two content conflicts, insrc/app/mockups/development/page.tsxandtests/developer-hub-page.dom.test.tsx— the expected consequence of this branch's base landing on main as a single squashed commit. On the merged tree I have run the seven affected test files (Tests 108 passed) and a Prettier check, not the whole gate, so CI is the first full verification of the merged tree. The conflict resolution discarded zero non-empty lines from main's side, asserted mechanically during the resolution rather than judged by eye, andgit diff --stat origin/main HEAD -- src testsis 573 insertions and 0 deletions.Three things were mutation-proven rather than assumed:
includesturns both the synthetic nesting test and the real-data property test red.vi.resetModules()clears the module cache but not the mock registry, so it silently ran against a previous test's fixture; it now callsvi.doUnmockfirst, and the mutation above is what exposed it.UI verification not run:
npm run verify:uiwas not run because no shared UI foundation changed — this adds one page and one band, with no layout, chrome, composer or routing-shell change. The changed surfaces were proven directly instead:npm run ensure,/api/local-project-idconfirmed as this project, and both/mockups/developmentand the new page rendered and inspected in a browser. The panel showed 3 open problems and 5 clinical questions with their real question text; the band rendered in warning colours beneath the existing blocking-work band.npm run verify:releasenot run: not a release or handoff-confidence claim.npm run eval:retrieval:qualitynot applicable: no retrieval, ranking, selection, chunking or scoring behaviour changed. See the RAG note below.npm run check:production-readinessnot run: no clinical workflow, privacy, environment, Supabase, source-governance or deployment behaviour changed. This page reads two committed files and renders text.Risk and rollout
discharge-documentationinsidequality-discharge-documentation;patient-safety-planinsidequality-patient-safety-plan-documents), so a plain substring match reported five questions where three were real, putting two clinical questions on screen that nothing had reported as broken. Matching is therefore whole-token, and the reverse case — a short id genuinely named in its own right — is pinned by its own test, so the fix cannot be traded for a false negative.data/outstanding-issues-snapshot.jsonand the eval case list). No network call, no Supabase read, no write path.src/lib/rag/rag-eval-cases.tsis imported read-only for case ids and question text; no file undersrc/lib/rag/**is modified by this branch (git diff --name-only origin/main HEADmatchingsrc/lib/rag/returns nothing), andclassifyPullRequestFilesreportsragRanking: false. Declared explicitly rather than left implicit, because this repository asks for retrieval-surface contact to be flagged even when it is only a read.Clinical Governance Preflight
classifyPullRequestFilesreportsclinicalRisk: truefor this diff.Unchanged. This page makes no clinical claim of its own: every line it shows is quoted from a committed ledger item or from the eval case list, each shown with its ledger id and case id so a reader can check it against the source record.
No patient data of any kind is read or rendered. The inputs are repository metadata about test questions and outstanding work.
Clinical KB Database(sjrfecxgysukkwxsowpy)No environment or project configuration was touched, and this page performs no database access.
No Supabase client of any kind is constructed by this change.
Unchanged, and the page inherits the hub's standing "synthetic data only" warning. The questions it names are evaluation cases, labelled as such.
This is where most of the care went. The panel is named for its evidence ("Open problems recorded against a named clinical question"), states above the list that a recorded problem which does not name a case is not shown, and renders an empty result in words saying that a quiet ledger is not a well answer engine. On a clinical surface the reassuring-looking empty state is the dangerous one, so both the caveat and the empty wording are pinned by tests rather than left to editorial goodwill.
No clinical decision-support behaviour changed. This is developer-facing reporting about test questions, on a page that 404s for everyone but a signed-in administrator.
Notes
Stacked on #2495 while that was open, and brought onto
mainby merge once it landed. The conflicts were the ordinary consequence of a squash merge: main carries the base work as one commit, so git cannot match it against this branch's originals.data/repo-awareness-snapshot.json,docs/site-map.mdanddata/outstanding-issues-snapshot.jsonare generated. The conflict in the first was resolved by regenerating, never by hand, as this repository requires.