Conducting CI testing for PR #111 - #113
Merged
Merged
Conversation
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
…gaps #112 (P2): verify:phone-chrome exited 0 while its own output ended "13 failed". All 13 failed at browser launch, so zero assertions ran and the gate still returned success -- the green-when-broken case AGENTS.md warns about, in the gate that is meant to be the proof. #113 (P3): the container's Playwright browsers are build 1194 while the repo's client resolves 1234, so browser gates cannot run in a remote session without hand-patching. Worked around in-session with symlinks under /opt/pw-browsers; nothing committed, and it vanishes with the session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
BigSimmo added a commit
that referenced
this pull request
Jul 30, 2026
…ed (#1405) * fix(mode-nav): size slots to their content so labels stop being clipped Ledger #113. `ModeNav` promises a slot shows its real word or folds into More, and it broke that promise at every phone width. The two lower bands used `grid-auto-columns: 1fr`, so the WIDEST slot set what every slot got, and the label's `truncate` hid the shortfall — nothing overflowed, nothing failed, the word was simply gone. Measured in Chromium before changing anything: 320px showed 15 of Compare's 56px, 390px showed 39, and 430px was WORSE at three of four labels clipped, because crossing a band adds a slot to the same space. The bar got less legible as the screen got wider. Only the widest band was clean — the one that already sized slots to their content. So that branch is now the only branch. `display: flex` at every band means no track can be narrower than its own label, which removes the failure mode rather than retuning it, and deletes a layout mode rather than adding one. It is also what the author already chose for wide screens: "slots size to their content and sit left, so the bar reads as a continuation of the header". Thresholds are re-derived from measured intrinsic widths (Search 92.4, Compare 144.3, Recommend 125.6, Pathways 108.5, More ~77, plus the bar's 8px padding) rather than the previous budget, which was ~80px short — about the size of the count badge nobody counted. 16rem -> 21rem for three slots, 26rem -> 31rem for four, 34rem -> 40rem for five. A fixed `rem` threshold still cannot prove fit for an arbitrary item list, and `ModeNav` is shared. `tests/ui-mode-nav-density.spec.ts` holds that gap: at both band boundaries and one pixel either side, every rendered label must be fully visible and the bar must not overflow. A mode with longer labels now fails CI instead of shipping clipped words. Registered in BOTH Playwright allowlists — `testMatch` and `productionSpecPattern` — because a spec missing from either silently never runs. The spec scopes to the nav inside the collapse host rather than the bare test id. That is not incidental: streaming can resolve the id to two elements (ledger #93), and the density decision is a container query that only means anything once the portal has placed the bar in the container it lives in. Verified against the pre-fix CSS with the new spec in place — 6 of 7 red, reporting e.g. `336px: "Compare" needs 56px, has 21px`. verify:cheap exit 0 — Test Files 432 passed (432), Tests 4450 passed | 4 skipped. Chromium, all Therapy/ModeNav journeys — 18 passed (56.7s). Text zoom now behaves as the `rem` mechanism claims: a 390px phone at a 32px root is 12.19rem and falls back to the collapsed control instead of clipping. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvU8z73P6TXUXoYBqN5K1P * fix(mode-nav): give the thresholds cross-environment font headroom CI caught what my local measurement could not: at 496px the boundary case failed by exactly one pixel — `"Compare" needs 59px, has 58px`. Locally the same label measured 56px. The variable face rasterises wider on the runner, so thresholds calibrated to one machine's font metrics are calibrated to nothing. Raised to 22 / 33 / 42rem, roughly 8% over the measured intrinsic widths rather than the pixel or two a single box suggests, so the bands clear the whole spread of environments instead of the one I happened to measure in. This is the same failure the boundary tests exist to catch, so they did their job on the first CI run rather than letting a one-pixel clip ship — the tests at 351/352/353 and 527/528/529 are deliberately the least forgiving place to assert, and that is the point of putting them there. Chromium, tests/ui-mode-nav-density.spec.ts — 7 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvU8z73P6TXUXoYBqN5K1P --------- Co-authored-by: Claude <noreply@anthropic.com>
10 tasks
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
The #113 archive row cited `c2edda18` as PR #1405's merge commit. That was the branch tip, which the squash merge discarded, so the claim was already unverifiable when it was written: git merge-base --is-ancestor c2edda1 origin/main -> fails git merge-base --is-ancestor 020c126 origin/main -> ok 020c126 fix(mode-nav): size slots to their content ... (#1405) In a ledger whose whole purpose is durable, auditable resolution history, a resolution nobody can check is worse than no resolution note. The trap is that this repo mixes merge strategies: #1407 and #1410 landed as merge commits, so their branch SHAs stay reachable; #1405 was squashed and its did not. Copying the PR head works three times in four, which is exactly the kind of rule that survives review until it doesn't. Verified after the edit: `c2edda18` no longer appears anywhere in the file, the replacement is an ancestor of main, and `check:outstanding-issues` still passes with `117 rows (55 open, 62 archived), unique ids, next-id=118 above the highest`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvU8z73P6TXUXoYBqN5K1P
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
Whole-table conflict in docs/outstanding-issues.md — main advanced 44 commits and gained rows while this PR sat open. Resolved by rebuilding from main's current file and re-applying this branch's three changes, not by taking either side: settling a whole-table conflict by picking a side is the row-loss failure ledger #112 exists to make loud. Renumbered the new row #117 -> #125. When this branch opened it, the marker was 117; main's marker is now 125 and #117..#119 were reserved by other sessions in the meantime, so keeping #117 would have collided with a row someone else is about to add under that number. Ids are never reused — renumber the incoming row above the marker and bump it. Audited row-by-row against origin/main rather than eyeballed: main rows: 119 resolved rows: 120 LOST from main: (none) ADDED vs main: #125 MOVED table: #113 open->archive, #114 open->archive duplicates: (none) check:outstanding-issues: 120 rows (58 open, 62 archived), unique ids, next-id=126 above the highest. verify:cheap exit 0, Tests 4493 passed | 4 skipped. format:check clean. `c2edda18` still absent from the file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvU8z73P6TXUXoYBqN5K1P
BigSimmo added a commit
that referenced
this pull request
Jul 30, 2026
… audit batch (#1426) * chore: close#108 after deleting empty remotes; record audit-branch cleanup batch Delete five superseded remote refs (#108 trio plus circleci-project-setup and execute-audit-code-remediation). Archive #108 and append branch-cleanup ledger rows for a ten-branch audit/remediation review batch (two rejected, eight retained). Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix: resolve outstanding-issues merge without conflict markers Rebuild from origin/main (archives #113/#114, adds #125) and re-apply the #108 archive so the duplicate-id and conflict-marker breakage from the prior merge commit is gone. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs: correct branch cleanup ledger references * docs: record PR 1426 review --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
2 tasks
BigSimmo added a commit
that referenced
this pull request
Aug 7, 2026
…rmulation and Differentials (#1647) * feat(navigation): put DSM on the shared mode bar, and state addon-slot ownership Starts the site-wide rollout of the header-integrated `ModeNav` that #1642 shipped for Therapy. DSM first: two destinations, so no More slot and no overflow band to calibrate. `RegistryModeNav` adapts the canonical `modeSecondaryNavigationRegistry` to `ModeNavItem` rather than each mode hand-writing a list, so the shell and the mode cannot drift onto different URLs for the same tab. Its icon map is exhaustive by a type derived from the registry literal: adding a routed entry without choosing an icon fails the typecheck instead of shipping a document icon nobody picked. Also closes an invariant that until now held by accident. The header addon slot takes ONE page-owned header, and what kept `documents` and `differentials` clear was that both had fewer than two destinations, so `ModeNav` rendered nothing. That protection expires the moment a claimant's mode gains one. `/differentials/diagnoses/<slug>` is the live case: it is locally-owned information navigation, so it skips the sections branch, and a submitted search makes `isModeSecondaryNavigationRoute` true ahead of every per-mode check — mounting a second header into a slot `DifferentialDetailPage` owns. `isHeaderAddonSlotOwnedRoute` now names the claimants, and the render-level test asserts one occupant rather than inferring it from the item count. Adoption is an explicit per-mode list, not a "has two routed entries" derivation, so a registry edit cannot silently move a mode onto a different navigation surface. The eight single-destination modes keep their existing `SecondaryNavigation` strip untouched — dropping it is a deletion decision about their lone entry, not part of this port. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S9mA7QhMw1hpZCVMzH7pZL * feat(navigation): move Specifiers and Formulation onto the shared mode bar Replaces the two page-owned strips (`SpecifierSubnav`, `FormulationSubnav`) with the header-integrated bar, and drops the shell exclusion that skipped `PageSecondaryNavigation` for both modes — that exclusion existed only to stop a duplicate row next to those strips, and with the strips gone it would have suppressed the modes' only navigation. Desktop labels shorten: "Build wording" / "Build formulation" / "Mechanism map" become the registry's "Build" / "Map". `ModeNavItem` has no `shortLabel` by design (a slot shows its real word or folds into More), and the registry is now the single source of the wording. Lengthening it instead was the alternative, but "Build formulation" plus "Mechanism map" do not fit the calibrated 42rem band, and ledger #113 is explicit that the thresholds are measured, not padding to be widened. Both mode homes go bar-free, as every mode home is: `/specifiers` and `/formulation` already surface builder, compare and map as tiles, so nothing is stranded by the removal. Their `[slug]` detail routes keep the information-page sections nav they share with every other record route, rather than the mode strip they used to carry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S9mA7QhMw1hpZCVMzH7pZL * feat(navigation): move Differentials onto the shared mode bar Completes the four multi-destination modes. Differentials is last because it is the only one with a route that portals its own header into the universal header's single addon slot, and that slot takes one occupant. The protection there was never stated. It used to be that `documents` and `differentials` — the only modes with a claimant — had fewer than MODE_NAV_MIN_ITEMS destinations, so `ModeNav` rendered nothing; Differentials now has three. What holds it today is a second coincidence: every claimant route is also `hasLocalInformationPageNavigation`, which `PageSecondaryNavigation` returns null on well before the mode branch. Two separately maintained lists that happen to agree. Rather than add a guard that cannot execute, `isHeaderAddonSlotOwnedRoute` names the claimants and the test asserts the agreement route for route, plus the render-level outcome. Breaking the cover fails three assertions; the earlier explicit guard failed none, which is what showed it was unreachable. When a future claimant falls outside the cover, that is the signal the mode branch needs a guard of its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S9mA7QhMw1hpZCVMzH7pZL * test(navigation): drive the mode-nav density spec over every adopting mode The spec's header promises that a mode whose labels are longer than the thresholds allow fails there rather than shipping clipped words. Every route it drove was `/therapy-compass/*`, so that promise covered exactly one mode. Four more have now adopted the bar; their labels are short enough to pass either way, which is why the gap survives until a mode with long ones arrives. Each mode is loaded at every band boundary and one pixel either side, with the expected slot count derived as `min(items, capacity)` rather than tabulated, so a mode with a different destination count needs no second table. Two cases are new behaviour, not just new coverage: Specifiers and Formulation carry four destinations, so at the 22rem band their last item folds and the More slot has to carry the current page's rule. That is invariant I5's container-query branch, and no route outside Therapy exercised it before. `tests/mode-nav-contract.test.ts` pins the spec's mode table against MODE_NAV_ADOPTED_MODES and the registry, so adopting a mode or adding a destination without extending this spec fails offline instead of leaving a mode unmeasured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S9mA7QhMw1hpZCVMzH7pZL * fix(navigation): restore record-page section anchors for mode-nav adoption (#1647) Wire the declared specifier/formulation information-section IDs into the record pages so "On this page" actually renders after Subnav removal. Also pin MODE_NAV_ADOPTED_MODES to the ≥2 routed-entry criterion, and stop marking Find/Search current on unmatched in-mode routes. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * test(mode-nav): always cleanup addon-slot DOM node after each case Co-authored-by: BigSimmo <87357024+BigSimmo@users.noreply.github.com> * test(navigation): pin which modes adopt the bar, not just the criterion The adoption test asserted that every listed mode has two or more routed entries, and that modes with fewer stay off the list. Neither half notices a mode being removed: drop `formulation` and the remaining three still satisfy the criterion, while the negative check only inspects modes with fewer than two entries. A mode silently losing its only navigation is precisely the regression the explicit list exists to prevent, so pin the membership as well. Raised in review. Confirmed by mutation: removing `formulation` passed before this, and fails after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S9mA7QhMw1hpZCVMzH7pZL * fix(test): address the formulation section label by id, not by its words `Production UI` caught this; my local run of the spec predated the commit that caused it. Wiring up the declared section anchors made `/formulation/<slug>` render an "On this page" nav, and its link carries the same words as the record's own section label — so `getByText("What matters now", { exact: true })` now resolves to two elements and fails Playwright strict mode. The assertion wants the record's label, so address it by `#formulation-what-matters-now-label` rather than by text that two elements legitimately share. `ui-specifiers.spec.ts:93` uses the same text but is not affected: "What matters now" is not one of the specifier section labels, so no nav link carries it there. Confirmed by running both specs, 12 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S9mA7QhMw1hpZCVMzH7pZL * docs(ledger): record PR #1647 mode-nav review at HEAD * fix(navigation): match mode-nav active ids by path segment (#1647) Specifier/Formulation active-slot detection used pathname.includes for builder/compare/map, so a future slug containing those substrings would falsely claim aria-current. Align with the exact-segment checks already used by isModeSecondaryNavigationRoute. * fix(test): pin formulation On-this-page link and specifier label by id Production UI failed when getByText("What matters now") matched both the formulation record label and the newly-wired On this page nav link. The tip already addressed the label by id; assert the nav link through its own role scope so the dual presence stays intentional and cannot regress into a second strict-mode collision. Mirror the id-scoped locator on the specifier record path so that suite does not depend on the current coincidence that specifier section labels omit those words. * ci: retrigger required checks after Actions outage (#1647) Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.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 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.
Pull request created by AI Agent