Set-field column headers (E-d): the unit leaves the field, the gate proves the budget

[stslex]

Implements set-field-column-headers.md (spec on dev, b366ee8c). Ten commits; every gradle proof ran with --rerun-tasks --no-build-cache --no-configuration-cache, detekt as separate serial invocations.

Correction: the branch is NOT bisect-green, and this PR no longer claims it is

An earlier revision of this description said "bisect-green commits". That is wrong and is withdrawn. Each commit is green for unit tests, both overflow gates, the alignment gate, detekt and lint — but verifyPaparazziDebug exits 1 on every commit from the suffix removal (56370b5) onward — seven commits, not one — because the code that moves the render was deliberately sequenced before the goldens that record it (Gate 2 exists precisely so the executor never re-records unilaterally). Making the branch genuinely bisect-green would mean folding the 36 PNGs into the commit that moved the render: a full-history rewrite, not the R25 squash that landed. Under a squash-merge, dev never sees those commits and the invariant holds by construction; under a merge-commit it does not. Ilya decides at merge.

What changed

• SetColumnHeader (kit): one AnnotatedString in one Text — name textSecondary, unit a textDim tail span, locale-aware uppercase() in the component; ellipsis order proven by test (unit cut, name intact at 150/130/110/90px).
• SetRowGeometry (kit): D3's single source — index minimum, the lifted WEIGHT_COLUMN_FLEX, the chip-slot reference, and resolveIndexColumnWidth (measured through the real text stack, survives fontScale). Containers resolve once; header and rows grow together at 10+ sets (exerciseTenSets canary).
• Suffix removed from all six set-row call sites; header wired into SetsColumn + CardBody; .height(48) → .heightIn(min = 48) (proven pixel-neutral against 100 goldens before wiring).
• D6: AppNumberInput.accessibilityLabel → semantics { contentDescription } (the AppTextField template); rows announce "вес, килограммы" / "повторения"; Robolectric runComposeUiTest asserts it. isError → error() deliberately NOT added (R5 veto, spec blocker B-2).
• D5: the value rung is now measured — first fit on the [26sp, 19sp] ladder against the parent-given slot; MAX_GLYPHS_AT_FULL_SIZE deleted. Acyclic (slot is flex-driven). PlanSetCard ripple pinned by the new readOnlyFiveGlyphWeight golden (R3); its 16 existing PNGs byte-identical.
• R8: the stale AppNumberInput width-budget KDoc now carries the measured figures (18.2dp/digit, 81.12dp for "102.5", 68.38/35.75dp boxes).
• Dead unit_reps_full removed (its only two consumers became the header).

The gate (R1/R2, proven both ways)

OverflowGateSdk drives PaparazziSdk with a discarding frame consumer — no snapshot handler, zero PNGs, non-golden packages; assertGoldenLiveness never sees it (full corpus run confirms). Slot widths are captured from the rendered production rows via valueSlotProbe (the flashAlphaOverride move).

• Known-positive (pre-fix): red with exactly the predicted cells — live reps slot 98px vs "12" at 100.0px (overflow by 2.0px, predicted 1.8); reps ×{2,3,5} glyphs red, weight green, past reps ×{3,5} red.
• Known-negatives: SIZE_TITLE_SP 26→40 grew failures 33→41; 26→12 cleared every title-rung cell (33→22; the residue is the section rung's own). CHIP_MIN_WIDTH 34→60 from the green baseline: gate red (reps slot 187→155px, 5-glyph +25px) — the loop is closed.

⛔ STOP GATE 2 — 36 moved goldens, prediction matched exactly

Full corpus (verifyPaparazziDebug, all 13 modules, rerun flags): 36 failing golden verifications, exactly the predicted set, zero divergence in either direction:

• SessionStateGoldenTest 20/30 — direct rows (suffix glyphs gone, boxes widen; "102.5" returns to 26sp) + 3 expanded-card cases (header row inserts)
• SessionScreenGoldenTest 2/2 — expanded pe-2 card in frame
• PastSessionGoldenTest 14/30 — expanded cards + direct rows
  All other 410 PNGs verified byte-identical (incl. every PlanSetCard consumer and the kit corpus). Not re-recorded — your call. Reproduce: ./gradlew :feature:live-workout:recordPaparazziDebug :feature:past-session:recordPaparazziDebug and read the image diff; the six new snapshots in this PR are first recordings, not re-records.

⛔ R4 band ruling needed — the ladder floor loses 8 cells (spec §7 ledger)

With the stepdown live, the measured 64-cell matrix passes 56/64. Every failure exceeds even the contrast-pinned 19sp floor (which the non-linear converter scales ×~1.695 at fontScale 2.0): reps×5-glyph fails the hard band at 1.3/1.6 (live) and 1.6 (past); at 2.0, four cells clip beyond the single sanctioned weight/5 limit (incl. live reps×3 by 2px). Options in spec §7: (a) cap reps input length, (b) sub-19sp rung for non-record/non-pending states, (c) extend the known-limit ledger to all 8 cells, (d) Phase-7 row re-layout. The gates assert the proven fontScale-1.0 band until you rule; nothing was carved out silently.

Commit-plan deviation

Planned commit 3 (bodyweight branch) had no code left — SetColumnHeader's isWeighted branch was the single implementation point — so it shipped as the RU ПОВТОРЕНИЙ fixture alone.

🤖 Generated with Claude Code

https://claude.ai/code/session_018TGniHCT2ZMpUw1fy2ku2i

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9d28e35421

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

[github-actions]

Unit Test Results

2 039 tests   2 039 ✅  11m 17s ⏱️
  234 suites      0 💤
  234 files        0 ❌

Results for commit cd6427d.

♻️ This comment has been updated with latest results.

[stslex]

Rulings R9–R12 landed as commit 88a0fe2:

• R9 — compact-inset lever: AppNumberInput drops Space.md→Space.sm under a measured 105dp field width (boundary between measured populations: in-app reps fields 92.15/103.06dp fire; the ten-set card's 106.36dp and PlanSetCard's 109.1dp don't). The 64-cell matrix re-ran: 57/64 pass; the lever rescued live reps×3@2.0.
• R10/R11 — the gates now assert the FULL matrix with the spec §7 ledger inverted (a ledgered cell that starts fitting fails). Seven entries: five reps×5-glyph cells tagged resolved by domain cap, follow-up PR (missing bound registered as blocker B-8, with InputHandler.kt:30 / PlanDraftReducer.kt:40 refs), two weight×5@2.0 cells on the 19sp contrast floor (+42px live, +6px past). No R11 genuine-conflict cell remains — every hard-band red is a five-digit rep count.
• R12 — no sub-19sp rung; floor unchanged.

Gate 2 contact sheet (old | new | diff, all 36, final branch state): https://claude.ai/code/artifact/0b875cc4-bf60-406c-b643-c8a0f6a8a8b9
Mover set re-verified identical to the approved 36 after the lever (record-diff across all three snapshot modules; plan-editor and the four new fixtures byte-identical). Per the Gate-2 condition, the re-record lands only after your review of the sheet — on your word it is one record run + commit, nothing else changes.

🤖 Generated with Claude Code

[stslex]

Rulings R13–R16 landed as commit 390cee3:

• R13 — AppNumberInput.fieldInset parameter replaces the 105dp threshold; set rows pass SetRowGeometry.compactFieldInset uniformly and the header's label inset reads the same source — which also closed a real 4dp label/value drift the threshold had introduced. Matrix re-ran: 58/64; past weight×5@2.0 left the ledger (its +6px deficit met +22px of freed budget — the inverted assertion caught the resolution and demanded the update, exactly as designed); live weight×5@2.0 improved to +20px.
• R14 — SetColumnAlignmentGateTest (layoutlib): header gutter == row index column, asserted from the rendered tree at 1 and 10 sets via two flashAlphaOverride-style size probes. Known-negative proven: hardcoded 12dp gutter reds at 10 sets (33px vs 40px). Honest finding en route: the first cut used Robolectric and passed vacuously — that stack lays a 3-digit mono index under the 12dp minimum and cannot reproduce the drift; it was deleted, and a growth precondition now makes any such stack fail loudly.
• R15 — the five reps×5 ledger cells are restated as DEBT (spec §7): red in production until B-8 ships, void the moment it does.
• R16 — kept.

Contact sheet regenerated from the final state (same URL): https://claude.ai/code/artifact/0b875cc4-bf60-406c-b643-c8a0f6a8a8b9 — the 36 approved movers plus an appendix with the three PR-local first-recordings R13 amended in-commit (setTwoDigitReps, exerciseTenSets, exerciseBodyweightRu — the parameter removed the width-conditionality that had kept golden frames on insets production never rendered). Baseline movers re-verified at exactly 36; plan-editor byte-identical.

Holding the re-record until Ilya clears the sheet. On clearance it is one record run over the two feature modules + one commit containing exactly the 36 re-recorded PNGs.

🤖 Generated with Claude Code

[stslex]

Rulings R17–R20 landed as commit c46110a:

• R17 — the alignment gate now asserts the actual LEFT EDGES: header label vs its column's value slot, captured from the rendered layoutlib tree via two onGloballyPositioned probes, at 1 and 10 sets. Measured green: label==field to the pixel (88.0/414.0px at 1 set, 95.0/417.0px at 10 — both sides moving together with the grown gutter). Known-negative proven: a divergent header inset reds all four edge asserts (11px drift) while every gutter assert stays green — precisely the class the width-only gate was blind to, as the ruling stated. The gutter assert is kept as the necessary-not-sufficient culprit-namer; the golden is no longer cited as an alignment guarantee.
• R18 — spec corrected: the Robolectric result is an instrument defect (laid a 3-digit mono.meta index 10.5dp under the 12.5sp × 0.6em arithmetic that layoutlib confirms — it did not render Plex Mono), recorded as the second confirmed Robolectric false-negative of this PR; the drift scenario is reachable in production and is exactly what the gate defends.
• R19 — the two detekt reds during this round, disclosed: (1) NoUnusedImports on AppNumberInput.kt:32 — a leftover androidx.compose.ui.unit.dp import after the 105dp constant was deleted; fixed by removing the import. (2) ImportOrdering on the new SetColumnAlignmentGateTest.kt:4 — kotlin.math.abs mis-sorted; fixed by reordering. Zero suppressions added, proven: git diff dev | rg '^\+.*[Ss]uppress' → no matches; no detekt config/baseline files touched.
• R20 — the baseline-vs-PR-local distinction is written into spec §7a; the three amended fixtures stay in the contact sheet's appendix.

Probes are pixel-inert, proven by record-diff: the mover set is unchanged at exactly the approved 36; plan-editor and the fixtures byte-identical — the contact sheet Ilya is reviewing remains valid as-is (same URL, no regeneration needed). Still holding the re-record for his clearance.

🤖 Generated with Claude Code

[stslex]

R21 — D6 proven both ways. Known-negative run: stripping the contentDescription assignment from AppNumberInput reds LiveSetRowSemanticsTest on both fields —

Failed: assertExists. Expected exactly '1' node but could not find any node that satisfies:
  (ContentDescription = 'weight, kilograms')
Failed to assert count of nodes. Expected '2' nodes but could not find any node that satisfies:
  (ContentDescription = 'repetitions')

Mutation reverted, test green again, tree clean. (Semantics-tree assertions read composition data, not font metrics — the class of Robolectric defect this PR hit twice does not apply here, and the known-negative now proves it empirically rather than by argument.)

R22 — probe cost, stated plainly: the position probes are attached UNCONDITIONALLY with null-checked callbacks. Inventory of what rides production:

probe	attachment	production cost when null
valueSlotProbe (width+style)	composition-time ?.invoke only, no modifier node	one null check per field composition — effectively free
valueLeftProbe (AppNumberInput)	unconditional onGloballyPositioned node — on ALL 8 call sites incl. PlanSetCard and past rows	callback dispatched after every layout pass AND on every global position change — including every frame of a scroll: a lambda + null check per visible field per scroll frame (~12/frame with 6 weighted rows visible)
labelLeftProbe (header cells)	unconditional onGloballyPositioned ×2 per weighted card	same per-frame dispatch class, 2 nodes per expanded card
indexGutterProbe / indexColumnProbe	unconditional onSizeChanged	fires only on actual size change — not per-frame, negligible
header label onTextLayout	Text always receives a non-null wrapper lambda	once per text relayout, negligible

So: pixel-inert, yes; free, no. The onGloballyPositioned nodes put ~14 null-check dispatches per scroll frame on the session screen's hot path plus one extra Modifier.Node per element in composition. Almost certainly unobservable against a frame budget, but it is real work in production for a test-only capture. Not optimised, per the ruling — the available shape if you want it is conditional attachment (.then(if (probe != null) Modifier.onGloballyPositioned(...) else Modifier)), which removes every per-frame dispatch from production at the cost of a composition-time branch; the flashAlphaOverride precedent carries no such per-frame cost, so these two probes are a genuinely new cost class for the seam pattern. Your call.

🤖 Generated with Claude Code

[stslex]

Ruling R23 landed as commits ab516a5 + 5f18346 — step 1 taken, no fallback needed:

• Layoutlib serves semantics bounds through public API — the same access path Paparazzi's own accessibility extension uses (ComposeView.getChildAt(0) as ViewRootForTest → semanticsOwner → getAllSemanticsNodes(false) → boundsInRoot). The alignment gate now renders a caller-owned host view (OverflowGateSdk.renderView, reading during the frame window where the composition is guaranteed live) and addresses the header label by its text and the field by its accessibility label. The onGloballyPositioned position probes are deleted from all three production composables — the entire per-frame cost class from the R22 disclosure is gone; the onSizeChanged gutter/index probes stay (size-change-only, per the same table).
• Re-proven per the ruling: the semantics path returns the probe path's exact numbers (88.0/414.0px at 1 set, 95.0/417.0px at 10); the R17 known-negative reds all four edge asserts (11px) through the new path with gutters green; the record-diff mover set is unchanged at exactly the approved 36, plan-editor and fixtures byte-identical.
• One process disclosure: commit ab516a5 was pushed with a detekt red (ImportOrdering on the gate test — the alias import sorted before kotlin.math.abs) because the verification pipeline's tail masked gradle's exit code. Fixed in 5f18346, zero suppressions; the pipeline lesson — an exit code behind a pipe is not evidence — is on the record.

R24 acknowledged: this closes the technical rounds. Standing state: 11 commits, all gates green (full ledgered matrix, edge+gutter alignment, D6 both ways), zero suppressions, mover set exactly 36, sheet valid at the same URL. Remaining sequence, on Ilya's sheet clearance: one recordPaparazziDebug run over :feature:live-workout + :feature:past-session, one commit with exactly 36 PNGs, merge (his).

🤖 Generated with Claude Code

[stslex]

R25 landed — 6a48eea9 force-pushed (lease against the old tip 5f183461); the import-ordering fix is folded into the commit that shipped it, so no commit on this branch is knowingly red for detekt. Content verified identical before the push (git diff 5f183461 HEAD empty); the old tip survives in reflog and in this PR's timeline.

Record corrected in the description above: the "bisect-green" claim is withdrawn. Each commit is green for unit tests, both overflow gates, the alignment gate, detekt and lint — but verifyPaparazziDebug exits 1 on seven commits (every one from the suffix removal 56370b5c onward), because the render change was deliberately sequenced ahead of the goldens that record it. Genuine bisect-green would require folding the 36 PNGs into the render-moving commit — a full-history rewrite, not the squash that landed. Squash-merge makes the invariant hold on dev by construction; a merge-commit does not. Ilya's call at merge.

R26 battery, re-run clean on the final state with gradle's exit codes captured directly (the earlier tail pipe certified the pipe, not gradle):

step	exit	result
repo-wide testDebugUnitTest	0	1732 tests, 0 failures, 0 errors, 0 skipped
detekt (serial)	0	zero findings (the 8 .kt: log hits are pre-existing build-logic deprecation warnings)
lintDebug, 5 touched modules	0	clean
verifyPaparazziDebug --continue	1, expected	2186 testcases; exactly 36 red in exactly the 3 predicted classes

Nothing previously reported green came back red. Gates proved non-vacuous in the same run: 32 cells each on the two overflow matrices, both alignment samples with real index growth (33→40px) and label/field edges equal to the pixel, 7-width ellipsis sweep, D6 green. And R2's claim now has direct evidence: live-workout 60 committed PNGs / 60 executed .golden. testcases, past-session 30 / 30 — the .gate. packages neither inflate nor deflate liveness, and no assertGoldenLiveness failed anywhere.

Nothing else moves until Ilya clears the contact sheet.

🤖 Generated with Claude Code

[stslex]

Review round 1 — classification (posted before the fixes, per AGENTS.md § "Merge flow")

Two open threads, both from chatgpt-codex-connector, both reviewed at 9d28e354 — four commits behind the current head. Reproduced both.

1. SetRowGeometry.kt:23 — "Remove the historical narrative from this KDoc" → CORRECT-AND-NEW. Fixing, and wider than flagged.
The reviewer is right, and the finding is systemic rather than local: a sweep of the comment text this PR adds found category-3 HISTORY in 7 files, not one — ruling ids, "the first cut", "replaces the old …", accounts of what the code looked like before. That is exactly the reflex AGENTS.md:L98-L108 names ("Writing 'corrected here', 'the first draft' … into a KDoc is the reflex this rule exists to interrupt"). Fixing all 7 in one commit: guards stay at the edit point, derivations collapse to conclusion + spec citation, history is deleted (it already lives in the commit bodies and the spec registries, which is where the policy puts it).

2. AppNumberInput.kt:234 — "Handle values that do not fit the final text rung" → CORRECT-BUT-ALREADY-DECIDED, on two stale premises.
The residual is real and I am not arguing it away: a five-digit rep count still scrolls out of view at font scales 1.3/1.6. Two of the comment's supporting premises are true at 9d28e354 and false at head:

• "both new gates assert only font scale 1.0" — the gates have asserted the full [1.0, 1.3, 1.6, 2.0] matrix since 390cee36; each run prints 32 cells per row type, and the ledgered cells are asserted overflowing, so a cell that starts fitting fails the gate and forces the ledger update.
• "even for three-digit live reps at 2.0" — that cell (+2px) was closed by the explicit compact inset in 390cee36; spec §7's current table has no 3-digit entry. The reviewer read §7 before that revision (the old table survives, clearly marked superseded, as §7b).

The remaining five-digit cells are deferred, not resolved — registered as blocker B-8 in the spec and tagged in both gates' KNOWN_LIMITS. Every candidate in-PR remedy is closed by a constraint already ruled on: shrinking below the 19sp rung breaks the contrast floor the record/pending colours must pay (and a state-gated exception was rejected outright); BasicTextField cannot ellipsise — it scrolls, which is the measured Phase-0 finding that retired the original "ellipsis permitted" wording; and bounding the input is new product behaviour that Ilya ruled belongs in its own PR. So the honest disposition is: the defect is acknowledged in writing, gated so it cannot silently widen, and its fix is scheduled — not patched here with a remedy that trades a clipped value for an illegible one.

Fixes for (1) push next; CI result and thread replies follow.

🤖 Generated with Claude Code

[stslex]

@codex review

[stslex]

@claude review

[claude]

Claude finished @stslex's task in 2m 22s —— View job

---

Review complete

• Gather context (PR description, prior rulings R1–R26, existing codex review threads)
• Read diff origin/dev...HEAD
• Review key new files: SetColumnHeader.kt, SetRowGeometry.kt, AppNumberInput.kt, OverflowGate.kt, gate tests, semantics test
• Spot-check claims in PR description against code
• Post final review

This is a thoroughly-instrumented change, and the two prior chatgpt-codex-connector threads are already correctly resolved (the KDoc-history fix landed in 727c2ef; the B-8 deferral on the five-digit-reps clip is well-argued and consistent with what's in the diff — the gates do assert the full [1.0, 1.3, 1.6, 2.0] matrix in LiveSetRowOverflowGateTest.kt / PastSetRowOverflowGateTest.kt, and the 3-digit@2.0 cell is in fact gone). I focused on what those threads hadn't already covered.

One real, if minor, finding:

SetColumnHeader.kt:52-54 and the PR description both describe the casing step as "locale-aware uppercase()":

internal fun buildSetColumnHeaderLabel(name: String, unit: String?, unitColor: Color): AnnotatedString =
    buildAnnotatedString {
        append(name.uppercase())
        ...
        append("(${unit.uppercase()})")

Kotlin's no-arg String.uppercase() is explicitly locale-invariant — it's implemented as toUpperCase(Locale.ROOT), which is precisely why it was introduced in Kotlin 1.5 (to replace the old locale-sensitive toUpperCase() and dodge the Turkish-İ/ı bug). So the comment's characterization is backwards: this call is locale-independent by design, not locale-aware. Harmless today (only en/ru ship, and Cyrillic case-folding doesn't differ by locale), but if a Turkish locale is ever added, "i".uppercase() yields "I" here, not the locale-correct "İ" — the opposite of what the KDoc promises a future reader. Given how carefully this PR's comments were scrutinized for accuracy in the codex thread, worth a one-word fix: s/locale-aware/locale-invariant/ (or switch to uppercase(Locale.getDefault()) if Turkish support is ever in scope — but invariant is almost certainly the right call for a UI label upper-casing pass, so I'd just fix the wording).

Fix this →

Everything else checked out:

• SetRowGeometry is genuinely the single source for index width, weight flex, chip slot and field inset — LiveSetRow, PastSetEditRow, SetColumnHeader, LiveExerciseCard.SetsColumn and PastExerciseCard.CardBody all read from it rather than carrying copies. AppCheckmarkButton.TOUCH_SIZE → public AppCheckmarkButtonTouchSize is a clean, minimal instance of the same discipline for the trailing gutter.
• SetColumnHeaderTest.ellipsisEatsTheUnitBeforeTheName genuinely exercises the ellipsis-eats-unit-first claim across a width sweep and asserts the interesting regime is non-empty, not just that assertions ran.
• SetColumnAlignmentGateTest matches its own claims: it reads left edges off the semantics tree (text + content-description lookup), not onGloballyPositioned, and the growth precondition (indexPx must grow 1→10 sets) genuinely guards against a vacuous pass.
• OverflowGateSdk's discarding-frame-consumer design is sound: no SnapshotHandler, so it can't be confused with a golden gate, and measureTextWidthPx fails loudly (check(!measured.isNaN())) rather than silently reporting a false pass.
• AppNumberInput.resolveValueStyle is acyclic as documented — slotWidthPx comes from the parent's BoxWithConstraints, so the chosen rung can't feed back into the constraint that produced it.
• The RU/EN string additions match what LiveSetRowSemanticsTest and SetColumnAlignmentGateTest assert against them ("вес, килограммы" / "повторения", "ВЕС (КГ)" / "ПОВТОРЫ"), and PastSetEditRow / LiveSetRow both pass SetRowGeometry.compactFieldInset consistently.
• SetsColumn / CardBody both correctly skip rendering SetColumnHeader when the set list is empty, mirroring each other.

No blocking issues found. The one finding above is a wording nit in a comment, not a functional defect — your call on whether it's worth a follow-up commit or can ride with the rest of the branch's known, disclosed state (R25/R26).

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 727c2efb1a

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

[stslex]

Review round 2 — 2 reviewers, 1 finding, classified and fixed

@codex — "Didn't find any major issues 👍" on 727c2efb. Nothing to answer; both of its round-1 threads stay resolved.

@claude — SetColumnHeader.kt, "locale-aware" is backwards → CORRECT-AND-NEW. AGREED, FIXED in 3a67291f.

Reproduced and confirmed: Kotlin's no-arg String.uppercase() maps through Locale.ROOT — locale-invariant — which is precisely why 1.5 introduced it to replace the locale-sensitive toUpperCase(). The behaviour was always right; the comment described it backwards, which for the next reader is worse than saying nothing.

Fixed in all four sites the branch owns — two KDoc blocks in SetColumnHeader.kt and two sentences in the spec — and rewritten so the invariance reads as the property this component wants, not an incidental detail: a label whose casing shifted with the device locale would let a golden and a user's screen disagree. That is a genuine guard, so it earns its place at the edit point; the reviewer's suggested one-word swap would have left a true-but-inert statement.

On the Turkish half of the note: agreed that uppercase(Locale.getDefault()) is the locale-correct call in the abstract, and equally agreed it is the wrong call here — it would couple rendered output to the JVM default locale and make the goldens non-deterministic. Only en/ru ship, and neither differs from the invariant mapping. If Turkish is ever added, the decision to revisit is the casing strategy, not this call site.

One neighbouring instance, flagged not touched: AppSectionHeader.kt:38 carries the same wrong characterization — "[String.uppercase] is locale-aware and correct for the Cyrillic…" — and :93 repeats the phrase. AppLabel is the pattern this header follows, so the error propagates from there. It is pre-existing and outside this PR's scope, so I have not edited it; say the word and I will either fold the one-word correction in here or open it as a follow-up. @stslex's call.

Also disclosing a mistake of mine in this round: the fix commit initially swept in documentation/metro-cleanup-discovery.md, an untracked file that predates this branch and is unrelated to the PR — git add documentation was too broad. Caught on the post-commit file listing, removed with git rm --cached + amend, and force-pushed with lease; the file is untracked again and the PR diff is back to 20 source/doc files + 44 PNGs (36 approved re-records + 8 first-recorded fixtures). No behaviour, no pixels, nothing else touched.

Verification, exit codes read directly (no pipes): kit + live-workout tests 0, detekt 0 with zero findings and zero suppressions, verifyPaparazziDebug over the full corpus 0. The change is comment/doc text only — verified mechanically that no non-comment line differs.

Re-requesting both reviewers on the new head once CI reports.

🤖 Generated with Claude Code

---

Correction to this comment: it first cited 6fdf09e3, which was the pre-amend sha of the same change. Removing the stray file with git rm --cached + --amend rewrote it, so the commit that actually carries the fix on the branch is 3a67291f. Cited sha updated above; nothing else about the change differs.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 3a67291f2e

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

[stslex]

@codex review

[stslex]

@claude review

[claude]

Claude finished @stslex's task in 52s —— View job

---

Review complete

• Gather context (PR description, prior rulings, existing review threads, my prior round-1 review)
• Check what changed since my last review (3a67291f, already cleared by codex)
• Read current diff origin/dev...HEAD
• Spot-check anything new against prior rulings
• Post final review

Result

HEAD is still 3a67291f — identical to what I reviewed in round 1 and what @chatgpt-codex-connector cleared twice (727c2efb and 3a67291f, both "no major issues"). No commits landed between my last review and this trigger, so there is no new code to evaluate.

Confirming the round-1 finding was fixed correctly. My earlier note was that SetColumnHeader.kt's KDoc mischaracterized String.uppercase() as "locale-aware" when the no-arg overload is actually locale-invariant (Locale.ROOT). Commit 3a67291f fixes this properly, not just cosmetically:

• Both KDoc sites (SetColumnHeader and buildSetColumnHeaderLabel) now correctly state the invariance, and — better than my suggested one-word swap — frame it as the property the component wants: a locale-shifting label would let a golden and a user's screen disagree. That's a real guard worth stating, not an inert fact.
• The spec (set-field-column-headers.md) got the matching correction in both places.
• Diff is comment/doc-only (verified: only comment lines change in the hunks above), consistent with the author's claim that tests/detekt/goldens all still exit 0.
• The reasonable-to-defer neighboring instance in AppSectionHeader.kt:38,93 (same wrong "locale-aware" phrasing, inherited by SetColumnHeader from that pattern) was correctly left out of scope and flagged for a follow-up call rather than silently ignored or silently expanded into.

No new findings. Everything else stands from the round-1 review: SetRowGeometry as the single geometry source, the overflow-gate design (discarding frame consumer, no golden pollution), the B-8 deferral reasoning for the five-digit-reps clip (contrast floor + BasicTextField can't ellipsize + input-bounding is a separate product decision), the semantics test, and the alignment gate's edge-based (not width-based) assertions all check out as previously described.

Nothing blocking. The two open items remaining are process/product decisions for @stslex, not code defects: the R4 band ruling (spec §7, five-digit reps debt/B-8) and the Gate 2 contact-sheet clearance for the 36-golden re-record.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3a67291f2e

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

[stslex]

@codex review

[stslex]

@claude review

[claude]

Claude finished @stslex's task in 1m 47s —— View job

---

Review complete

• Gather context (PR description, prior rulings R1–R26, existing review threads, my two prior reviews)
• Check what changed since my last review (3a67291f → current HEAD cd6427d2)
• Read the diff for the new commit
• Verify the codex P2 finding (SetRowGeometry.kt:44, trailing tag width) and its fix
• Spot-check anything else new against prior rulings
• Post final review

Result

One new commit since my last review: cd6427d2, addressing @chatgpt-codex-connector's P2 finding on SetRowGeometry.kt:44 (trailing tag width uses the shared 34dp minimum, but PersonalRecordTag's label outgrows it at fontScale 2.0).

The fix is correct and closes the loop properly:

• PersonalRecordTag.personalRecordTagIntrinsicWidth() measures the tag's own label at its own style (prTagTextStyle(), extracted for reuse) plus AppDimension.Space.xs * 2 — matching the Box's actual padding(horizontal = Space.xs) exactly (xs applied to each side = 2×xs total). Measured beside the label/style it describes, so the two can't drift, consistent with this PR's established pattern (resolveIndexColumnWidth does the same on the leading side).
• SetRowGeometry.resolveTrailingSlotWidth() = max(setTypeSlotWidth, personalRecordTagIntrinsicWidth()) is now the single call site used by all four consumers I checked: SetColumnHeader's callers in LiveExerciseCard.kt:422 and PastExerciseCard.kt:255, plus LiveSetRow.kt:184 and PastSetEditRow.kt:147 directly. All four resolve independently but from the same measured source, so header and rows can't disagree.
• Both LiveSetRow and PastSetEditRow now pin the trailing slot with an explicit .width(trailingSlotWidth) on both branches (chip and tag), where previously only a widthIn(min = CHIP_MIN_WIDTH) bounded the tag — so the tag can no longer render wider than what the row/header actually reserved for it, and can no longer render narrower either (both would misalign the fields after it).
• The extended gate (SetColumnAlignmentGateTest) adds exactly the two axes the old fixtures were blind to — font scale and "which trailing component" — and the new Case data class correctly parameterizes isRecord. The growth-precondition check (tenSet.indexPx > oneSet.indexPx) was updated to filter by fontScale so it still guards against a vacuous pass now that there are 4 cases instead of 2.
• Necessary imports (Alignment) were already present in both row files, so no import-hygiene issue from the contentAlignment = Alignment.Center additions.

Minor, non-blocking observation: resolveTrailingSlotWidth() calls personalRecordTagIntrinsicWidth() unconditionally on every row (even work/non-record rows that render the plain type chip), so every row measures the "PR" label every recomposition regardless of whether it's shown. That's consistent with this PR's existing measured-geometry pattern elsewhere (e.g. resolveIndexColumnWidth per row), so I'm not flagging it as a defect — just noting it as the same class of "measurement over caching" tradeoff already accepted in this branch, not a regression this commit introduces.

No blocking issues. This closes out the second and last open reviewer finding on the branch (the codex history-KDoc finding and the "locale-aware" wording finding were already resolved in earlier rounds). The two remaining items are the process/product decisions already on record for @stslex: the R4 band ruling (spec §7, five-digit-reps DEBT/B-8) and Gate 2 contact-sheet clearance for the 36-golden re-record.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: cd6427d230

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