Polish: de-halo selection glows, neutral answer skeleton, verified gates - #116
Merged
Conversation
…d gates - Retune --glow-primary/--glow-soft to a crisp Aegean ring + neutral shadow (no colored ambient halo) in light and dark, fixing the last glassy selection states (launcher tile, selected chips, viewer citation highlight). - Move the answer loading skeleton off the accent wash onto a neutral raised panel; step shimmer bones to --surface-inset for visibility on true white. - Record verification results in the redesign changelog: typecheck, eslint, vitest 688 passed/2 skipped, ui-smoke 29/29, ui-accessibility 2/2 (chromium). Presentation-only; no clinical logic touched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
5 tasks
BigSimmo
pushed a commit
that referenced
this pull request
Jul 30, 2026
Two review findings, both about working-tree state leaking into the verdict: - A dynamic `prettier.config.mjs` could not be evaluated by the previous hand-staged config approach, so the guard fell back to the working-tree config — recreating the mismatch it exists to prevent. - A push that changes prettier policy (.prettierrc, .prettierignore, .editorconfig, package.json) changes the verdict for files the push never touched, which a changed-paths-only check cannot see. Both are answered by checking the pushed commit where CI checks it: a `git worktree` of that sha (<1s here) with node_modules linked in, so a dynamic config loads its plugins. A policy change escalates to a whole-tree `prettier --check .`; otherwise only the changed paths are checked. Failure to create the worktree now fails closed — being unable to check is not evidence the push is clean. Verified on six isolated scratch repos, exit codes 1/0/1/1/1/0: committed unformatted with a clean working tree blocks; all-clean passes; a committed-only broken static config blocks; a committed-only broken dynamic config blocks (was passing); a config-only change that breaks an untouched file blocks (was passing); a benign config change still passes. Also records #116: an unmergeable PR runs no CI at all and says nothing, which is what cost this branch three pushes of missing checks today. 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
Second conflict in docs/outstanding-issues.md in under an hour, same shape: main re-padded the table so the hunk spanned every row, while only two rows differed. Took main's #108 (completed by #1403) and kept this branch's #115 closure. #116 does not collide — main's next-id was still 116. This recurrence is the case #116 itself describes: while conflicted, the PR ran no CI at all and nothing said so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
7 tasks
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
added a commit
that referenced
this pull request
Jul 30, 2026
BigSimmo
added a commit
that referenced
this pull request
Jul 30, 2026
* process: cut future PR conflict churn and silent CI gaps Add outstanding-issues ID/marker/union guards (#112), a read-only pull_request_target mergeability signal for dirty heads (#116), and an anti-conflict CI-speed operating procedure that prefers bundling and format-before-push without weakening required gates or touching active PRs. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): record PR #1416 merge-readiness as not ready Main advanced with #1410 overlapping the outstanding-issues gate; merge-tree is conflicting. Unique value remains the anti-conflict playbook, #116 signal, and merge=union. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * style(issues): format outstanding-issues after main sync Prettier realigns the archive table after the #116 close and #112 note update. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): supersede #1416 merge-readiness as ready Record the post-sync review: merge-tree clean, unique #116/process value kept, duplicate #112 checker dropped in favor of #1410. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): pin #1416 READY review to final tip SHA Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(test): shrink zip-bomb fixture to stop CI coverage timeout Unit coverage failed because reject-high-compression-ratio allocated and deflated 24MB of zeros under the 30s Vitest timeout. 1MB still exceeds the 150:1 admission ratio (~480:1) without the CI flake. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix: refresh mergeability after base advances * fix: preserve ledger rotations during merges * fix: resolve issue ledger merge collision --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo
pushed a commit
that referenced
this pull request
Jul 30, 2026
Third main sync on this branch. The new `PR mergeability` check on main caught the drift, which is #116's fix working as designed. The replay script now carries the review-corrected #95 text. Re-running it against a fresh main would otherwise have reintroduced the exact `cancelled_error` description Codex flagged, since the script held the pre-review wording. main's .gitattributes now sets `merge=union` for this file and SKILL.md adds: "Never resolve a conflict by taking one side wholesale — that drops the other agent's rows." Proven not to happen here, by ID-set comparison against origin/main rather than by assertion: origin/main : 60 open, 65 archived, 125 total resolved : 55 open, 70 archived, 125 total IDs lost from main: NONE IDs added: NONE main's open rows preserved except the 5 moved: True main's archived rows all still archived: True So the result is a true union: main's 60 open minus the five archived equals 55, and main's 65 archived plus those five equals 70. Verified: npm run verify:cheap -> EXIT=0; "Test Files 434 passed (434)"; "Tests 4563 passed | 4 skipped (4567)"; "Outstanding-issues guard passed: 125 rows (55 open, 70 archived), unique ids, next-id=128 above the highest, union merge active". npx prettier --check . -> "All matched files use Prettier code style!". Zero conflict markers. 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
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
Resolves the conflict that was silently stopping ALL CI on PR #1427: with a real conflict GitHub cannot build refs/pull/1427/merge, so every `pull_request` workflow was skipped with no failing check while `pull_request_target` ran. That is issue #116, and main's new `PR mergeability` check named it. Conflict resolutions: - `.github/workflows/ci.yml` — keep BOTH. Main split the UI gate into `ui-critical-fast` (@critical fail-fast) which `ui-critical` now `needs:`; the shard matrix lands on `ui-critical` only, preserving main's `needs:`/`if:` verbatim. `ui-critical-fast` stays unsharded: it is already the small subset, so sharding it would spend runners without shortening the critical path. - `playwright.visual.config.ts` — take main. It already sets `fullyParallel: false` / `workers: 1`, so this branch's version of that change was redundant and is dropped entirely. - `docs/outstanding-issues.md` — main allocated 125/126/127 concurrently, the exact #112 id race. This branch's five rows renumber to #128-#132, marker to 133, and the cold-cache cross-reference follows. - `docs/process-hardening.md` — keep both sections; the CI-timing note now records that its numbers predate `ui-critical-fast`.
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
This was a REAL conflict, not staleness, and it is why PR #1466 had no `CI` workflow run at all — only the four `pull_request_target` checks. GitHub could not build refs/pull/1466/merge. That is #116's signature: a missing check list is a conflict signal, not a green pass. `git merge-tree` exited 1 on tests/ui-phone-scroll.spec.ts while `PR mergeability` still showed success from before main advanced. main's #1432 edited a hunk inside the `phoneOwner` loop — precisely the region this branch moved to ui-phone-scroll-page-owned.spec.ts — so git saw one side delete the region and the other modify it. Resolved by taking this branch's shell-only ui-phone-scroll.spec.ts and re-applying #1432's change at its new home. That change is a genuine flake fix (sheet focus restoration retries after 50 ms when focus falls through to <body>, so a bare blur races the retry and can re-pin the header), and dropping it in a merge would have silently reintroduced the flake — the same fails-by-running-nothing class this branch exists to close. It went to the occurrence in the section-sheet round-trip, matched by the hunk's own context, not the similar-looking line 95 lines earlier. No content lost, proven rather than asserted: every non-trivial line of main's ui-phone-scroll.spec.ts is present across the three split files plus the shared helper. The only 22 differences are declarations that gained an `export ` prefix and the two imports whose path changed depth. Verified on the merged tree: verify:cheap exit 0, Test Files 437 passed (437), Tests 4588 passed | 4 skipped (4592); tsc --noEmit clean; prettier --check . clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHLPEV4o1rzipPDqshCSHY
16 tasks
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
BigSimmo
added a commit
that referenced
this pull request
Jul 30, 2026
) #133 asked to "stop padding this table (Prettier will still render it readably)". Measuring it showed the fix was infeasible as written, because Prettier is what pads the table: stripping the padding and running `prettier --write` restored the file byte-for-byte identical to the original. Un-padding therefore requires prettier-ignoring the file, which this does — following the precedent already set for docs/branch-review-ledger.md, which is ignored for the same reason. Nothing structural is lost. check:outstanding-issues gates row shape, ids, the next-id marker and the merge attribute far more strictly than column alignment ever did, and it is required in verify:cheap and CI static-pr. The mechanism is also narrower than #133 claimed, and the row is corrected rather than left overstating it. "A single row's edit re-pads all 59 open rows" holds only when the edit raises a column's *maximum* width; an edit inside the existing maxima is 2 changed lines either way. Measured on the real file: lengthen a Detail cell padded 2 lines un-padded 2 lines widen a Summary past max padded 144 lines un-padded 2 lines That distinction is the point rather than a caveat: appending a new row is precisely the max-raising operation, and appending is what agents do to this file constantly, so the worst case was also the common case. A 144-line hunk conflicts with anything else touching the table, and each such conflict stops all CI on the PR until resolved (#116). The one-time reformat is 178 rows. It will conflict with any open PR that touches this file, so merge it when the queue is quiet rather than mid-sweep. Verified: check:outstanding-issues 144 rows (70 open, 74 archived), unique ids, next-id=147, no merge driver; docs:check-links 1407 references resolve; whole-tree prettier clean with the file now ignored. Not caused by this change: verify:cheap exits 1 on tests/installed-lock-parity.test.ts ("expected 10000000 to be less than or equal to 40000"). It fails identically on clean main with none of these changes applied, so it is pre-existing and left untouched rather than absorbed here. Everything else passed: 436 passed (437) files. Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ Co-authored-by: Claude <noreply@anthropic.com>
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 free
to 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.
Summary
Perfection pass on the Clinical White / Aegean Graphite theme (follow-up to #112):
--glow-primary/--glow-softare now a crisp Aegean ring + neutral shadow (no colored ambient halo), fixing the last glassy selection states (launcher active tile, selected chips, DocumentViewer citation highlight)--surface-insetso they read on true whiteClinical governance preflight
Presentation-only. No changes to ingestion, answer generation, retrieval/ranking, document access, privacy, production env, or clinical output.
Verification (all run this pass)
npm run typecheck✓ ·npm run lint✓tests/ui-smoke.spec.ts(chromium): 29/29 passedtests/ui-accessibility.spec.ts(chromium): 2/2 passedverify:release(full build + e2e + eval)🤖 Generated with Claude Code