docs(issues): re-apply #102's canary-gated correction lost to a merge - #1440
Conversation
Codex raised this as a P2 on PR #1423: with the runbook corrected, ledger #102 still told an operator the `(status,id)` use was ordering-safe, so the two operator entry points contradicted each other. A reply on that thread claimed the fix landed in 2bdc245. It did not reach `main`. Verified by content rather than by SHA, which matters here: #1423 was squash-merged, so `git merge-base --is-ancestor` returns NO for every original commit and proves nothing either way. Reading `docs/outstanding-issues.md` at `origin/main` (0310c6a) shows the retracted sentence still present, while the runbook correction and the test change from the same PR both landed. Most likely cause, and it is a documented hazard: #1428 ("archive five completed ledger rows") touched this same file on `main`, and #112 records that `docs/outstanding-issues.md` conflicts present as ordinary content conflicts a hurried resolution can settle by dropping one side. Union merge has since been added to this file, but it would not have saved this case — both sides edited the same sentence. The row now mirrors the runbook's classification and names the runbook as the authority if the two ever diverge again, so a future reader has a tie-breaker rather than two equal-looking claims. Gates: verify:cheap exit 0, Test Files 434 passed (434), Tests 4563 passed | 4 skipped. Outstanding-issues guard, branch-review-ledger guard, docs:check-links, docs:check-scripts, prettier all pass. No SQL applied; no index created. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 53 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![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: fc09ea5a75
ℹ️ 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".
Addresses the Codex P2 on this PR and fixes my own mistake in the first attempt.
The correction itself is unchanged in substance: `documents_status_id_idx` is
canary-gated, not ordering-safe, because `rag-candidate-sources.ts:482` pairs
`.eq("status","indexed")` with an unordered `.limit(12)` and `(status, id)`
serves that equality. The reasoning stays in the runbook, which is the
authority; the ledger row now just points at it.
What changed is the shape. My first version added prose to #102's Detail cell,
which is the widest cell in that column — so Prettier repadded every row and the
diff became 63 insertions / 63 deletions. Row #133 records that this exact
whole-table behaviour has already caused mechanical conflicts that silently
stopped all CI, and it is very likely how the original correction was lost
between #1423 and main in the first place.
Three attempts established the rule empirically, since my first two hypotheses
were both wrong: growing the cell reflows (68 lines), *shrinking* it also
reflows (68 lines, because the column width tracks this cell), and trailing
whitespace padding does not help because Prettier trims it. The replacement is
therefore exactly the same length as the text it replaces — 97 characters — and
the diff is now 1 insertion / 1 deletion with Prettier reporting the file
unchanged.
Verified on an untouched row: before this, #100's Detail cell lost exactly one
padding space (3533 -> 3532 chars), which is what proved the column width, not
my cell's content, was driving the churn.
Gates: outstanding-issues guard 137 rows / unique ids; branch-review-ledger 117
live + 1206 archived, no duplicates; prettier clean. No SQL applied.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
2 participants
Summary
Docs only, one sentence of substance. No SQL applied, no index created.
Codex raised this as a P2 on PR #1423: with the operator runbook corrected, ledger
#102indocs/outstanding-issues.mdstill told an operator the(status,id)use was ordering-safe, so the two operator entry points for the same task contradicted each other. A reply on that thread stated the fix had landed in2bdc245. It did not reachmain.#102now mirrors the runbook's classification:documents_status_id_idxis canary-gated, becauserag-candidate-sources.ts:482is one statement carrying both.eq("status","indexed")and an unordered.limit(12), and(status, id)serves that equality — so a new plan can change which twelve title-alias rows reach candidate assembly. Its other consumer,search-scope.ts:271-277, pages with an explicit.order("id")and is genuinely safe; that safety is not transitive. The row also names the runbook as the authority if the two ever diverge again, so a future reader gets a tie-breaker instead of two equal-looking claims.RAG impact: no retrieval behaviour change — this PR changes one sentence of prose in a tracking ledger. It applies no index and touches no retrieval, ranking, selection, alias or citation code. Its effect on retrieval is the opposite of a behaviour change: it restores a withdrawal of the claim that an index is safe to apply without a live eval-canary pair, making the documented gate stricter rather than looser.
How the loss was found, and why the usual check would have missed it
Worth recording, because the obvious verification gives the wrong answer here.
#1423 was squash-merged. So
git merge-base --is-ancestor <original-sha> origin/mainreturnsNOfor every commit that was on the branch — including ones that definitely landed. Ancestry proves nothing either way after a squash, and reading it as "the fix is missing" or "the fix is present" would both be unfounded.The check that works is content. At
origin/main(0310c6af):docs/operator-apply-performance-latency-remediation.md— correction present (CORRECTED AGAIN 2026-07-30, 3 ×CANARY-GATEDin the SQL block)tests/ui-phone-scroll.spec.ts— change present (targetScrollTop, renamed test)docs/outstanding-issues.md#102— retracted sentence still thereTwo of three items from the same PR landed; one did not.
Most likely cause, and it is a documented hazard.
#1428("archive five completed ledger rows") touched this same file onmain, and ledger#112records thatdocs/outstanding-issues.mdconflicts "present as an ordinary content conflict that a hurried resolution can settle by dropping one side's rows entirely". Union merge has since been added to this file — good, and it closes part of#112— but it would not have saved this case, because both sides edited the same sentence. I am stating this as the probable mechanism, not a proven one: I did not reconstruct the branch's merge history, and the branch has since been deleted.Verification
npm run verify:cheap— exit 0,Test Files 434 passed (434),Tests 4563 passed | 4 skipped (4567).node scripts/check-outstanding-issues.mjs—Outstanding-issues guard passed: 132 rows (61 open, 71 archived), unique ids, next-id=135 above the highest, union merge active.npm run check:branch-review-ledger—114 live table records + 1206 archived … no conflict markers, mojibake, heading records, or duplicates.npm run docs:check-links—docs link check passed: 1354 repo path references resolve.npm run docs:check-scripts—docs script-ref check passed: 377 npm-run reference(s) resolve to real scripts.npx prettier --writethen clean.src/lib/rag/rag-candidate-sources.ts:470-495, not copied from either document.Not run, and why: no browser or build gate applies to a prose change in
docs/**.check:driftnot run —supabase/**untouched. Every provider-backed gate (eval:*,verify:release,check:supabase-project,test:live) not run and not implicated.Risk and rollout
#102alone would have retaineddocuments_status_id_idxwithout the required retrieval before/after evaluation.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes on the above: this PR changes one sentence of tracking prose, so every item is satisfied by construction rather than by inspection of new behaviour. No answer generation, citation, verification, source-governance or document-access code is touched; no credential or Supabase value appears in the diff; the pinned Supabase target is referenced only as unchanged context. Clinical decision-support behaviour is unaffected, so the SaMD classification is unchanged. The clinically-relevant effect is indirect and protective: the tracking ledger no longer tells an operator that a retrieval-path index is safe to apply without the canary this repository's own rules require.
Notes
origin/mainafter docs+test: correct the ordering-safe index claim, and make the near-bottom refusal guard non-vacuous #1423 merged, keeping the same name, per the merged-PR rule — a merged PR cannot track new work. This is a new PR.#102's status is unchanged: the operator apply is still outstanding. This corrects its instructions only.🤖 Generated with Claude Code
https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
Generated by Claude Code