Hand over the corpus health panel and the privilege trap behind it - #2539
Conversation
…ege trap behind it Both changes are merged and nothing is in flight, so this records what a fresh session needs and what it must not undo. The substance is the finding both fixes exist for: the cookie-bound user client cannot read `documents` or `document_index_quality` at all. `schema.sql:5299` revokes all `public` table privileges from `anon`/`authenticated`, migration `20260725000000` re-applies that revoke after every earlier grant, and no later migration restores it -- so the owner-read policies sit behind a privilege the role does not hold, and a policy cannot hand back a missing SQL SELECT. Both modules degraded the resulting permission-denied to `null` by design, which is why the hub's document count rendered "document count unavailable" from the day it shipped and nobody saw it. The variant check is recorded as closed rather than left as a hunt: every `createSupabaseServerClient` caller in `src/` was audited on main at 45a3dca. `access.ts` and the auth callback are auth-only; `document-source-loader.ts` already used the correct shape (user client for the viewer id, admin client plus `withOwnerReadScope` for the query). No third instance exists. Also records the six rules a later change must not undo -- chiefly that the explicit `owner_id` filter is now the whole owner-scoping guarantee rather than a second layer over row-level security, and that restoring the user client would read nothing while looking healthy -- and names the tests that hold each one. What is NOT verified is stated as plainly: neither surface has been seen against the real library or on screen, because a cloud container has neither. The handover carries the exact confirmation steps, including reading which of the five `resolveQualitySpread` cases the live data reports -- the unverified uniform-placeholder-score report is the reason that derivation exists, and a `uniform` reading is a finding against the scoring pipeline rather than a panel defect. A ledger inbox request queues the same confirmation, so a fresh session is told at startup rather than having to find this file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XG7wQurapeZwWRsNhHA1PY
|
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 |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
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. |
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_e76832a5-09bb-4902-84c8-79e5749e676f) |
…panel-na3hbs # Conflicts: # data/repo-awareness-snapshot.json
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab8ee0825f
ℹ️ 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".
…coring is broken Raised as a P2 by Codex on #2539 and verified against the source before acting. `assessDocumentIndexQuality` starts `qualityScore` at 1 and only subtracts penalties, then rounds to three decimals (`src/lib/index-quality.ts:138-167`). A corpus that extracted cleanly -- no issues, no duplicate chunks, adequate heading and section-path coverage, nothing visual to score -- therefore lands on `1.000` for every document, legitimately. Equality proves an absence of observed spread and nothing more, so the false positive is not a corner case: it is what a healthy library looks like. Three surfaces carried the overclaim and all three are corrected: - The panel told the reader the figures were unusable "until scoring writes real values", which presumes a fault. It now says the tie cannot rank or separate the documents, that this is not by itself evidence the scorer is broken and why, and to read the issues and metrics on the rows before concluding anything. The `0.00` note stays as it was: zero is the column default, so that reading really is what a corpus nobody scored looks like. - The handover recorded a uniform reading as confirming the placeholder-score rumour. It now separates the strong case (uniform `0.00`, where `extraction_quality` also defaults to `unknown`) from the weak one (uniform `1.00`), and requires corroboration from the stored issues, metrics and the worker write path before anything is recorded against scoring. - The queued ledger request said the same thing and is replaced with a corrected one. It had not reached `main`, so nothing immutable was rewritten. This is the overclaiming the panel exists to avoid, in the panel's own prose -- the reason its caveats are page content pinned by tests rather than comments. The corrected wording is pinned the same way, proven by deleting the caveat and watching "says outright when every scored document carries the identical score" go red, then restoring the file byte-identical by SHA-256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XG7wQurapeZwWRsNhHA1PY
|
What failed. One test, Why it is not this PR's. The diff is one Markdown file, one ledger request and two regenerated snapshots — no executable code, so the mockup renders from exactly the same sources as on So the page correctly renders The fix, for whoever picks it up. Not ported here, because no fix exists to port yet and adding one would make a documentation-only PR carry an unrelated executable change — including through its governance preflight, which states no executable change. The count is incidental to what this test is actually about (that - await expect(mockup.getByText("1 tool", { exact: true })).toBeVisible();
await expect(mockup.getByRole("heading", { level: 2, name: "Risk & Safety" }).first()).toBeVisible();Not verified in a browser from here — this container has no display and the check above was the ranking function, not the rendered page. Worth confirming the heading is the first result before landing it. Generated by Claude Code |
…panel-na3hbs # Conflicts: # data/outstanding-issues-snapshot.json
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_fc1c810d-2095-4d53-a643-4036d50d414f) |
|
Updating my note above on one point: a fix now exists. #2554 carries it — it drops the absolute count and asserts the ranking instead, which is what that test is named for. Deliberately not ported into this PR. The usual reason to port a fix rather than wait is that the failure blocks the PR; this one cannot — Everything else above stands: deterministic, not a flake, and not caused by this diff. Generated by Claude Code |
Second sync: main advanced again (PR #2539) touching the same generated data/repo-awareness-snapshot.json. Regenerated with npm run snapshot:repo-awareness against the merged tree rather than hand-resolving the conflict. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FDiC2BK8XcPbstaJf7So2x
Summary
docs/corpus-health-panel-handover.md, a handover for the corpus health panel (Show which documents finished indexing and produced nothing usable #2504) and the environment-strip document count fix (Read the hub document count with a credential that has privileges #2512). Both are merged; nothing is in flight. Documentation and a ledger request only — no executable change.data/repo-awareness-snapshot.jsonanddata/outstanding-issues-snapshot.json, which the new document and the new request make stale.What the handover records
The finding both fixes exist for. The cookie-bound user client cannot read
documentsordocument_index_qualityat all:schema.sql:5299revokes allpublictable privileges fromanon/authenticated,20260725000000_audit_security_remediation.sql:81re-applies that revoke after every earlier grant, and no later migration restores it. A policy cannot hand back a missing SQLSELECT, so the owner-read policies sit behind a privilege the role does not hold. Both modules degraded the resulting permission-denied tonullby design, which is why the hub's document count rendered "document count unavailable" from the day it shipped and nobody noticed.The variant check, recorded as closed rather than left as a hunt. Every
createSupabaseServerClientcaller insrc/was audited onmainat45a3dcacb:access.tsandsrc/app/auth/callback/route.tsare auth-only with no table read;src/lib/sources/document-source-loader.tsalready used the correct shape (user client for the viewer id, admin client pluswithOwnerReadScopefor the query); the two developer-area modules are fixed. No third instance exists.Six rules a later change must not undo, each named with the test that holds it — chiefly that the explicit
owner_idfilter is now the whole owner-scoping guarantee rather than a second layer over row-level security, and that restoring the user client would read nothing while looking healthy.What is not verified, stated plainly: neither surface has been seen against the real library or on screen, because a cloud container has neither. The document carries the exact confirmation steps, including reading which of the five
resolveQualitySpreadcases the live data reports. The unverified uniform-placeholder-score report is why that derivation exists — but auniformreading is a prompt to investigate, not a verdict.assessDocumentIndexQualitystarts at1and only subtracts penalties before rounding to three decimals (src/lib/index-quality.ts:138-167), so a cleanly extracted library legitimately scores1.000on every document. Uniform0.00is the strong reading, because zero is also the column default; uniform1.00needs corroboration from the storedissuesandmetricsbefore anything is recorded against scoring. (Raised in review of this PR and corrected on the panel, the handover and the queued request inc12632a03.)Verification
npm run verify:pr-local— green, all 19 steps completed, none failed:An earlier run of this same change failed
check:repo-awareness-snapshot, because a new file underdocs/makes the committed snapshot stale. Regenerated with the command the gate names and re-run clean. Recorded rather than dropped.npm run verify:ui— not applicable; no component, route, style or browser behaviour changes.npm run verify:releasebefore release or handoff confidence claimsnpm 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 PR adds no executable code: a Markdown document, an append-only ledger request, and two regenerated snapshots.npm run check:deployment-readinesswhen deployment startup, hosting, or rollout behavior changedRisk and rollout
Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Evidence for each, in order:
Notes
docs/outstanding-issues-inbox/f5078359-d4ea-4a91-b376-d5bbecb452a6.json. Runnpm run issues:reconcilefrom a dedicated fresh-base branch after this lands; it is append-only and merge-safe until then. (It replaced42fd3baa-589b-4f2e-9434-dc259810b9ef.json, which carried the same uniform-score overclaim; that request never reachedmain, so no immutable record was rewritten. The Cursor Bugbot note below still names the withdrawn id because it reviewed the earlier commit.)🤖 Generated with Claude Code
https://claude.ai/code/session_01XG7wQurapeZwWRsNhHA1PY
Generated by Claude Code
Note
Low Risk
Documentation and generated ledger snapshots only; no runtime, migration, or credential behavior changes in this diff.
Overview
Adds
docs/corpus-health-panel-handover.mdso a later session knows what merged in #2504/#2512, why the cookie-bound Supabase client could not readdocuments/document_index_quality, and six rules (service-role reads, explicitowner_id,nullnot0, etc.) that must not be reverted. It also records that nothing has been verified against a live library and lists concrete confirmation steps.Queues ledger inbox
42fd3baa-589b-4f2e-9434-dc259810b9ef(action: add) so startup/reconcile surfaces a P2 task to exercise the developer hub document count and/mockups/development/corpus-healthwith real Supabase + an admin session.Regenerates
data/repo-awareness-snapshot.json(new doc entry, refreshed revision/counts) anddata/outstanding-issues-snapshot.json(ledger SHA,pending: 0 → 9, including this add plus other pending reconcile actions bundled in the snapshot). No application or schema code changes.Reviewed by Cursor Bugbot for commit ab8ee08. Configure here.
Note
Cursor Bugbot is generating a summary for commit 8338b04. Configure here.