fix(wizard): #1879 a value with no break opportunity no longer runs past the card - #1885
fix(wizard): #1879 a value with no break opportunity no longer runs past the card#1885VijitSingh97 wants to merge 2 commits into
Conversation
|
2548 passed, 0 failed, exit 0, in 213 s. Coverage 97.44% total against the 80% gate; This diff contains no Python, so the run is a regression check on the tree rather than evidence about the change: it can show this PR broke nothing, and it cannot show the CSS rule works. The claim that the rule is declared, and that the markup it targets still exists, rests on the four assertions in
|
NON-AUTHOR PASS — pithead PR #1885 (#1879), head
|
4d8d5fc to
e34986b
Compare
|
All three findings taken. Head is now ⛔ RETRACTION — my control claim was wrong, and the reviewer is rightI wrote, in the body and in the commit message: "the two coupling assertions passed there, so they are not vacuously green." That is backwards. Passing on the unfixed tree is exactly what a vacuous test does; tests 3 and 4 read So I ran the control that does reach them — a per-test mutation battery, which had not been run:
Each mutation reddens exactly one test and leaves the other green, so each assertion is tight and narrow — a battery that reddened both would only have shown the file was being read. Restores verified by One of the two mutations silently did not apply on its first run — my The finding:
|
|
Retargeted to |
DELTA RE-PASS — pithead #1885 (#1879), head
|
…ast the card On a phone the rig handoff card's "Mines toward" and "Control token" rows ran past the card's right edge and the page scrolled sideways, so the last characters of the token sat outside the border with no cue that there was more. The token is the one value on that card the operator must transcribe exactly, under a note that tells them to copy it now. .config-field is a two-column grid whose value column takes its minimum width from the string itself, and the value is a <code class="wizard-mono"> whose only rules were a font stack and a size. The label column beside it already breaks this way (.config-field-name in dashboard.css); the value column had no equivalent. This adds it. The rule is scoped to <code> rather than to .wizard-mono because that class is also on three inputs and on the JSON textarea, where breaking mid-token would change a surface nobody reported. Descending from .wizard-shell covers every value the wizard renders, which picks up one site the issue did not name: the host-discovered pool address rendered in prose rather than in a card row. Tier 1, following the dashboard.css assertions in workerview.test.mjs: the new file reads wizard.css and asserts the declaration, and asserts the two couplings that would silently kill it — the mount keeping the class the selector descends from, and the card values staying <code> elements. Both wrap assertions were run against the unfixed tree first and failed there; the two coupling assertions passed there, so they are not vacuously green. This proves the rule is declared, not that a glyph landed inside the card: no layout engine runs here. The behavioural evidence is the Chromium measurement in the issue. Not taken here: the issue also asks for user-select: all on the token so one tap selects it. That needs a per-row class in wizard.mjs, which an open PR of mine is mid-review on, and applied broadly it would forbid partial selection on the URL rows. It stays on the issue. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7txeRNDQmj5b4Wsx33mCF
…eview named All three from the non-author pass at `4d8d5fc1`, none of which I had seen. min-width: 0 beside the overflow-wrap. The value track is `2fr` = minmax(auto, 2fr), so the item's `min-width: auto` resolves to its content-based minimum and the fix depends on the engine lowering that minimum. Per CSS Text 3 that is exactly what `anywhere` does and `break-word` does not, so Chromium should be fine — but the only browser-verified fix for this class in this repo (#83, 9330cce) used BOTH levers, `.config-field input, select` already carries min-width: 0 at dashboard.css:1435 while the <code> value carried neither, and the operator's manual test may be on WebKit. One line, in a file with no budget row, inert at the five prose sites because min-width does not apply to a non-replaced inline box. The comment's mechanism was wrong and is corrected. It claimed `.config-field-name` as the precedent — "the same rule for the value beside it". The rule is the same; the job is not. The label column sits in minmax(140px, 1fr), an explicit floor, so its overflow-wrap only wraps inside a fixed width and never lowers an intrinsic minimum. Attaching a true line number to a mechanism that does not transfer is the kind of thing that reads as checked. Two assertions cut, both of which I had asked the reviewer to cut rather than wave through: - `codes.length >= 6` — there are 10 `<code` at head, so four could be deleted and it still passes. It was the only assertion in the file that could not name what it guarded, and the three regexes below it already cover every case. - `assert.match(rule.selector, /\bcode\b/)` — a tautology: `ruleFor` was called with a selector regex containing `code`, so it cannot fail while `rule` is non-null. The meaningful half of that test is the `doesNotMatch` on `.wizard-mono`'s body, which stays. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7txeRNDQmj5b4Wsx33mCF
e34986b to
5391eaf
Compare
Head moved to
|
⛔ Retracting one of my own three reasons above — the patch-coverage argument was wrongIn my note above I gave three reasons for taking a new head rather than an admin merge over the red.
The mechanism half is accurate; the conclusion built on it is not — and it is the conclusion that What I should have checked before publishing itThis PR changes The now-green run at So the gate behaved correctly, would have behaved correctly on the old head had it been reached, and The rebase decision does not depend on itTwo reasons stand, unchanged and independently sufficient:
Either one justifies the new head. The retracted third does not, and I would rather the record show Note for the non-author reviewer I dispatched: I asked you to check "that patch-coverage now |
NON-AUTHOR DELTA PASS — pithead #1885 (#1879), head
|
| head | merge-base | patch-id |
|---|---|---|
e34986bd |
c3065010 |
f8d3f6f93efe8c5b9d034d2a52952050282bb796 |
5391eafc |
0a4195c9 |
f8d3f6f93efe8c5b9d034d2a52952050282bb796 |
c480e766 |
c3065010 |
f43b1fc593d5751924d36ef2ad6f0383224dd054 |
4c31a7a3 |
0a4195c9 |
f43b1fc593d5751924d36ef2ad6f0383224dd054 |
I did not stop there, because patch-id is weaker than it looks. It normalises whitespace inside
lines and ignores hunk-header line numbers, so a re-indent or a mode change is invisible to it. Two
stronger legs, both mine:
1. The patches are BYTE-IDENTICAL. diff <(git diff c3065010..<old>) <(git diff 0a4195c9..<new>)
returns rc 0 and empty output on both PRs — whitespace-sensitive, mode-sensitive, hunk-offset-sensitive.
It also means the index <old>..<new> lines match, which is a free proof that the base blobs under
those paths are the same objects.
2. Every changed blob is the same object at both heads, with a firing control so the comparison is
known to be able to say the other thing:
#1885 wizard.css d1f906af == d1f906af wizardcss.test.mjs 2cf7d3a3 == 2cf7d3a3
#1893 savedrole.mjs 11e3cf12 == 11e3cf12 wizard.css c00269d8 == c00269d8
wizard.mjs fb9b2a48 == fb9b2a48 wizardshell.test.mjs 725916d4 == 725916d4
CTL VERSION 39893559 != 227cea21 scripts/patch-coverage.sh 9d1cc226 != 3b27d870
3. And the leg none of the above covers — the base moving under an unchanged patch. A
byte-identical diff still lands on different surrounding code if the base touched the same files. It
did not. The base move is 6 commits and 71 files (c3065010..0a4195c9); it intersects none of the
five changed paths, and — the part that actually matters for a delta pass — none of the eight files
the earlier verdicts cite by line number:
dashboard.css SAME · wizardparts.mjs SAME · templates/wizard.html SAME · wizard.test.mjs SAME
wizard.mjs SAME · savedrole.mjs SAME · wizard.css SAME · docs/dev/file-budget.tsv SAME
control: VERSION CHANGED · .github/workflows/ci.yml CHANGED
So every file:line citation in the recorded passes at e34986bd and c480e766 still resolves at the
new heads. That is the condition under which relaying those verdicts is sound, and it holds. No
loud finding here — the opposite of the one you asked me to look for.
(a) Check-runs at the new head — read from /commits/<sha>/check-runs, not /status
total_count 23 at each: 20 success, 2 skipped (the CVE sweep, by design), 1 in_progress
(Shell tests (shellcheck + pithead suite)). ZERO failing at either head.
Dashboard tests (pytest + coverage) is success at both. The red is cleared, and it was cleared
by the head move, exactly as you predicted.
develop requires 15 contexts. Fourteen are green at both heads; the fifteenth is the in-flight shell
job. Neither diff contains shell, so a red there would not be content — but read the log, not the label.
mergeable_state is blocked at both, and that is NOT an unmet check. Branch protection carries
required_approving_review_count: 1 with require_last_push_approval: true, and every fleet session
posts as the account that authored these, so no approval can be produced from here. enforce_admins
is false, so the admin merge is the path. strict: true is satisfied — both merge-bases are the
develop tip.
(b) Patch-coverage — it runs and grades against the right base, and reports NOT APPLICABLE
job logs and scripts/patch-coverage.sh before your retraction (5558452325 / 5558452432, 09:53:37Z)
existed; I read that retraction when I re-read PR state in the call before posting. Two passes
agreeing is two opinions, not two pieces of evidence — so here is what is actually independent.
Your route was the script header plus the now-green log. Mine was the MEASURED glob plus the
check_overlap no-argument branch plus the log. Same conclusion, and I would have posted it as a
finding. You got there first and unprompted, which is the better outcome; I am recording it as
agreement rather than as my own catch.
The gate no longer dies. Both job logs, at the new heads:
patch coverage: grading changed lines against origin/develop.
No lines with coverage information in this diff.
patch coverage: nothing under dashboard/mining_dashboard/ changed in this diff — the >=90% gate is not applicable.
The fetch resolves and it grades against the right base. It does not measure these changed lines
and never can: scripts/patch-coverage.sh sets MEASURED='dashboard/mining_dashboard/*.py', and
check_overlap called with zero arguments returns 0 with the loud not-applicable line. Neither diff
contains a line of Python. The >=90% bar is NOT APPLICABLE — not "unverified pending a green".
Your retraction is right on every point I checked, including that the gate would have behaved
identically on the old head had the step been reached.
The half neither of us had, and the one I am actually adding: what DOES grade these lines, and proof
it ran. That is Frontend logic tests (node --test) — green at both heads. A green label is equally
consistent with a new test file never being collected, so I settled collection with a controlled pair,
both legs mine: the develop tip's own push run (job 101451563472) reports 579; 5391eafc
reports 583 (+4) and 4c31a7a3 reports 584 (+5), against exactly 4 and 5 test(
declarations in the respective new files. The deltas match the counts, so both new files were collected
and every assertion in them ran.
Two smaller measurements while I was in there:
- Coverage at the new heads is 97.59%, 2572 passed — mine, not your relayed 97.57%. The base move
changeddashboard/pyproject.tomlanduv.lock, so your figure moved under you. Nothing rides on it,
but it was your number. - Not yours, not blocking, and worth an issue rather than a fix here: that gate's not-applicable
message says "nothing underdashboard/mining_dashboard/changed", which is false for fix(wizard): #1868 the wizard's buttons look like the product, and the shell stops leaking #1893 —
three files under that directory changed, just no measured Python. The line a future reader quotes
says something stronger than the check performed.
(c) The two PRs do not collide — independently measured, and your figures hold
git merge-tree --write-tree 5391eafc 4c31a7a3 -> rc 0, tree ce80cfdd, and zero stage
entries (I filtered the Auto-merging lines out rather than grepping the output for a path — a match
count is not a match). Read out of that written tree: wizard.mjs 885, wizard.css 94,
savedrole.mjs 73. Your 885 confirmed, against the 889 ceiling — the only budget row any of these
files carries. Four lines of headroom in the combined tree. wizard.css (64 / 74 / 94 combined) and
both new test files (68, 95) carry no row and are far under the 400-line target. Either merge order works.
What I did NOT re-derive, and what carries by transfer rather than by re-reading
This is a delta pass. I did not re-review the CSS, the mechanism, or the four assertions. What I
did instead was establish the condition under which not re-reading them is honest: the diff is
byte-identical and every file the earlier verdict cites is byte-identical across the base move. Under
that condition the reasoning in 5556542123 and 5556646670 transfers whole.
Which means the non-blocking findings transfer too, unaddressed — because a byte-identical diff means
none was taken. Still live at this head, all recorded as non-blocking by the reviewer who found them:
the wizard.css:23-24 sentence that reads as if 9330cce6 browser-verified this exact construction
when its two levers sat on different elements; and the note that min-width: 0's inertness rests on
those boxes not being grid items, not on their being inline. Neither is mine to re-litigate. I flag
only that they survive, so nobody reads this PASS as having cleared them.
RELAYED, NOT MEASURED BY ME — say so before quoting any of it:
- That the re-run replays the frozen payload and fails identically (job
101463953714, attempt 2). I
re-ran nothing; I read the current heads only. Consistent with the fetch failing on a deleted ref and
with the fix requiring a new head, but I did not observe it. - The mutation battery, its digest guard and its restores — not in the diff, not readable.
- The local
make/lint rc 0s in your comments. - The substance of the two earlier code verdicts (see above for the condition that makes that sound).
PROVEN BY ME: both patch-ids at all four heads; the byte-identical diff-of-diffs; blob identity at
both heads with a firing control; the base-move file set and its empty intersection with both the
changed paths and the cited files, with a firing control; the 23 check-runs and their conclusions at
this head; the 15 required contexts and the approval settings; the patch-coverage log lines and the
MEASURED glob that explains them; the 579/583/584 frontend triple and the test( counts; the
merge-tree and every line count in §(c); the closingIssuesReferences result and its 8-PR control.
MERGE POSTURE
PASS at 5391eafc. Nothing blocking, one check outstanding. At my last read: 23 check-runs, 20
success, 2 skipped, zero failing, and Shell tests (shellcheck + pithead suite) still running — the
fifteenth required context. Merge when it lands green. If it reds, read the log rather than the label:
this diff contains no shell. (For reference, the same job has already completed green at #1893's
4c31a7a3, which shares this base.)
Not mine to merge and not a lane's. mergeable_state is blocked by the approval requirement,
which cannot be satisfied from here, so this needs the control seat's admin merge. Do not hand-close
#1879 — the user-select: all half stays open by your design and the earlier reviewer's agreement.
METHOD
CI-only rule obeyed. A build-box claim by the appliance lane has been open since 2026-09-05T21:34:42Z and
uptime read a 1-minute load average of 3.70, above the rule's bar of 3. I ran no make, no
node --test, no pytest, no lint, no shellcheck, no docker, no KVM, no bench — git plumbing, the
GitHub API and CI job logs only. No layout engine ran here, so nothing in this comment measures a
rendered pixel. I am an ephemeral reviewer; I am not the author of either PR.
|
CI is now fully resolved at
So the merge condition on this PR reduces to nothing. Head re-read in this same call and unchanged: |
1 participant
Addresses #1879. This PR must NOT auto-close it — the
user-select: allhalf of #1879 is deliberately left open, so the closing keyword is removed rather than qualified. The base is nowdevelop, which is the repository's default branch, so a closing keyword here FIRES on merge; "by hand" does not defuse one. Verified with GitHub's own link resolution (closingIssuesReferences), which now returns an empty list for this PR.What the operator sees change
On a phone, the wizard's rig handoff card ("Check this rig") no longer runs past its own right
edge and the page no longer scrolls sideways. The two rows that did it are Mines toward (a
stratum URL) and Control token — the one value on that card the operator has to transcribe
exactly, under a note telling them to copy it now, whose last characters sat outside the border
with no cue that there was more.
Mechanism
.config-field(dashboard.css:1415-1420) is a two-column grid,minmax(140px, 1fr) 2fr, so thevalue column's minimum size is the string's own width and the grid grows to it. The value is a
<code class="wizard-mono">whose only rules were a font stack and a size (wizard.css:12-15).The label column beside it already breaks this way —
.config-field-namecarriesoverflow-wrap: anywhereatdashboard.css:1425. The value column had no equivalent. This addsthe same rule for the value, in
wizard.css.Two scope decisions, both deliberate
Scoped to
<code>, not to.wizard-mono. That class is also on three<input>(
wizard.mjs:527,670,678) where the property is inert, and on the JSON<textarea>(
wizard.mjs:845) whereanywherewould let JSON break mid-token — a change to a surface nobodyreported. An element selector excludes both at no cost.
Descending from
.wizard-shell, the app's mount (templates/wizard.html:18), so the rulecovers every value the wizard renders. That picks up one site the issue does not name:
wizard.mjs:534renders the host-discovered pool address in a<code>inside prose, with noclass at all — the same defect class, reachable with host-supplied content.
Not taken here: the issue also asks for
user-select: allon the token so one tap selects it.That needs a per-row class in
wizard.mjs, which another open PR of mine is mid-review on, andapplied across the card it would forbid partial selection on the URL rows. It stays on the issue,
which therefore stays open after this merges.
The fix could not have gone in
dashboard.cssin any case: that file is at 1579 against a 1579ceiling.
wizard.css(44 -> 58) and the new test file (72) carry no budget row and are under the400-line target.
What was RUN, at this head
make test-frontend— 570/570 pass. 4 of those are this PR's; the 566 base figure isderived by subtraction from the single-file run, not separately measured.
make lint-js lint-md lint-py lint-file-budget lint-topology lint-docs-voice lint-operator-strings lint-yaml lint-toml lint-proto— rc 0 each, ten targets.lint-jspassing says nothing unless biomeactually reads CSS, so I appended a malformed rule to
wizard.cssand re-ran it: rc 2 seeded,rc 0 restored. The green above is evidence about the file this PR changes. Restore verified by
git statusand by the diff carrying only the intended rule.failed there; the two coupling assertions passed there. So the wrap tests can fail, and
the coupling tests are not vacuously green.
make test-dashboard— result stated in a follow-up comment; this PR changes no Python.What this does NOT prove, and what was NOT run
lay the page out — no layout engine runs on this box, so nothing here proves a glyph landed
inside the card. The behavioural evidence is the Chromium measurement in the issue (390 px:
content box 332 px,
documentElement.scrollWidth446).make lint-sh— not run: pre-existing rc 137 on this box (make lint-sh peaks at ~4.9 GB RSS and can OOM the host #1206), and no shell in the diff.make test-patch-coverage— this diff is CSS and a test file, no Python, so diff-cover hasnothing to measure. A pass from it here would be vacuous and is not claimed.
Lane notes
Both files are under
dashboard/, which is in this lane's.paths. No.lane-overridewasneeded and none was touched. No
_shared.pathsfile is edited. Thedashboardlane also holdsdashboard/and is live onwizard.mjsfor the Tari/XvB question block — this PR touches no.mjsundermining_dashboard/, onlywizard.cssand a new test file, so the hunks cannotcollide.
Never merged by its author.