Skip to content

Show which documents finished indexing and produced nothing usable - #2504

Merged
BigSimmo merged 13 commits into
mainfrom
claude/corpus-health-panel-na3hbs
Sep 1, 2026
Merged

Show which documents finished indexing and produced nothing usable#2504
BigSimmo merged 13 commits into
mainfrom
claude/corpus-health-panel-na3hbs

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds a Corpus health panel to the developer hub, answering the one question the hub could not answer before: which of the signed-in owner's documents are broken. The existing ingestion panel shows documents moving through the queue; this one shows the library at rest, which is where a document that finished and produced nothing usable hides.
  • src/lib/developer-area/corpus-health.ts reads two owner-scoped tables through the cookie-bound user client: public.documents (status, chunk/page/image counts, error message) and public.document_index_quality (score, extraction label, issues). Row-level security (documents owner read, index quality owner read, each owner_id = auth.uid()) does the scoping, so the module never sees another owner's library.
  • The page reports counts by state; documents that are indexed with zero chunks; failures with the reason the worker recorded; the extraction-quality distribution; and the lowest-scoring documents with their recorded issues.
  • One entry added to hub-panels.ts (group system, phase 1). docs/site-map.md and data/repo-awareness-snapshot.json regenerated.

The two rules the module is built around

The user-session client, never the service-role admin client. createAdminClient bypasses row-level security and would report every owner's library to whoever happened to be signed in. No behavioural test can catch that substitution — the mocks answer either client identically — so the module carries the same source assertion environment-facts.ts uses.

Every failure returns null, never 0, and every read is guarded separately. On this panel 0 is the reassuring answer ("nothing failed", "nothing is unsearchable"), so a read that did not happen must never be able to impersonate it. The reads are guarded one at a time because the Supabase client rejects rather than resolving with an { error } when a request is aborted or exhausts its network retries, and an unhandled rejection would fail the whole page rather than degrade one line of it — during exactly the outage that makes this page worth opening. An unread count renders as the words "Not read", and a page that read nothing says so instead of showing a healthy-looking wall of zeroes.

What the page refuses to imply

A panel on a clinical system that implies coverage it does not have is worse than no panel, so each caveat is page content, sits above the evidence it qualifies, and is pinned by a test:

  1. Every figure describes what indexing produced. Nothing here reads answer quality, retrieval accuracy, or whether the extracted text is correct — an empty page means indexing ran to completion, not that the answers are good.
  2. "Finished but unsearchable" is status = indexed and chunk_count = 0. Other index units and page images may still exist for those documents, so it is a strong signal of a broken extraction rather than proof that nothing about the document is reachable.
  3. An empty unsearchable list is the absence of the worst failure, not proof of a good index — the test is chunk count alone, and a document can hold chunks that are empty or mis-extracted and still pass it.
  4. The failures list is current state, not history: a document that failed and was later reindexed successfully leaves nothing behind.
  5. quality_score defaults to 0 and extraction_quality to unknown, so a row carrying those values may never have been scored rather than having scored badly. document_index_quality.owner_id is nullable while the read policy matches on it, so a row written without an owner is invisible here — an honest reason this total can sit below the indexed count.

The uniform-score finding

I was asked to check an unverified report that every document in the library carries an identical placeholder quality score, which would make the quality half of this panel meaningless. I could not check it: a cloud container has no live database, and this repository holds no committed record of the live score distribution. So the panel is built to report that condition rather than to hide it. resolveQualitySpread reads both ends of the score range and distinguishes five cases — unreadable, none, a single row, every row identical, and a genuine range. When the scores are uniform the page says so in bold, states that the quality figures are not usable as a measure of any document, and adds that a uniform 0 is also what a corpus nobody ever scored looks like. If the live data turns out to be uniform, that is a finding to raise, not a panel to quietly ship.

Verification

  • npm run verify:pr-local — green, all 19 steps completed, none failed:
 Test Files  941 passed (941)
      Tests  11842 passed | 4 skipped (11846)
✓ Compiled successfully in 2.4min
- completed: check:runtime, check:installed-lock-parity, format:changed, sitemap:check,
  check:repo-awareness-snapshot, docs:check-index, docs:check-inventory, docs:check-scripts,
  docs:check-links, check:branch-review-ledger, check:outstanding-issues,
  check:ledger-write-discipline, lint, typecheck, test, build, check:rag:fixtures,
  check:medication-interactions, check:medication-lexicon-report
- failed: (none)
- not reached: (none)

An earlier run of the same gate failed two tests, and neither was this diff: tests/clinical-hazard-controls.test.ts (reviewedCommit does not exist 883f100…) and tests/rag-plan-package-parity.test.ts (manifest reconciledBase is unavailable locally: f3d1a3c…). The container's clone was shallow — 137 commits — so both objects were simply absent. git fetch --deepen brought them in, both tests pass, and the run quoted above is on full history. Recorded rather than dropped, because "it was the environment" is a claim worth being able to check.

  • npm run verify:ui when UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changed
  • npm run verify:release before release or handoff confidence claims

UI verification not run: the change adds one developer-hub panel route built from the existing PanelPageShell / CountTile / PanelSection primitives with no change to shared chrome, tokens, or the search composer, and Chromium journeys do not cover /mockups/development/**. Its rendering is covered by a jsdom page contract test.

  • npm run eval:retrieval:quality — not applicable; no retrieval, ranking, selection, chunking, or scoring behaviour is touched.
  • npm run eval:rag -- --limit 15 + npm run eval:quality -- --rag-only — not applicable; answer generation is untouched.
  • npm run check:production-readiness — provider-backed, and not run. This change adds no clinical workflow, privacy, environment, Supabase, source-governance, or deployment behaviour: it is a read-only panel over two existing tables through the existing user client, and it adds no migration, no environment variable and no new access path.
  • npm run check:deployment-readiness when deployment startup, hosting, or rollout behavior changed

Proven by mutation, not by a passing test

Each load-bearing rule was broken on purpose, the specific test watched go red, then the file restored and confirmed byte-identical by SHA-256. Eleven mutations, eleven reds, no survivors:

  1. Admin client swapped in for the user client → source assertion red.
  2. A failed count reports 0 instead of null → "reports a missing count as unavailable rather than as zero" red.
  3. A failed list reports a zero count → "keeps an empty library distinct from a count it could not read" red.
  4. The rejection guard around a list read removed → "degrades one unreadable line and keeps every other reading when a query rejects" red.
  5. The counts run for a request with no signed-in user → "issues no query at all for a request with no signed-in user" red.
  6. Half a score range described as a distribution → "reports the score range as unread when either end of it rejects" red.
  7. Uniform scores no longer named → "says outright when every scored document carries the identical score" red.
  8. An unread count renders as a zero tile → "renders a count it could not read as words, never as zero" red.
  9. The scope caveat paragraph deleted → "states the limits of its own coverage above the evidence" red.
  10. The empty state stops saying what it does not prove → "says what an empty result does not prove" red.
  11. (First attempts at 9 and 10 replaced only the opening words and left the asserted claim standing; both survived, were redone as real deletions, and then went red. Recorded because a mutation that survives for the wrong reason is not evidence either way.)

Risk and rollout

  • Risk: Low. One new read-only developer-hub route behind the existing DeveloperAreaGate, plus one card on the hub. No existing surface changes behaviour. The realistic failure is a wrong or over-confident number on the panel, which the null-not-zero rule, the per-read guards and the wording tests are built to prevent.
  • Rollback: Revert the commit. The route and the hub card disappear together; nothing else depends on either.
  • Provider or production effects: None. No migration, no schema change, no environment variable, no new provider call. At runtime the panel issues eleven read-only PostgREST requests for a signed-in developer who opens it — all owner-scoped by existing row-level security, all against tables the app already reads. No provider-backed gate was run.
  • RAG impact: no retrieval behaviour change — nothing under src/lib/rag/**, retrieval-selection, ranking, the eval harness or the golden fixtures is touched; this is a read-only panel over the documents and document_index_quality tables.

Clinical Governance Preflight

  • Source-backed claims still require linked source verification before clinical use
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
  • Service-role keys and private document access remain server-only
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative
  • Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed

Evidence for each, in order:

  1. The panel makes no clinical claim and renders no clinical content. It reports index bookkeeping — counts, statuses, error strings and quality labels — and never document text, so nothing here can be relied on in place of a linked source.
  2. No document workflow was added or widened. The panel is read-only, adds no upload, export, sharing or retrieval path, and shows document titles and ids that the same signed-in owner already sees on the documents surface.
  3. Untouched. No migration, no Supabase configuration change, and no project ref appears anywhere in the diff.
  4. The module imports createSupabaseServerClient and carries import "server-only". A source assertion test fails if supabase/admin ever appears in it, by import, dynamic import or re-export — this is the rule mutation 1 above proves is enforced.
  5. Demo mode has no Supabase configuration, so createSupabaseServerClient returns null, no read is attempted, and the page renders the explicit "Nothing was read" notice naming the demo corpus as one of the two causes. It never renders synthetic figures as though they were the real library.
  6. More conservative, not less: every unreadable value degrades to "Not read" rather than to a number, an empty result states in words what it does not prove, and the quality section refuses to present a uniform placeholder score as a measurement.
  7. Checked, and the classification is unchanged: this adds no decision-support behaviour. The panel is an internal developer diagnostic behind the developer-area gate, it produces no clinical output, and it does not influence retrieval, ranking or answer generation.

Notes

  • Real-data confirmation is outstanding. This was built in a cloud container with no live database and no browser access to the running app, so every test uses stand-in data and nobody has yet seen this panel against the real library or on screen. Two things need a local run before the numbers are trusted: that the eleven reads return what the panel expects from the live schema, and what the quality section actually reports — see the uniform-score finding above.
  • The ReadingTile helper (a count tile that can say "Not read") is deliberately local to the page rather than added to panel-primitives: this is the only panel whose numbers come from a live database read and can therefore be missing. It should move there the day a second page needs it.
  • Counts are computed in Postgres with head: true rather than by counting fetched rows, because PostgREST caps the rows it returns and a truncated fetch would under-report breakage — the one direction a panel about breakage must not fail in. A test pins that.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XG7wQurapeZwWRsNhHA1PY


Generated by Claude Code


Note

Medium Risk
New administrator-gated reads bypass RLS via the service role, so correctness depends on every query applying the verified owner_id filter; tests and source assertions target that, but a regression could expose cross-owner data.

Overview
Adds a Corpus health developer-hub panel at /mockups/development/corpus-health plus a hub card, wired through resolveCorpusHealth / resolveQualitySpread in corpus-health.ts.

Data access: Session client only resolves a signed-in administrator; table reads use the service-role admin client with .eq("owner_id", …) on every query (authenticated SELECT on documents / document_index_quality is not available). Failed or partial reads surface as null / “Not read”, not 0, with per-query guards so one outage does not take down the page. Totals use Postgres head: true exact counts; lists are capped with explicit truncation copy.

UI: Live status tiles, “finished but unsearchable” (indexed + zero chunks), failed jobs with error text, extraction-quality buckets, lowest-scoring samples, and copy that calls out uniform/placeholder quality scores and what the panel does not measure (answer/retrieval quality).

Tests & docs: Module + page DOM contracts (owner filter, non-admin issues no queries, null-not-zero, uniform-score messaging); repo-awareness snapshot, site map, codebase index, and a branch review ledger row updated.

Reviewed by Cursor Bugbot for commit 3b1dc7c. Configure here.

…ced nothing usable

The hub's ingestion panel shows documents *moving* through the queue. Nothing
showed the library at rest, which is exactly where a document that finished and
produced nothing usable hides: it is not queued, it is not an error, and its row
says `indexed`.

`corpus-health.ts` reads two owner-scoped tables through the cookie-bound user
client -- `public.documents` and `public.document_index_quality`. Row-level
security (`documents owner read`, `index quality owner read`, each
`owner_id = auth.uid()`) is what does the scoping, so `createAdminClient` would
silently turn one account's library into every account's. No behavioural test can
catch that substitution, because the mocks answer either client identically, so
the module carries the same source assertion `environment-facts.ts` uses.

Every failure returns `null`, never `0`, and each read is guarded on its own. On
this panel `0` is the reassuring answer -- "nothing failed", "nothing is
unsearchable" -- so a read that did not happen must not be able to impersonate
it. The per-read guard is because the client *rejects* rather than resolving with
an `{ error }` when a request is aborted or exhausts its retries, and an
unhandled rejection would fail the whole page instead of degrading one line of
it, during exactly the outage that makes the page worth opening. An unread count
renders as "Not read"; a page that read nothing says so rather than showing a
healthy-looking wall of zeroes.

What the page refuses to imply, each pinned by a test and each above the evidence
it qualifies: every figure describes what indexing *produced*, so an empty page
means indexing ran to completion and not that the answers are good; zero chunks
is a strong signal of broken extraction rather than proof nothing is reachable;
an empty list is the absence of the worst failure rather than proof of a good
index; the failures list is current state, not history; and `quality_score`
defaults to 0 while the quality table's owner column is nullable, so a low total
there has honest explanations that are not breakage.

`resolveQualitySpread` exists for one reading in particular. A score column that
defaults to 0 and a scorer that never ran produce a perfectly tidy distribution,
so the page names a corpus where every document carries the identical score
instead of rendering it as a measurement. Whether the live library is in that
state could not be checked here and is called out on the PR.

Proven by mutation, not by a passing test: eleven load-bearing rules broken on
purpose, each watched red in its own named test, each file then restored and
confirmed byte-identical by SHA-256. Two first attempts survived because they
replaced only the opening words of a caveat and left the asserted claim standing;
both were redone as real deletions and then went red.

Gate: `npm run verify:pr-local` green -- 941 test files, 11,842 tests passed,
4 skipped; build compiled successfully in 2.4min; all 19 steps completed, none
failed. An earlier run failed `tests/clinical-hazard-controls.test.ts` and
`tests/rag-plan-package-parity.test.ts` on missing git objects; the clone was
shallow, and both pass after `git fetch --deepen`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XG7wQurapeZwWRsNhHA1PY
@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 974ecfbe-c1ef-4b79-be02-c409473ef0f9


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@supabase

supabase Bot commented Sep 1, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-01T13:21:07.198133Z 4eebf12 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cursor

cursor Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot 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_d2dda0f3-943f-4f4d-a452-85d725f37fc6)

@BigSimmo
BigSimmo enabled auto-merge September 1, 2026 13:16

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4eebf126ac

ℹ️ 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".

Comment thread src/lib/developer-area/corpus-health.ts Outdated

BigSimmo commented Sep 1, 2026

Copy link
Copy Markdown
Owner Author

The Codex P1 is correct, and it is wider than this PR

I checked it against the schema rather than taking it on trust, and it holds.

Primary source. supabase/schema.sql:5299 runs revoke all privileges on all tables in schema public from anon, authenticated;, and the grant select, insert, update, delete block that follows names public.documents and public.document_index_quality for service_role only. The comment immediately below the block says so in as many words: "Browser clients do not receive direct table privileges. Public and signed-in access is mediated by the server routes, while the owner policies below stay in place as defense in depth if the Data API posture changes later." So the owner-read policies are a second layer behind a privilege the authenticated role does not hold, and RLS cannot restore a missing SQL SELECT.

It is the current posture, not a stale mirror. supabase/migrations/20260725000000_audit_security_remediation.sql:81 re-applies the same blanket revoke, which postdates every earlier grant select … to authenticated (rag_retrieval_logs, import_batches, document_index_units, and the rest). I checked every migration after 20260725000000 for a re-grant: the only to authenticated hits are RLS policies in 20260823090000_user_favourite_sets.sql, not privileges. Nothing restores table SELECT to authenticated.

Consequence, exactly as reported. All eleven reads would fail with permission denied, every guard would convert that to null, and the panel would render "Not read" in every position. The failure is silent by construction — which is the honest degradation this module was built for, and also what would have hidden the defect if the panel had shipped.

This is not confined to this PR. src/lib/developer-area/environment-facts.ts, merged earlier today in #2498, counts public.documents through the same cookie-bound client and is subject to the same revoke, so the hub's environment strip can only ever say "document count unavailable". Its unit tests pass because they mock the client, and its source-assertion test pins the very client that has no privileges. This PR copied that module deliberately — it was the stated model for the read — so the pattern, not just this diff, is what needs the decision.

The repository's own working pattern for this read is the sibling ingestion panel: src/app/api/ingestion/jobs/route.ts uses createAdminClient() plus requireAuthenticatedUser(request, supabase, { administrator: true }) and an explicit owner filter. In src/lib and src/app/api, the only modules using createSupabaseServerClient are the auth gate (access.ts, which makes auth calls and reads no table), environment-facts.ts, and this new module.

Not pushing a fix yet, deliberately. Both viable repairs are the owner's call rather than mine:

  1. Admin client with an explicit owner_id filter on every query, behind the existing administrator gate, matching the ingestion route — with a test that fails if any query omits the filter, replacing the current source assertion. This works today and adds no migration, but it moves owner scoping from the database to application code on a private clinical library, which is exactly what the instruction for this panel forbade.
  2. A narrowly scoped security definer RPC filtering on auth.uid(), with execute granted to authenticated. This keeps scoping in the database, and it needs a migration — which in this repository deploys to the live clinical database on merge and so needs an approved window.

Guessing between those two would be the wrong move on this surface, so I have put the choice to the repository owner. Everything else on the PR is green: PR policy, PR mergeability, Build, Static PR checks, Unit coverage, Safety and config checks, Lighthouse budget and every Production UI shard have passed on 4eebf12.


Generated by Claude Code

@BigSimmo
BigSimmo disabled auto-merge September 1, 2026 14:14
@BigSimmo
BigSimmo enabled auto-merge (squash) September 1, 2026 14:14
@cursor

cursor Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot 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_366959ae-2a86-4258-a272-6cb858373b72)

@BigSimmo
BigSimmo merged commit c944fcd into main Sep 1, 2026
29 checks passed
@BigSimmo
BigSimmo deleted the claude/corpus-health-panel-na3hbs branch September 1, 2026 15:11
BigSimmo pushed a commit that referenced this pull request Sep 2, 2026
…iering branch

Bundled into this PR rather than pushed as a separate ledger-only tip, matching
how every recent review record landed (#2517, #2508, #2504).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ou4w8atP9kDKHVAXUvYPe
Sign up for free to 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