[WIP] Optimize search and ingestion performance - #59
Merged
Conversation
Copilot stopped work on behalf of BigSimmo due to an error
June 23, 2026 05:06
BigSimmo
marked this pull request as ready for review
June 23, 2026 05:54
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:f1cddc78a4
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…tests - DocumentViewer: remove || !isConfigured from serverDemoMode init so demo mode only activates via NEXT_PUBLIC_DEMO_MODE=true, not when Supabase is unconfigured (fixes 'document viewer private missing source state is coherent') - ClinicalDashboard: add storedSessionExists check so test-seeded localStorage auth token enables canUsePrivateApis when env is ready (fixes 'duplicate upload warning and exact-copy batch count are visible') Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # src/lib/search-scope.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:73a156f63a
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo
enabled auto-merge
June 26, 2026 12:21
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo pushed a commit
that referenced
this pull request
Jul 21, 2026
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9
BigSimmo pushed a commit
that referenced
this pull request
Jul 21, 2026
…ers (E-3c PR-C) Dose and monitoring answers must carry the asked-for figure/schedule when a cited chunk verbatim supports it (run-#58 baseline: targeting 1/5 on both intents, every miss = missing figure). Lead-slot promotion swaps/appends the first later figure-bearing fact ONLY when no lead carries one, guarded by the claim-support atom corpus (sourceEvidenceText, now exported) so a promoted figure can never convert a grounded answer into an evidence gap - the guard proven load-bearing by red-testing it disabled. The dose/threshold generation fallback now prefers the safe candidate that carries the intent figure over the first safe one; the safety gate itself is unchanged. Fallback helpers extracted to rag-extractive-answer (cycle-check verified); rag.ts 4908->4901. Six discriminating tests, each verified failing on prior code. Full suite 3068 passed / 1 known container artifact. RAG impact: behaviour change (extractive answer composition + fallback candidate preference only) - E-4 pair vs banked #57/#58/#59 baselines post-merge; retrieval, ranking, selection ordering untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9
12 tasks
BigSimmo added a commit
that referenced
this pull request
Jul 21, 2026
…ers (E-3c PR-C) (#1039) * docs(ledger): E-3b + PR-B live pair verdict (run #59 green vs #57/#58) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9 * feat(rag): figure-aware extractive selection for dose/monitoring answers (E-3c PR-C) Dose and monitoring answers must carry the asked-for figure/schedule when a cited chunk verbatim supports it (run-#58 baseline: targeting 1/5 on both intents, every miss = missing figure). Lead-slot promotion swaps/appends the first later figure-bearing fact ONLY when no lead carries one, guarded by the claim-support atom corpus (sourceEvidenceText, now exported) so a promoted figure can never convert a grounded answer into an evidence gap - the guard proven load-bearing by red-testing it disabled. The dose/threshold generation fallback now prefers the safe candidate that carries the intent figure over the first safe one; the safety gate itself is unchanged. Fallback helpers extracted to rag-extractive-answer (cycle-check verified); rag.ts 4908->4901. Six discriminating tests, each verified failing on prior code. Full suite 3068 passed / 1 known container artifact. RAG impact: behaviour change (extractive answer composition + fallback candidate preference only) - E-4 pair vs banked #57/#58/#59 baselines post-merge; retrieval, ranking, selection ordering untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9 * fix(rag): verbatim-corpus check for zero-atom figure promotion (reviewer P2) Both reviewers converged on the gap: bare-number intervals like 'every 6 weeks' match the monitoring figure pattern yet yield no clinical value atom, so the atom-only guard passed them trivially - a figure sourced only from adjacent context could then be promoted and nuked by claim support (safe direction, but converts a shippable answer into an evidence gap). Zero-atom figures now require the matched figure substring verbatim in the citing chunk's claim-support corpus (intentFigureMatchText). Proven both directions by three new tests; comment-precision nit on the corpus lockstep folded in. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9 * fix(rag): full-interval figure matching + genuine zero-atom guard coverage CodeRabbit review on the P2 guard: reorder monitoringIntervalFigurePattern so the unit-bearing "every N unit" alternative precedes bare "every N" — exec() now returns the full schedule instead of a truncated "every 6". Digit+unit intervals turn out to produce quantity atoms (6/week), so atom identity already refuses unit mismatches; the reorder hardens the verbatim fallback against atom-coverage drift, and both comments claiming "every 6 weeks" is zero-atom are corrected. The non-discriminating first guard test is replaced with a mismatched-unit pin (shared number, different unit, figure only in adjacent context) plus two genuinely zero-atom tests ("annually") that exercise the verbatim-corpus path both directions. promotionAtomKey hand-mirror replaced by exporting clinicalValueAtomKey from rag-claim-support. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9 --------- Co-authored-by: Claude <noreply@anthropic.com>
BigSimmo pushed a commit
that referenced
this pull request
Jul 21, 2026
…59) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9
This was referenced Jul 24, 2026
10 tasks
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
`PR mergeability` flagged this branch, but `git merge-tree` returned a clean tree — behind-but-clean staleness, not a content conflict. The merge itself then reported success while producing every open-items row twice (`#59 appears 2 times (lines 101, 166)` and so on for the whole table): `.gitattributes` sets `merge=union` on this file, which is git's built-in concatenate-both-sides driver with no dedupe, and the table is not append-only. Rebuilt from `origin/main` (now through #134) with only the one row this branch actually changed re-applied. Recorded as #135, since the driver turns a resolvable conflict into a guaranteed guard failure and makes `merge-tree` look clean. Also corrects #127's own framing. `Production UI` PASSED on run 30530393684, so the failure is intermittent at 2 of 3 completed runs, not reproducible as the previous row claimed — that was premature on two datapoints. The `data-scroll-signal` diagnostic therefore has not yet had a failure to report; it is still the thing that will name the cause when one comes. Verified: check:outstanding-issues 133 rows / 67 open / unique ids / next-id=136; check:branch-review-ledger 112 live + 1206 archived; whole-tree prettier clean; no conflict markers under docs/, tests/ or src/. Not re-run for this merge: verify:cheap and the phone-scroll spec — the code changes are unchanged from 5495f28, where both passed (434 test files / 4562 tests, and 56 passed), and this commit touches only the ledger plus main's own already-verified tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
BigSimmo added a commit
that referenced
this pull request
Jul 30, 2026
…1430) * test(phone-chrome): name which value holds the header open, not just that it did `data-scroll-hidden` on the collapse wrapper is `scrollHidden && !sharedChromePinned`, so a missing attribute has two very different causes the assertion cannot separate: the scroll state machine never fired, or it fired and a pin held the chrome open. The bare assertion reads as the first even when it is the second — which is how a stuck pin was misread as a flaky scroll gesture across two CI runs on 2026-07-30. `expectChromeHidden` keeps the same pass condition and adds a failure message. The discriminator is already in the DOM: DocumentViewer's page-owned composer hides on `composerScrollHidden`, which consults `scrollHidden` and not the pin, so composer-hidden plus header-visible proves the pin. Every term of `sharedChromePinned` also has a DOM tell — an `aria-expanded` trigger, a popover, or focus inside the portaled addon host — so when all read false the pin is a stale latch rather than a live surface, and the message says so. Also updates ledger #127 with what the traces establish: `scrollHidden` is TRUE and `sharedChromePinned` is stuck, reproducible on both completed full-suite runs and both variants, always at the reduced-motion hide that follows the section-sheet round-trip and never at the first hide. `main` only looks green because `Production UI` is skipped on its docs-only pushes; it has not run this test since 90b3e34, with zero `src/` changes since. Deliberately not the fix. Which term latched is proven; the mechanism is inferred, and it does not reproduce locally — every local run used the container's Chromium 1194 rather than the bundled 1234 CI installs (#121), so no local green is evidence here. This makes the next CI failure name its own cause instead of costing another trace download. Verified: typecheck clean, lint clean, prettier clean, check:outstanding-issues 125 rows / unique ids / next-id=128, and both affected tests still pass locally (2 passed, 12.6s). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ * fix(test): publish the header's own scroll signal; withdraw the pin claim Codex review is correct and this retracts the previous commit's central claim. Reading `form.document-viewer-composer[data-scroll-hidden]` as a proxy for the header's `scrollHidden` was wrong: they are separate state machines. The header is driven by the shell's `chromeScrollHide` (global-search-shell.tsx:332), fed only by `useDocumentScrollHideReporter` (line 345) and passed in at line 876, while DocumentViewer runs its own two `useHideOnScroll` instances (use-document-viewer-chrome-scroll.ts:20-30). Composer-hidden therefore proves DocumentViewer's reporter fired and says nothing about the header's, so it never separated a pin from a reporter-never-fired — the exact distinction the helper claimed to make. `data-scroll-signal` on the collapse wrapper now publishes the header's raw `scrollHidden` before the pin is applied, and `expectChromeHidden` reports it alongside DocumentViewer's so a divergence between the two feeds is visible instead of collapsed into one verdict. Nothing styles the attribute; no CSS or code reads it (verified by grep), so behaviour is unchanged. Ledger #127 is corrected rather than patched over: the "traces prove sharedChromePinned is stuck" claim is explicitly withdrawn, what the traces do establish is separated from what they do not, and the new leading hypothesis is recorded as untested — the shell's feed is document-only, so where `#main-content` owns scrolling `window.scrollY` never moves and the shell reporter cannot see the gesture, which would explain the standalone-PWA variant directly. It also now says not to infer the header's scroll state from any page-owned composer. Verified: verify:cheap exit 0 — Test Files 434 passed (434), Tests 4562 passed | 4 skipped (4566); typecheck and lint clean; prettier clean; the full phone-scroll spec 56 passed (4.4m) against an isolated production build. That build used the container's Chromium 1194, not the bundled 1234 CI installs (#121), so it proves the attribute broke nothing and nothing more. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ * Merge origin/main; rebuild the ledger the union driver doubled `PR mergeability` flagged this branch, but `git merge-tree` returned a clean tree — behind-but-clean staleness, not a content conflict. The merge itself then reported success while producing every open-items row twice (`#59 appears 2 times (lines 101, 166)` and so on for the whole table): `.gitattributes` sets `merge=union` on this file, which is git's built-in concatenate-both-sides driver with no dedupe, and the table is not append-only. Rebuilt from `origin/main` (now through #134) with only the one row this branch actually changed re-applied. Recorded as #135, since the driver turns a resolvable conflict into a guaranteed guard failure and makes `merge-tree` look clean. Also corrects #127's own framing. `Production UI` PASSED on run 30530393684, so the failure is intermittent at 2 of 3 completed runs, not reproducible as the previous row claimed — that was premature on two datapoints. The `data-scroll-signal` diagnostic therefore has not yet had a failure to report; it is still the thing that will name the cause when one comes. Verified: check:outstanding-issues 133 rows / 67 open / unique ids / next-id=136; check:branch-review-ledger 112 live + 1206 archived; whole-tree prettier clean; no conflict markers under docs/, tests/ or src/. Not re-run for this merge: verify:cheap and the phone-scroll spec — the code changes are unchanged from 5495f28, where both passed (434 test files / 4562 tests, and 56 passed), and this commit touches only the ledger plus main's own already-verified tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ * Merge origin/main (fcd9041); rebuild the ledger rather than trust union Third staleness resolution on this branch in about forty minutes. `PR mergeability` again reported `mergeable_state: dirty` and "a real merge conflict", but `git merge-tree --write-tree` returned a clean tree, so this is behind-but-clean staleness, not a content conflict. The single overlapping path between this branch and main is `docs/outstanding-issues.md` — nothing else on this branch is contested, which is why the code files are byte-identical to 5495f28. The file is rebuilt from `origin/main` with this branch's two rows re-applied (#127 corrected, #135 added after #134, marker 136) instead of keeping the union driver's output, which concatenates both sides of every overlapping hunk without dedupe and doubled the whole table last time — that behaviour is what #135 records. Main's #127 still carried the withdrawn "sharedChromePinned is stuck" text and #135 was unclaimed, so neither graft overwrote anyone else's edit. Verified: check:outstanding-issues 133 rows / unique ids / next-id=136; check:branch-review-ledger 113 live + 1206 archived; whole-tree prettier clean. Not re-run: verify:cheap and the phone-scroll spec — `git diff 5495f28 -- src/ tests/` is empty, so the code carries that commit's evidence (434 test files / 4562 tests, and 56 passed) unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ * docs: record PR 1430 review --------- Co-authored-by: Claude <noreply@anthropic.com>
BigSimmo added a commit
that referenced
this pull request
Jul 30, 2026
* fix(ledger): remove merge=union from the issues ledger, per its own #133 Ledger #133 already recorded union as the wrong driver for this file — "two sides each bumping the marker produce two `next-id` lines, corrupting the file silently where a conflict would fail loudly" — but `.gitattributes` still set it and `check-outstanding-issues.mjs` *required* it, so the repo's own tested conclusion was contradicted by its own config. PR #1430 confirmed the cost at scale: four merges in one session, each reporting success while duplicating the entire open-items table (`#59 appears 2 times (lines 101, 166)` and so on for every row), each needing a manual rebuild from origin/main. Union also makes `git merge-tree` report a clean tree, so the pre-merge conflict check cannot warn. Unlike docs/branch-review-ledger.md — which keeps its custom `merge=ledger` driver, union plus exact-row dedupe — this file allocates IDs by read-modify-write. Concurrent appends therefore need manual renumbering whatever the driver does (hit twice on 2026-07-30: #125 and #135 collisions), so union bought nothing and only hid the overlap. Default 3-way merge conflicts honestly instead. The gate's attribute check is inverted rather than deleted, so a driver reappearing here is a red gate. AGENTS.md, docs/process-hardening.md, .claude/skills/issues/SKILL.md and docs/scripts-index.md are updated to match, and #133's driver half is marked resolved with its still-open half (fixed-width padding making every row edit one hunk) left intact. Verified: reintroducing `docs/outstanding-issues.md merge=union` fails the gate with "must have NO merge driver (found merge=union)", and removing it passes with "no merge driver" — the gate bites, not just passes. verify:cheap exit 0: Test Files 434 passed (434), Tests 4563 passed | 4 skipped (4567). Whole-tree prettier clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ * fix(gate): reject `-merge` too, not just a named driver Codex review is right: the new check accepted `unset` alongside `unspecified`, and those are not the same state. Per gitattributes, an Unspecified `merge` attribute is the documented default 3-way text merge — the contract this PR establishes — while Unset (`-merge`) takes the current branch's version and declares the merge conflicted, so every two-sided edit becomes a manual resolution. A global or future attributes file could therefore have violated the contract with the gate still printing "no merge driver". Reproduced before fixing: appending `docs/outstanding-issues.md -merge` made `git check-attr` report `merge: unset` and the guard passed. It now fails with a message naming the Unset/Unspecified distinction and telling the reader to drop the negated attribute rather than add one. The acceptance decision moves into an exported `mergeAttributeProblem` so the distinction is unit-tested rather than only reasoned about, with four cases in tests/repo-hygiene.test.ts: `unspecified` accepted; `unset`, `union`/`ledger`, and an empty reading all rejected. The empty case matters because an unparsed check-attr output would otherwise make the whole check vacuous. Verified: with `-merge` present the gate fails on the new message; with it removed it passes "no merge driver". repo-hygiene 47 passed (47). verify:cheap exit 0 — Test Files 435 passed (435), Tests 4508 passed | 4 skipped (4512). Whole-tree prettier clean. (Test total differs from this branch's earlier run because it now carries main's #1423/#1427/#1438; this commit adds four.) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ --------- Co-authored-by: Claude <noreply@anthropic.com>
7 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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
uploadAndCaptionImagesto classify/caption images in parallel batches instead of sequentially, reducing latency for documents with many imagesPromise.all