Skip to content

fix(text): follow-up — normalize indexed-page viewer text, flat-memory backfill dry-run - #127

Merged
BigSimmo merged 11 commits into
mainfrom
claude/clever-heisenberg-9a53a9
Jul 2, 2026
Merged

fix(text): follow-up — normalize indexed-page viewer text, flat-memory backfill dry-run#127
BigSimmo merged 11 commits into
mainfrom
claude/clever-heisenberg-9a53a9

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Summary

  • Post-merge follow-ups from the Copilot review on fix(text): repair extraction glyph artifacts app-wide and route all document-derived text through formatters #121:
    • sourceTextForIndexedPage now routes through normalizeExtractedGlyphs so the Document Viewer's indexed source text panel can no longer show ligatures/soft hyphens/control chars. The primitive preserves spaces/tabs/newlines, so the fixed-width table spacing this path depends on is untouched (existing test still passes).
    • The backfill script's dry-run keeps only a counter and capped samples in memory; the full changed-row list is materialized only in write mode where the mandatory backup needs it.

Verification

  • typecheck, eslint (touched files), prettier, full unit suite (802 tests) — pass

Clinical Governance Preflight

  • Display formatting + ops-script memory only; no grounding, access, metadata, or Supabase behavior changes. Target remains Clinical KB Database (sjrfecxgysukkwxsowpy).

🤖 Generated with Claude Code

BigSimmo and others added 7 commits July 2, 2026 11:29
…ocument-derived text through formatters

- Add normalizeExtractedGlyphs (ligatures, soft hyphens, zero-width/control
  chars; deliberately no line-break de-hyphenation — fusing would corrupt
  clinical compounds like low-dose) wired into the base compactWhitespace/
  readableWhitespace cleaners so every formatter inherits it
- Fix bypass surfaces: verbatim quote body + clipboard, citation labels
  (formatCitationLabel/formatCompactCitationLabel now glyph-clean titles),
  source titles, visual-evidence labels, table snippets, image captions/alt,
  evidence-map details, AccessibleTable cells/captions
- Drop unknown-filler segments from provenance joins while always keeping
  clinical governance warnings (source-metadata.ts, ui-primitives.tsx)
- Normalize glyphs at ingestion (buildChunks) with source-span offsets kept
  consistent via a single normalized page text
- Add scripts/backfill-text-normalization.ts: in-place, no-re-embed, dry-run
  by default, revertible JSON backup, project-guarded
- Centralize static UI copy in src/lib/ui-copy.ts (empty states, errors,
  starter prompts) for the dashboard and document manager
- Add tests/rendered-text-formatting.test.ts guard against new raw
  interpolations; document conventions in docs/process-hardening.md

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…erg-9a53a9

# Conflicts:
#	src/components/ClinicalDashboard.tsx
#	src/components/clinical-dashboard/answer-status.tsx
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…arify verbatim-quote whitespace contract

Addresses Copilot review on PR #121: '--limit foo' / '--limit 0' previously
fell through the falsy truthiness check and silently meant unlimited.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…omitted-image markers, boilerplate keys, backfill hardening

- Convert VT/FF/NEL controls to newlines instead of deleting (deleting fused
  words like dose\fmonitoring)
- Strip [[IMAGE_DATA_OMITTED]] markers from verbatim quotes and all display paths
- Build repeated-boilerplate keys from the same normalized page text that
  removePageNoise compares against
- Backfill: make the revertible backup mandatory (no --no-backup escape hatch)
  and include retrieval_synopsis in scan/patch/backup

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dry-run memory flat

Post-merge Copilot follow-ups on PR #121:
- sourceTextForIndexedPage now starts from normalizeExtractedGlyphs (spaces/
  tabs preserved, so fixed-width table parsing is unaffected) instead of only
  normalizing CRs — the indexed source text panel no longer leaks ligatures
- Backfill dry-run tracks a counter + capped samples only; the full changed-row
  list (needed for the mandatory backup) is materialized only when writing

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 2, 2026 04:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR is a follow-up to the text-sanitization work from #121, ensuring additional UI and ingestion surfaces consistently normalize PDF-extraction glyph artifacts, while also hardening ops tooling (a backfill script) to be safe in dry-run mode.

Changes:

  • Add and test a shared normalizeExtractedGlyphs primitive and route additional sanitizer paths through it (including the indexed-page viewer).
  • Centralize more UI chrome copy in src/lib/ui-copy.ts and replace inline strings in key dashboard panels.
  • Add a dry-run-by-default backfill script to normalize already-stored document_chunks text (with mandatory backup on write), plus regression tests to prevent raw rendering regressions.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/source-text-sanitizer.test.ts Adds focused unit coverage for normalizeExtractedGlyphs and sourceTextForVerbatimQuote.
tests/source-metadata.test.ts Extends provenance-summary tests for dropping unknown filler while keeping governance warnings.
tests/rendered-text-formatting.test.ts Adds a static guard test to prevent raw JSX interpolation of document-derived fields on known surfaces.
tests/citations.test.ts Adds label tests ensuring citation titles are glyph-normalized and synthetic prefixes are removed.
src/lib/ui-copy.ts Introduces a central module for static, user-facing UI copy.
src/lib/source-text-sanitizer.ts Implements normalizeExtractedGlyphs, expands image-marker stripping, and adds sourceTextForVerbatimQuote.
src/lib/source-metadata.ts Drops unknown filler segments from the visible provenance summary while keeping governance warnings.
src/lib/citations.ts Normalizes citation titles centrally before building full/compact labels.
src/lib/chunking.ts Normalizes extracted glyphs during chunking to keep boilerplate keys, spans, and chunk text consistent.
src/components/ui-primitives.tsx Updates provenance rendering to drop unknown filler segments while preserving governance signals.
src/components/DocumentViewer.tsx Normalizes table headings/captions and removes raw marker leakage from viewer surfaces.
src/components/DocumentManagementActions.tsx Clarifies an intentional raw-title rendering for destructive-action confirmation UX.
src/components/ClinicalDashboard.tsx Routes additional document-derived surfaces (titles, quotes, snippets) through sanitizers and centralizes empty/error copy usage.
src/components/clinical-dashboard/master-search-header.tsx Uses cleanDisplayTitle consistently for document scope titles/tooltips.
src/components/clinical-dashboard/DocumentManagerPanel.tsx Centralizes upload/indexing empty/error copy and cleans document titles for display.
src/components/clinical-dashboard/display-text.ts Adds cleanDisplayTitle and ensures title cleaning includes glyph normalization.
src/components/clinical-dashboard/answer-status.tsx Replaces inline UI strings with centralized copy constants.
src/components/AccessibleTable.tsx Normalizes extracted glyph artifacts in accessible table text cleaning.
scripts/backfill-text-normalization.ts Adds a safe backfill script (dry-run by default; --write --confirm; mandatory JSON backup) to normalize stored chunk text.
package.json Adds an npm script entry to run the new backfill script.
docs/process-hardening.md Documents text-formatting and UI-copy conventions and the purpose/safety model of the backfill script.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/components/DocumentViewer.tsx
Comment thread src/components/DocumentViewer.tsx
@BigSimmo

BigSimmo commented Jul 2, 2026

Copy link
Copy Markdown
Owner Author

@copilot resolve the merge conflicts in this pull request

auto-merge was automatically disabled July 2, 2026 04:41

Head branch was pushed to by a user without write access

Copilot AI review requested due to automatic review settings July 2, 2026 04:42
@BigSimmo
BigSimmo removed the request for review from Copilot July 2, 2026 04:42

Copilot AI commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Merge conflicts resolved. The merge keeps the memory-efficient dry-run implementation for the backfill script (using a counter instead of materializing the full changed-row list in dry-run mode) and the improved accessibility changes for document images (tableHeading fallback for alt text and pre-cleaned tableTextSnippet for context).

@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

detail:
sourceTextForCompactDisplay(row.quote || row.source.snippet || row.source.reason || "") ||
cleanDisplayTitle(row.source.title),

P2 Badge Sanitize the evidence-map passage actually rendered

When an evidence row's quote or snippet contains an image-data marker or PDF glyph artifact, this only cleans detail, but EvidenceMapTable renders the “Passage sample” from row.bestLinkedPassage, which is still assigned the raw quote/snippet in this same row object. The new formatter therefore does not protect this visible evidence-map surface; sanitize bestLinkedPassage as well or have the table use the cleaned detail.

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

@BigSimmo
BigSimmo merged commit b4422c2 into main Jul 2, 2026
3 checks passed
@BigSimmo
BigSimmo deleted the claude/clever-heisenberg-9a53a9 branch July 2, 2026 08:12
BigSimmo pushed a commit that referenced this pull request Jul 30, 2026
docs/outstanding-issues.md conflicted again — main advanced 5 commits and took
#125 for itself. Resolved by taking main's file wholesale and re-appending my
three rows renumbered to #126/#127/#128, marker to 129, since those rows are
this branch's only change to the file.

This is #127 reproducing within minutes of being filed: the conflict was
entirely mechanical, and it had already silently stopped every
pull_request-triggered check on PR #1424 (3 check runs instead of 16) exactly
as #116 describes.

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
PR #1418 landed its own `#126` (quarterly ledger rotation) on main while
this branch was carrying one — the same unprotected read-modify-write
race archived as #112, hit for the second time in an hour. Main holds
first claim, so its row keeps #126, the phone-scroll CI capture moves to
#127, and the marker advances to 128. Both rows are kept.

check:outstanding-issues: 125 rows (62 open, 63 archived), unique ids,
next-id=128 above the highest.
check:branch-review-ledger: 90 live + 1206 archived, ledger merge active.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
BigSimmo pushed a commit that referenced this pull request Jul 30, 2026
Fourth conflict in this file today. main advanced 30 commits and took #126 and
#127 for itself, so my rows renumber again to #128/#129/#130, marker 131.
Resolution is mechanical as always: main's file plus this branch's three
appended rows.

That is now four conflicts and four rounds of ID collision in one session,
which is the evidence #129 (the padding recommendation) rests on.

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
GitHub reported this PR dirty, but `git merge-tree --write-tree origin/main HEAD`
exited 0: clean. That is the anti-churn case, not a content fight — GitHub's
server-side merge does not read custom merge drivers, which is already recorded
as ledger #127. So the documented remedy applied: merge locally and push.

`git merge origin/main` then produced a CORRUPT tree and it must not be pushed
as-is. `.gitattributes` sets `merge=union` for this file (added by #1418), and
union keeps both sides of every differing line. Prettier pads these tables to
the widest cell, so a single row's edit re-pads every other row — both sides'
copies of all 128 rows are textually distinct, and union kept both. Result:
every row duplicated, both table headers duplicated, 133 insertions.

`npm run check:outstanding-issues` caught it, failing with 128 duplicate-id
errors, both separator rows reported as non-canonical ids, and all five archived
rows "in BOTH the open and archive tables". The gate #112 added is what stopped
a silently doubled ledger from reaching the PR — it failed closed exactly as
designed.

This is the outcome ledger #127 predicted: it records that `merge=union` is the
wrong fix for this file, with evidence, "so nobody re-proposes it". #1418
enabled it anyway. Union is safe for append-only single-line records like
docs/branch-review-ledger.md; it is actively destructive on a width-padded
table, because padding turns every unrelated row into a conflicting line.

Recovered without a destructive reset: the merge commit and both parents are
kept, only its tree was rebuilt — take origin/main's ledger, re-apply the
five-row archive move, reformat. Proven lossless against origin/main rather
than assumed:

  origin/main : 63 open + 65 archived = 128, 0 duplicates
  rebuilt     : 58 open + 70 archived = 128, 0 duplicates
  IDs lost from main: NONE      IDs invented: NONE
  open == main's open minus the five moved: True

Verified: npm run verify:cheap -> EXIT=0; "Test Files 434 passed (434)";
"Tests 4563 passed | 4 skipped (4567)"; "Outstanding-issues guard passed: 128
rows (58 open, 70 archived), unique ids, next-id=131 above the highest, union
merge active". 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 pushed a commit that referenced this pull request Jul 30, 2026
…laim

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
BigSimmo pushed a commit that referenced this pull request Jul 30, 2026
Main's side carried #128-#130, archived #116 and reordered #117-#119, so it
is taken whole; only the #127 row is grafted from this branch, since main
still holds the withdrawn "sharedChromePinned is stuck" text. Marker stays
at main's 131. The file now reports `union merge active`, so this class of
collision should stop.

Also cross-links #130 into #127. PR #1396 ("overlay the phone header so
hiding it never moves content") merged at 06:49:55, about twenty minutes
before the first failure, and rewrote the exact overlay path this defect
sits in — phoneOverlayMotion, the overlay stack transition, and
readChromeCollapseMetrics charging zero released top geometry. Its merge
commit 90b3e34 is the single main run that passed, which fits the
passed-once-then-failed-twice-escalating shape better than the focus-latch
reading. #130 also records that #1396's own declared physical-device
prerequisite was never run, and that headless Chromium cannot certify what
it covers.

Verified after the merge: check:outstanding-issues 128 rows / unique ids /
next-id=131 / union merge active; check:branch-review-ledger 97 live + 1206
archived; prettier clean on all three changed files. Not re-run after the
merge: verify:cheap and the phone-scroll spec, which both passed on this
branch's own commit before it (434 test files / 4562 tests, and 56 passed).
The merge adds only main's already-verified tree plus one docs cell, so the
earlier evidence still covers this diff — but it is the earlier commit's
evidence, not this one's.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
BigSimmo pushed a commit that referenced this pull request Jul 30, 2026
The `merge=union` driver on `docs/outstanding-issues.md` preserves concurrent
appends, but when both sides restructure the same region it concatenates them
wholesale. Merging the latest main did exactly that: every open row appeared
twice and both `issues:next-id` markers survived — 66 duplicate-id errors from
`check:outstanding-issues`, which is precisely the failure that gate exists to
catch (#112).

Resolved by rebuilding on main's canonical file rather than by hand-editing the
duplicated table: reset to `origin/main`, then re-apply this branch's five
captured rows at #131-#135 (main had advanced its allocation to #130 while this
branch was open, so the earlier #128-#132 numbering collided again) and
re-apply the #127 narrowing note. Marker bumped to 136.

Union merge cannot allocate unique ids; only the structural gate can catch when
it has produced an invalid file. It did.
BigSimmo added a commit that referenced this pull request Jul 30, 2026
…indings (#1424)

* docs(ledger): record PR #1400 closeout and capture three unrecorded findings

Documentation only — two ledger files, no code.

**Review closeout for PR #1400** appended with `ledger:append` (never
hand-written), recording the 17 findings fixed, the verification behind each,
and the post-merge check that all 8 commits are ancestors of main with the 4
changed files byte-identical.

**Three findings from that session that nothing else records:**

- `#125` — `@codex fix` produced 11 commits across a branch named `work`,
  none fetchable, the same finding rewritten four times. It reads as success
  while the branch is unchanged, which is the actual hazard.
- `#126` — both client-side push guards are inert for agent pushes:
  `gh` absent makes the auto-merge sentinel fail open, and `core.hooksPath`
  is set only by a local install. They protect the environment least likely to
  need them.
- `#127` — this ledger's fixed-width padding makes one row's edit re-pad all
  59, so it conflicts on nearly every main advance; each conflict silently
  stopped all CI on #1400 via `#116`. Records that `merge=union` is the wrong
  fix, with the evidence.

CircleCI was deliberately not filed — already captured as `#122`. Checked
before writing rather than after.

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

* docs(issues): correct unsafe pull_request_target advice in #129

Review caught a real problem in the guidance I filed, not in code: #129's
next-action suggested moving *both* push guards server-side into a
`pull_request_target` job. That context carries secrets and a write token, and
a format check must execute PR-head code — including the dynamic
`prettier.config.*` this very PR taught the guard to load. That is the classic
privileged-context vector, and `.github/workflows/pr-policy.yml` already avoids
it deliberately by checking out only `github.workflow_sha`.

Corrected, and the row now records why the whole idea was unnecessary:
formatting is already enforced server-side by `Static PR checks` running
`format:check` on ordinary `pull_request` CI, so the guard's only unique value
is failing fast before the push. Only the metadata-only auto-merge sentinel
could safely live in a target job.

Bad advice in a durable ledger is worse than no advice — someone would have
acted on it.

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

* fix(issues): repair the duplicated table from the sixth main merge

Sixth conflict today, and the first that auto-merged *wrongly*: git's text
merge concatenated both tables, duplicating all 63 open rows. PR #1421 had
landed on main using #128/#129/#130 — the exact id collision #112 describes —
so both sides had those ids with different content and the merge kept both.

`npm run check:outstanding-issues` caught it and stated the correct resolution
verbatim: renumber the incoming rows above the marker and bump it, rather than
taking one side wholesale and dropping the other's rows. Done exactly that —
main's table is authoritative, this branch's four rows renumber to
#131/#132/#133/#134, marker to 135. Verified both sides' rows survive:
main's #128-#130 and mine are all present and distinct.

Worth noting main's new #129 (`update-branch` API does not honour the
`merge=ledger` driver) is the server-side twin of my #134 (the driver is absent
wherever `npm install` was skipped). Same root cause from two directions.

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

* docs: record PR 1424 review

---------

Co-authored-by: Claude <noreply@anthropic.com>
BigSimmo pushed a commit that referenced this pull request Jul 30, 2026
First real run of the sharded shape (CI 30530618838, all green, whole run
13m39 against a 16.8-18.6 min unsharded baseline):

  ui-critical-fast  15 tests   3m14
  Production UI (1) 121 tests  9m36
  Production UI (2) 111 tests  6m54
  Production UI (3) 110 tests  6m20

The prediction was wrong by ~40%. ~6.8 min was expected for the largest shard
from 121/342 tests x 13.5 min; 9m36 happened. Per-test cost is not uniform —
111 tests took 6m54 while 121 took 9m36 — so a count-balanced split understates
the slowest shard whenever the slow specs land in one group. `--shard` can only
balance by count; balancing by duration would mean splitting the slow spec files
themselves.

The win is real but smaller than claimed, and the workflow comment and
process-hardening now carry the measured numbers plus the reason the arithmetic
misleads, so the next person re-measures instead of re-deriving.

Also merges origin/main. The ledger conflict was GitHub-visible only: that file
carries merge=union locally, which GitHub does not honour (#129). Resolved by
keeping the one genuinely new record and dropping three that main already had
elsewhere in the file — append-only forbids dropping a record that exists once,
not keeping a second copy. Superseding record appended for this HEAD, since the
prior one asserted a root cause that #127's trace evidence refutes.
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 pushed a commit that referenced this pull request Jul 30, 2026
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
BigSimmo added a commit that referenced this pull request Jul 30, 2026
…t did (#1427)

* test(phone-scroll): prove the drag delivered before asserting the chrome hid

CI run 30518866604 failed `ui-phone-scroll.spec.ts:423` on

  expect(getByTestId('universal-header-collapse'))
    .toHaveAttribute('data-scroll-hidden', 'true')  // received ""

after the full 10s auto-retry, and the classifier recorded it as "needs
investigation". The assertion was right; the scroll never happened.

`dragScrollBy` moved the scroller with `scrollTop +=`, which clamps silently
at the end of the range, and returned nothing. When a page lays out shorter
than the test assumed — content still settling under full-suite CI load — a
720px request delivers a fraction of that, the chrome correctly stays visible
because document-detail chrome only hides past `scrollTop > 120`, and the
failure surfaces ten seconds later looking like a product regression. The
helper also resolved the scroll owner once up front, so a mid-drag layout
change left it pushing an element that had stopped scrolling.

- `dragScrollBy` now re-resolves the owner each step and returns the distance
  actually travelled.
- `dragScrollUntilHidden` waits for the remaining downward runway (a condition
  wait, not a settle sleep), drags, and fails naming the shortfall if the drag
  could not cross the threshold. Used at the four sites that assert a hide
  immediately after a fixed-distance drag.
- `addPhoneScrollRunway` waits for its 1600px filler to reach layout instead of
  sleeping 50ms. All 14 call sites already depend on that runway existing.

Every assertion is byte-identical: a genuinely stuck header still fails exactly
as before, once the drag is proven to have happened. No `.first()` was added
(#93's stop rule) and no tolerance was relaxed.

* ci: shard Production UI across three runners

Measured on 2026-07-30 from the Actions API, two full UI-scope PR runs
(30520443076, 30519912667): `Production UI` took 15m26-16m31 of a 16.8-18.6
minute run — 83-89% of wall clock — while every other job finished by minute 4
and then waited. Playwright itself reported `339 passed (13.5m)`; the balance is
the isolated production build.

That single job is also where the churn cost lands: 42% of PR runs in the
sampled window were cancelled (25 of 60 completed), almost all superseded
mid-Production-UI.

Sharding is across runners, not workers. `workers: 1`, `fullyParallel: false`
and `retries: 0` are unchanged inside each shard, so determinism is identical
and per-runner load falls — which matters because #93's duplicate page root is
load-dependent. `run-playwright.mjs` already forwards argv to `playwright test`,
so `--shard` needed no runner change.

The shard count is measured, not chosen. `fullyParallel: false` makes a spec
file indivisible, so shard sizes are lumpy and more shards is not monotonically
faster. Over the 340 required chromium tests:

  N=3 -> 121/106/113       largest 121
  N=4 -> 121/106/96/17     largest 121  (same critical path, one more runner)
  N=6 -> 65/56/106/5/91/17 largest 106
  N=5 -> 121/106/0/96/17   and N=8 -> two empty shards

N=4 buys nothing over N=3, and any N with an empty shard would go red because
`test:e2e:pr` deliberately omits `--pass-with-no-tests`. Expected critical path
~15.5 -> ~7 min, assuming per-test cost is roughly uniform.

`fail-fast: false` so a failing shard cannot cancel its siblings and re-create
the cancelled-vs-failed ambiguity #95 removed. Artifact names are shard-scoped
because upload-artifact runs with `overwrite: false`. Branch protection requires
only the `pr-required` aggregate, and `needs` on a matrix job yields the roll-up
of all shards, so the aggregate is unchanged.

Also adds `restore-keys` to both Playwright browser caches: without a prefix
fallback a lockfile bump forced a cold browser download in every UI job at once,
now three times over.

* ci: bound the codex auto-resolve jobs and serialise the visual config

Two inconsistencies found while mapping the pipeline, neither load-bearing but
both silent:

- `codex-autofix-review-comments.yml` was the only workflow in the repo with no
  `timeout-minutes` on either job, so both inherited GitHub's 360-minute default
  for work that reads PR metadata and posts one comment.
- `playwright.visual.config.ts` set neither `workers` nor `fullyParallel`, so it
  inherited Playwright's default `workers = 50% of CPUs`. The production config
  pins both to serial deliberately; the visual lane was quietly opting out of
  the anti-flake posture the rest of the suite is configured for.

* chore(gates): pin the documented gate count to the real chain

Both numbers were wrong. `CLAUDE.md` said 24 static/consistency gates against an
actual 25 — `check:assets` landed before that line was written, so it was wrong
at authoring — and the `gates` skill said "check 2 of 26" against an actual 28.

A stale count is not cosmetic here. The skill's whole point at that line is that
`verify:cheap` stops at the first failure and everything after it never ran; an
agent that believes the chain is 26 long cannot say how much a mid-chain failure
skipped.

`check:gate-manifest` already derives the real count from
`verify:cheap:internal`, so it now asserts the documented numbers against it.
The assertions fail closed: if the anchor phrasing disappears, the guard reports
a lost anchor rather than passing on a document it no longer checks.

Mutation-proven: reverting the skill to "26" fails with
".claude/skills/gates/SKILL.md says 26 where the chain has 28".

* docs(issues): capture the CI review's deferred findings

Five items from the CI/testing review that should not be changed blind:

- #125 `ui_changed` matches all of `src/app`, so an API-only diff pays the
  15-minute UI gate. Narrowing it can hide a real regression, so it needs a
  decision plus a compensating check rather than a quieter filter.
- #126 the Playwright build writes to a per-run distDir, so Next's build cache
  is cold every run (~2 min, now ~29% of the sharded critical path). Fixing it
  means suppressing the runner's documented always-cleanup, which must not ship
  without executing the runner.
- #127 the advisory UI lane spends ~3 min per UI PR on 5 mockup tests; there are
  currently zero `@quarantine` tests for it to cover.
- #128 CI Triage is complete and self-tested but inert pending a repo variable.
- #129 four `changes` outputs are computed and consumed by nothing, and
  `coverage_changed` fires on any non-doc file.

* docs(ledger): record the ci-testing-review pass at this HEAD

* ci: re-measure the shard split on the merged tree and refresh stale gate counts

The merge changed both numbers this branch had recorded.

Shard balance, re-measured against 342 required chromium tests (was 340):
  N=3 -> 121/111/110    largest 121
  N=4 -> 121/106/98/17  largest 121
N=3 remains correct — one 121-test spec group bounds both, so N=4 spends an
extra runner for the same critical path. The re-measure command is now in the
workflow comment so the next person does not have to rediscover it.

Gate counts: merging main added `check:gitleaks-pinned` and
`check:pr-mergeability` to `verify:cheap:internal`, so the documented counts
went stale the moment the merge landed — 25 -> 27 static, 28 -> 30 total. The
guard added earlier in this branch caught it immediately rather than letting the
docs drift again, which is the whole reason it exists.

Also records the `ui-critical-fast` interaction: the UI critical path is now that
15-test fail-fast job plus the slowest shard, not the full 13.5-minute suite, so
neither of this branch's pre-merge timings can be read on its own.

* docs(issues): rebuild the ledger after a union-merge duplication

The `merge=union` driver on `docs/outstanding-issues.md` preserves concurrent
appends, but when both sides restructure the same region it concatenates them
wholesale. Merging the latest main did exactly that: every open row appeared
twice and both `issues:next-id` markers survived — 66 duplicate-id errors from
`check:outstanding-issues`, which is precisely the failure that gate exists to
catch (#112).

Resolved by rebuilding on main's canonical file rather than by hand-editing the
duplicated table: reset to `origin/main`, then re-apply this branch's five
captured rows at #131-#135 (main had advanced its allocation to #130 while this
branch was open, so the earlier #128-#132 numbering collided again) and
re-apply the #127 narrowing note. Marker bumped to 136.

Union merge cannot allocate unique ids; only the structural gate can catch when
it has produced an invalid file. It did.

* ci: record the measured shard result, correcting the predicted one

First real run of the sharded shape (CI 30530618838, all green, whole run
13m39 against a 16.8-18.6 min unsharded baseline):

  ui-critical-fast  15 tests   3m14
  Production UI (1) 121 tests  9m36
  Production UI (2) 111 tests  6m54
  Production UI (3) 110 tests  6m20

The prediction was wrong by ~40%. ~6.8 min was expected for the largest shard
from 121/342 tests x 13.5 min; 9m36 happened. Per-test cost is not uniform —
111 tests took 6m54 while 121 took 9m36 — so a count-balanced split understates
the slowest shard whenever the slow specs land in one group. `--shard` can only
balance by count; balancing by duration would mean splitting the slow spec files
themselves.

The win is real but smaller than claimed, and the workflow comment and
process-hardening now carry the measured numbers plus the reason the arithmetic
misleads, so the next person re-measures instead of re-deriving.

Also merges origin/main. The ledger conflict was GitHub-visible only: that file
carries merge=union locally, which GitHub does not honour (#129). Resolved by
keeping the one genuinely new record and dropping three that main already had
elsewhere in the file — append-only forbids dropping a record that exists once,
not keeping a second copy. Superseding record appended for this HEAD, since the
prior one asserted a root cause that #127's trace evidence refutes.

* docs(issues): renumber this branch's rows above main's concurrent allocation

CI caught what I did not: `static-pr` failed on `check:outstanding-issues` with
#131-#134 duplicated and two `issues:next-id` markers.

Cause: main's PR #1424 allocated #131-#134 for its own findings at the same time
this branch held #131-#135, and `merge=union` did what union does — kept both
sides under the same ids. That is #112's documented limit: union preserves
concurrent appends but cannot allocate unique ids, so the structural gate is the
only thing that catches it.

My error was pushing without re-running that gate. The previous push resolved a
`docs/branch-review-ledger.md` conflict, and I validated only that file before
pushing to win the race against main — but the same merge also touched
`docs/outstanding-issues.md`. `verify:cheap` would have caught it locally.

Main's rows keep #131-#134 (already merged and referenced elsewhere); this
branch's five renumber to #136-#140, one marker at 141, and the cold-cache
cross-reference in process-hardening follows its row.

Two of main's new rows also make a planned addition here redundant: #134 is the
absent ledger merge driver and #133 is the outstanding-issues merge churn — both
hit during this branch's work, both already captured upstream, so nothing new is
filed for them.

* docs(issues): rebuild against main's current id allocation

The union merge duplicated the whole open and archive tables again (two header
rows, every id twice) because main restructured the file while this branch held
rows in it. Same resolution as before and for the same reason: rebuild on main's
canonical file rather than hand-editing a doubled table, then re-apply this
branch's five rows.

Main is now at next-id=135, so they land as #135-#139 with the marker at 140.
None of the five is duplicated upstream — checked by summary before re-applying.

This is the third renumber of the same five rows in one PR. That is not a
mistake being repeated, it is #133 ("outstanding-issues conflicts on nearly
every main advance") happening: any branch that holds rows in this file
re-collides every time main lands one. Worth weighing whether captures should
land in their own PR ahead of the work rather than riding along with it.

* docs: record PR 1427 review

---------

Co-authored-by: Claude <noreply@anthropic.com>
BigSimmo pushed a commit that referenced this pull request Jul 30, 2026
…it asks for

A real conflict this time, not staleness: main's PR #1427 rewrote the same
helpers this branch touches, and it very likely found the actual cause of
#127. `addPhoneScrollRunway` slept 50ms and merely hoped the appended 1600px
runway had reached layout; `dragScrollBy` clamped silently at the end of the
range while reporting nothing. Under CI load the drag therefore delivered
less than it asked for and the chrome was right to stay visible. #1427 polls
for the runway, returns the distance actually travelled, and
`dragScrollUntilHidden` refuses to expect a hide until remaining runway and
delivered travel both clear 160px.

Main's helpers are taken whole. This branch keeps only what #1427's own
comment says is still missing: "Separating THOSE two still needs the pin
state exposed in the DOM; today only the composite `data-scroll-hidden`
(`scrollHidden && !sharedChromePinned`) is observable, so both look
identical." `data-scroll-signal` publishes the raw signal, and
`expectChromeHidden` is cut down to answer only that question, now layered
after `dragScrollUntilHidden` rather than duplicating its travel proof.

#127 is rewritten again and withdraws a second wrong diagnosis of my own: a
short/clamped drag was ruled out early using a maxOffset of 2753 read at a
different trace moment than the failing drag, when the pre-runway reading in
that same trace was 1153 — and a runway not fully landed puts the offset in
the near-bottom band where computeScrollHideUpdate legitimately refuses.
That is exactly what #1427 fixes. The row now points at #1427 as the likely
fix, keeps the observability gap as the only open part, and says to close it
if no recurrence appears on a post-#1427 head.

Main also claimed #135 for an unrelated issue, so the union-driver finding
renumbers to #140, marker 141.

Verified: typecheck 0 errors, lint 0 problems, whole-tree prettier clean,
check:outstanding-issues 138 rows / unique ids / next-id=141, and the merged
phone-scroll spec 56 passed (4.1m) against an isolated production build —
under Chromium 1194, not CI's bundled 1234 (#121).

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 pushed a commit that referenced this pull request Jul 30, 2026
Codex was right, and the finding is worth stating plainly: the earlier rows
argued that because the sibling `documentScrollTop` assertion did not fail, the
scroll position held and scroll-restoration causes were ruled out.

Playwright aborts a test at the first failing `expect`. Once the `anchorTop`
assertion threw, the `documentScrollTop` assertion never executed at all — its
absence from the failure output is evidence of nothing. The narrowing is
withdrawn and that class of cause is explicitly not ruled out.

The capture itself is unaffected: the Services viewport-anchor failure is real,
intermittent on byte-identical code (pass / pass / fail / pass-on-rerun), and
distinct from #127.

A superseding ledger record carries the correction rather than editing the
earlier rows, per the append-only rule.
BigSimmo pushed a commit that referenced this pull request Jul 30, 2026
…and #146

Conflict was docs/outstanding-issues.md only. Took main's copy wholesale (it
carries #122's closure and the #98/#130 records from #1455) and re-applied the
two one-line relocation notes, rather than hand-editing the conflict region.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JHLPEV4o1rzipPDqshCSHY
BigSimmo pushed a commit that referenced this pull request Jul 30, 2026
Second time a real conflict has blocked this PR's CI entirely — GitHub could
not build refs/pull/1466/merge, so no pull_request workflow ran and the thin
check list read as pending rather than blocked (#116).

Only docs/outstanding-issues.md conflicted; scripts/ci-change-scope.mjs and
docs/process-hardening.md auto-merged (main's regions are 150+ lines from this
branch's). Took main's ledger wholesale rather than hand-editing a 140-row
table around conflict markers.

Only ONE of this branch's two ledger edits was re-applied:

- #146 keeps its relocation note. The row is still open, and main's #1481 added
  two further data points to it (head c739340, anchorTop expected -138 received
  -7; six data points, two failures, shard 1 only) which are left untouched.
- #127's edit is DROPPED as obsolete. Main's #1487 archived that row, and the
  archived form no longer cites tests/ui-phone-scroll.spec.ts at all, so there
  is nothing left to relocate. Re-applying it would have matched nothing or
  corrupted a differently-shaped row.

Marker is main's 149 (not this branch's 147): #149 was allocated, withdrawn and
retired rather than reused.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JHLPEV4o1rzipPDqshCSHY
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.

3 participants