Improve search RAG validation and retrieval quality - #109
Merged
Conversation
# Conflicts: # scripts/classify-documents.ts # src/lib/rag.ts
BigSimmo
marked this pull request as ready for review
June 30, 2026 09:35
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo pushed a commit
that referenced
this pull request
Jul 29, 2026
Branch-cleanup review of all 92 remote branches against main, recorded with scope `branch-cleanup-deletion-pending` because deletion could not be completed: the session git proxy rejects ref deletion with HTTP 403 and no delete-branch capability is exposed. Five branches are verified safe to delete (empty diff against main, no open PR) and are tracked as #108. The other 87 were deliberately not cleared — their touched files still differ from main, the conservative direction. #109 records the trap that made the first pass wrong: the session clone was shallow (74 of 2829 commits), which silently invalidated every merge-base, cherry-pick and ahead/behind result. In that state local main read as "ahead 52" with unrelated histories, and 90 of 91 branches read as unmerged. `git fetch --unshallow` corrected both, and the guide and sweep script should check for it rather than report silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YRCXgX4AWZ579bKN6sk6b
BigSimmo added a commit
that referenced
this pull request
Jul 29, 2026
…low-clone trap (#1387) Documentation and ledger bookkeeping only; the whole diff is docs/branch-review-ledger.md and docs/outstanding-issues.md. Records the PR #1383 prlanded verification (squash-vs-tip content diff empty, nothing orphaned by the auto-merge race), carries the babysit closeout row that the squash left behind, and files the branch-cleanup review with a per-candidate row keyed to each branch's own HEAD. New issue #109: remote sessions clone shallow, which silently invalidates every merge-base, cherry-pick and ahead/behind result. A sweep in that state wrongly reported 90 of 91 branches as carrying unmerged work; git fetch --unshallow corrected it. #108 records the ordering constraint that makes the cleanup executable: append each completed branch-cleanup ledger row BEFORE deleting the ref, because resolveHead refuses a HEAD that is no longer a commit in the repository, and the n/a escape hatch never satisfies hasCompletedCleanupReview. Clinical governance preflight not applicable: classifyPullRequestFiles reports clinicalRisk false for a docs-only diff.
BigSimmo pushed a commit
that referenced
this pull request
Jul 29, 2026
Resolved docs/outstanding-issues.md by keeping this branch's updated #98, #102, #104 and #105 entries (the ones this PR rewrites) and adding main's new #108 and #109 rows; the rest of the conflict was prettier re-padding column widths after a summary-cell length change. next-id stays at 110. Removed four exact-duplicate branch-review-ledger rows produced by the union merge driver keeping both copies of rows present on this branch and on main - the duplication risk tracked as #88.
BigSimmo pushed a commit
that referenced
this pull request
Jul 29, 2026
Resolves the `docs/outstanding-issues.md` conflict. Two independent causes: 1. Rows #98–#105 diverged. `main` carries the newer text (PR #1377 landed "Done 2026-07-29" outcomes and rewrote several summaries); this branch carried the pre-#1377 wording. Took `main`'s rows wholesale — the branch made no edits to that range, it simply forked before them. 2. `#108`/`#109` were allocated twice. `main` used them for the verified-landed branch deletions and the shallow-clone finding; this branch had used the same two ids for the design-system token manifest and the `ui-overlap` flake. The `issues:next-id` marker has no concurrency protection, which is exactly the failure the branch's own PR notes predicted. Renumbered this branch's two rows to #110/#111 and bumped the marker to 112. No content was dropped from either side; all 111 rows are unique. Also records #111 as done, since this branch is what fixes it: the ui-overlap inset measurement now retries inside `toPass` with the 2px symmetry tolerance and the assertions unchanged. Leaving it open with a "Next: apply the retry shape" action would have re-queued work this PR already did.
BigSimmo added a commit
that referenced
this pull request
Jul 29, 2026
(#1391) Follow-ups to #1375 (Clinical Sky design system), which merged before these landed. `text-4xs` was dead on main. #1375 retired the `--text-4xs` token, so Tailwind emits no such rule and every remaining `text-4xs` class was a silent no-op with the text falling back to inherited size. Two files carrying it landed from main while #1375 was in flight and were dead on arrival. Repointed to `text-3xs`, the 10px floor, and guarded: the design-token contract now fails if any tracked file under `src/` references the retired class outside a comment. Mockups are deliberately not exempt — a dead utility breaks a mockup exactly as it breaks production. Six hydration races fixed at the source. In this app visible never implies interactive: the server-rendered tree paints before React owns it, so a single un-retried action asserted once is a coin flip under load. Every fix retries the action together with the effect it should produce, so a swallowed first attempt retries while a genuine regression still fails. Both retries added to ui-smoke are idempotent, which review caught and which matters more than it looks. `toPass` schedules another attempt whenever the inner assertion's own deadline expires — which can happen after the click already landed. Without a guard, `openGuide` clicked a Settings trigger the modal was already covering, and the mode-menu retry clicked a TOGGLE a second time, closing a menu that had just opened and oscillating until the budget ran out. The mobile branch needed a second guard: a swallowed Settings click leaves the phone menu open, so re-running `openMobileClinicalGuideMenu` would toggle it shut and then fail to find Settings inside it — meaning the <768px branch could not recover on retry at all. The ui-overlap phone-inset measurement now retries inside `toPass` with the 2px symmetry tolerance and the assertions byte-for-byte unchanged, so a genuinely asymmetric header still fails once the retry budget is spent. Documentation records that the intermediate font weights on the variable face are deliberate rather than drift, and that redefining Tailwind's own leading names silently retunes every existing call site. Also resolves an `issues:next-id` double-allocation: #108/#109 were claimed concurrently by two branches, so this branch's two rows are renumbered #110/#111 and the marker moved to 112. #111 is archived rather than left open, since this change is what closes it. RAG impact: no retrieval behaviour change — test robustness, two mockup class names, and docs only; nothing under `src/lib/rag/**`, clinical-search, ranking-config, the eval harness or the golden fixture is touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvU8z73P6TXUXoYBqN5K1P
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
…when shallow CodeRabbit (Major) and Codex (P2) independently flagged the same hole in the guard added by the previous commit, and they are right. `tryGit` swallows every error into "", so the original `!== "true"` check treated an indeterminate result as proof of a complete clone. If `git rev-parse --is-shallow-repository` failed for any reason on a genuinely shallow clone, the sweep would emit exactly the merge-base-derived inventory this guard exists to prevent — the #109 defect wearing a different hat. The decision is now three-way, and only one branch proceeds: "false" -> complete history, sweep runs "true" -> shallow, refuse anything else, including "" -> INDETERMINATE, refuse The two refusals carry different remedies, since one needs `--unshallow` and the other needs a real git checkout. Whitespace is tolerated in both directions, and the exact value is compared both ways: "false" is a truthy string, so coercing on truthiness would refuse on every healthy clone instead. The `--json` error code becomes `history-not-verified`, which is what the condition actually establishes. One of my own tests asserted the wrong behaviour — that empty or unexpected output should permit the sweep. It is replaced, and the reason is recorded in the test so it is not reintroduced. Verified: 30 cases in tests/repo-hygiene.test.ts pass; typecheck, eslint and prettier clean; the sweep still runs normally on this full clone (exit 0). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YRCXgX4AWZ579bKN6sk6b
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
… not the CLI The guard sat in the preflight CLI, but the CLI is not the only caller. buildReconciliationEvidencePack calls collectReconciliationState directly and stamps the result `status: "complete"`. Proven in a real --depth 1 clone: the guarded CLI exited 1 while `reconciliation-evidence-pack --output` exited 0 and wrote a completed pack around merge-base-derived ahead/behind values that a grafted root makes fiction. collectReconciliationState now throws UnverifiedHistoryError (code: "history-not-verified") before resolving the base ref, so every caller — CLI, evidence pack, and any future one — fails closed by default rather than by remembering to ask. The CLI catches it only to keep its --json envelope. The check runs against the passed root, so an injected fixture root is judged on its own history. Regression cases live with each entry point and build a real --depth 1 clone (--depth is ignored for local-path clones, hence the file:// origin), asserting is-shallow-repository is "true" first so a git behaviour change cannot make them pass vacuously. Same shallow clone, before -> after: exit 0 with a `status: "complete"` pack written -> exit 1, refusal with the --unshallow remedy, no pack written. Ledger #109. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YRCXgX4AWZ579bKN6sk6b
BigSimmo added a commit
that referenced
this pull request
Jul 30, 2026
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
Resolves the docs/outstanding-issues.md conflict: main re-padded the whole table and extended #109, so the conflict spanned all 58 rows while the only semantic difference was this branch closing #115. Took main's block and kept this branch's #115 row. The conflict is why no CI ran here: GitHub cannot build refs/pull/1400/merge for an unmergeable PR, so every pull_request-triggered workflow (CI, Gitleaks, Semgrep) was skipped while pull_request_target ones still ran, and CircleCI failed 3 seconds after each push. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
main's #1418 (ledger merge dedupe + L4 quarterly archive rotation) and #1413 both edited docs/outstanding-issues.md, so this was a real content conflict rather than staleness: git merge-tree --write-tree confirmed CONFLICT before any resolution was attempted. Resolved by taking main's version of the ledger wholesale and re-applying this branch's five-row archive move on top, so neither side's work is lost: - from main: #88 and #97 archived, new open row #126 (quarterly ledger rotation) with queue order 35, the #23 "When" update (release-browser-matrix no longer blocked by pr-required), the #86 detail update, and the issues:next-id bump to 127. - from this branch: #95, #96, #104, #109 and #115 moved from Open items to Resolved / archive. No row from either side was dropped, and no id appears in both tables. Verified: 121 rows (52 open, 69 archived), marker next-id=127 above the highest; each of #88, #97, #95, #96, #104, #109, #115 resolves to exactly one archive row and #126 to one open row; zero conflict markers remain. npm run verify:cheap -> EXIT=0; "Gate-manifest OK: all 29 verify:cheap gates are enforced in CI"; "Test Files 431 passed (431)"; "Tests 4496 passed | 4 skipped (4500)". npx prettier --check . -> "All matched files use Prettier code style!" Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011YdPS2KhKqz2buzsUgmX3c
BigSimmo added a commit
that referenced
this pull request
Jul 30, 2026
* issues: archive five completed rows, delete none Triage pass over all 58 open items found five rows whose work is finished and whose next action is empty. Each moves from Open items to Resolved / archive with its fix evidence and the 2026-07-30 date: - #95 the pr-required aggregate now routes a cancelled result through a shared cancelled_error helper; guarded by seven cases that execute the extracted script, three mutation-proven. The red is deliberately retained, since GitHub counts a skipped required check as passing. - #96 every PR #1316 sub-item is dispositioned: the adoption-gate root-path gap closed on PR #1394, four findings were fixed independently, and the Therapy Compass retry-waiter finding was corrected to not-a-live-defect. - #104 a correction row with no next action - the worker's triple image read is an accepted peak-memory trade-off documented at worker/main.ts:866-869, not debt. Archived so a fourth audit does not re-file it. - #109 the branch sweep refuses on a shallow clone, an indeterminate result is its own failure, and the guard moved into the exported collector so the evidence-pack path fails closed too. - #115 the band adoption gate was redesigned to walk a real reachability graph rather than asking whether a file mentions the band. Nothing is deleted. The ledger contract is archive-only (SKILL.md:44 "Archive, never delete"; this file's own conventions: "rows are archived, not deleted, so the history stays auditable"), so no row qualifies for deletion. Row total is unchanged at 120: 58 -> 53 open, 62 -> 67 archived. Prettier widened the archive Outcome column to fit the new evidence, which repads the other archive rows; git diff --ignore-all-space is 7 insertions / 7 deletions, i.e. the five moved rows plus both separators. Verified: node scripts/check-outstanding-issues.mjs --self-test && node scripts/check-outstanding-issues.mjs -> "Outstanding-issues guard passed: 120 rows (53 open, 67 archived), unique ids, next-id=126 above the highest". npx prettier --check . -> "All matched files use Prettier code style!" Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011YdPS2KhKqz2buzsUgmX3c * docs: record PR 1428 review * docs: align issue 109 resolution date --------- Co-authored-by: Claude <noreply@anthropic.com>
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
My push of dd3a1b6 reported "[new branch]", which was the warning sign. The remote ref was absent at push time, so git created it instead of rejecting a non-fast-forward, and the branch tip moved from 1831cd2 back to a commit built on a14d9e7 — silently dropping three commits authored from a codex/review-pr1428 session between 09:57 and 10:07: 8d2710f Merge remote-tracking branch 'origin/main' into codex/review-pr1428 6157021 docs: record PR 1428 review 1831cd2 docs: align issue 109 resolution date They were recoverable: the objects still fetch by SHA and refs/pull/1428/head still pointed at 1831cd2, so nothing was lost permanently. Saved as recovered/pr1428-head and merged back here. Both of their real changes are kept: - 6157021 appends one record to docs/branch-review-ledger.md, which is append-only — losing it would have meant the PR #1428 review was never recorded and would be run again. - 1831cd2 corrects the Resolved date on the #109 archive row I wrote, from 2026-07-30 to 2026-07-29, aligning the column with the "RESOLVED 2026-07-29" in its own Outcome text. Their version is kept; mine was inconsistent. Resolved by taking their ledger wholesale — it already contained a14d9e7, so it carries every earlier commit of mine — and re-applying only the #122 move on top. Proven against their head rather than assumed: their head : 61 open + 71 archived = 132 merged : 60 open + 72 archived = 132 IDs lost from their head: NONE IDs invented: NONE duplicates: 0 #122 archived: True the five earlier archives still archived: True their #109 date fix kept: True The #122 replay script now refuses when #122 is already archived, rather than appending a second row, so a later merge cannot duplicate it. Verified: npm run verify:cheap -> EXIT=0; "Test Files 434 passed (434)"; "Tests 4563 passed | 4 skipped (4567)"; "Outstanding-issues guard passed: 132 rows (60 open, 72 archived), unique ids, next-id=135 above the highest, union merge active". npm run check:branch-review-ledger -> "113 live table records + 1206 archived ... no conflict markers, mojibake, heading records, or duplicates". npx prettier --check . -> "All matched files use Prettier code style!" Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011YdPS2KhKqz2buzsUgmX3c
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.
Draft handoff for the codex/RAG_FIX worktree. This preserves the current RAG/search validation, retrieval selection, answer rendering policy, documentation, and test updates for review. Verification was not run in this cleanup pass.