fix(admin): measure the panel in columns, not code units (#401) - #416
Merged
Merged
Conversation
Every width promise in the admin panel was a UTF-16 code-unit count. `clip`, `pad`, `styler.cell`, `divider`, `clipPlain`, `visibleLen` and both frame builders' top rules all sized themselves with `.length`, so a framed pane this module reported as exactly 80 columns came out at 90 in the terminal for a CJK job id, and ragged the other way for a combining mark. Measured against pi-tui's own `visibleWidth` at the pin, as this module counted it against what a terminal draws: a CJK job id 5 against 10, fullwidth latin 3 against 6, Hangul 5 against 10, combining marks 5 against 3. ONE TABLE, IN `panel.mjs`, AND THE STYLER COMES TO IT. Importing pi-tui's `visibleWidth` into `style.mjs` was the obvious alternative, and that module is overlay-only and already depends on pi. It was rejected: `panel.mjs` owns `clip` and the monochrome renderer, it is pinned to have no imports, and the two renderers draw the same geometry. A width rule that holds in the framed pane and not in the plain one is the shape that holds on one branch of an `if`, which issue #403 had just been about. That shape then turned up inside this fix. With only `frame`'s top rule pinned, a mutation reverting `box`'s top rule to `.length` SURVIVED the whole suite, so the monochrome pane is pinned too and both rules are mutation-checked. What the table counts, each line the smallest thing that gets the measured cases right: a combining mark (Mn/Me) is 0, a zero-width or formatting character is 0, an East Asian Wide or Fullwidth character is 2, everything else is 1 including an unassigned code point, because guessing wider on an unknown is how such a rule starts breaking the panes it was added to fix. Mc is deliberately not zero: a spacing mark does occupy a column. Its limit, pinned as a disagreement rather than left to surprise a reader: the table sums CODE POINTS, so an emoji ZWJ sequence counts every member where pi collapses it to one glyph, 6 against 2. Terminals disagree with each other there too. Over-counting cuts early, so the residual is a short line, never a broken border. The oracle is pi itself, loaded from the pinned installation per CONST-PI-VERSION-PINNED, and asserted to be a function rather than skipped on failure: a sibling pin made exactly that silent-fallback mistake once, and an oracle that quietly vanishes stops being one. Also corrected here, because both were the same false claim in prose: two comments still said that post-strip `.length` is a safe column proxy because the content is ASCII. It is not. A repository name, a job id and a branch name carry whatever the forge accepts. `meter`'s label keeps `.length` and now says why: it is two integers and a word from a closed set, so no caller can put anything else in it. Ten mutations checked, every one red: `columnsOf` back to `.length`; a wide character counting as one; a combining mark counting as a column; a zero-width character counting as a column; `sliceColumns` cutting after the budget rather than before it; `sliceColumns` counting every character as one; `sliceColumns` slicing by code units; `box`'s top rule by `.length`; `frame`'s top rule by `.length`; `visibleLen` back to a code-unit count; and `pad` filling by `.length`. DES-ADMIN-VIA-PI-EXTENSION AMENDED: the entry's own sentence saying column width for non-ASCII is NOT promised is what changed, and the two other places stating it are corrected with it. DES-PANEL-SEPARATE-FROM-RECEIVER UNCHANGED, checked: nothing about where the panel runs or what it binds moves. The bidi and zero-width residual is UNCHANGED and still open: those code points sit outside a control-byte class defined by what a terminal interprets, and they are a reader-deception question rather than a width one. No version moved. Signed-off-by: Rob Boerman <robboerman@live.nl>
…e standard (#401) The first commit's table was hand-written out of UAX #11 and checked against ten strings. It passed, and it UNDER-counted 139,820 code points: CJK Extension B through H, Tangut, the Kana supplement, the Hangul Jamo extensions and every emoji block added since Unicode 13. CJK Extension B is exactly the repository name this issue is about, so the fix carried the defect it was fixing, and in two classes it was WORSE than the code-unit count it replaced, because an astral character is two code units and two columns and `.length` had been right there by accident. The renderer draws these panes, so the renderer is the authority. `WIDE` is now its own double-width set, transcribed from the pin; the zero rule is `\p{M}` plus `\p{Cf}` plus the fillers; and one piece of state handles U+FE0F, which asks for the emoji form of the character before it and cannot be answered by a per-character table at all. Mc is no longer excepted. The first version reasoned from Unicode that a spacing mark occupies a column. The renderer that draws the pane says otherwise, and between a standard and the thing actually painting the characters, the painter wins. THE BOLT IS A SWEEP, because ten examples cannot see a 139,820-code-point hole. It walks every code point there is, plus every character-with-U+FE0F pair, and requires that the table never measure NARROWER than the renderer and that every place it measures wider is an unassigned code point. The direction is the whole property, and "over-counting is harmless" would be too kind. Both directions rag a frame, differently. An over-count pads a body line as though it were wider than it is, so the line comes out short and the right border sits left of the one above it, contained by the pane. An under-count runs the line past the pane and past the terminal, where it wraps and takes the border with it. The oracle's VERSION is asserted, not just its presence: pi depends on pi-tui by a range, so a resolve that is not the lockfile's would answer any 0.80.x, and a table pinned to the wrong renderer is not pinned. Repaired with it, as the same defect class rather than as scope creep: - the line editor windowed and padded by code units, so a CJK value measured half what the terminal draws and a window edge put a bare low surrogate into the live trigger editor; - `renderRuns`, the model-visible table behind `/dispatch runs`, sized its columns by `.length`, so one CJK character in a `local:<basename>` target shifted every later column of that row; - a cut to zero returned a floating combining mark, and a cut through a ZWJ sequence left the joiner dangling in front of the ellipsis; - the cut carried a lone surrogate the input already held, where the old slice-then-repair had dropped it; - two hostile-string caps in the graph model could split a surrogate pair. Also corrected: three comments and two spec sentences that overclaimed, the docblock measurement table (its numbers were doubled and named two different strings), a dead `dropLoneSurrogate` import with the stale comment above it, and a clause testing `\p{Emoji_Presentation}` that was dead because every code point with that property is already in the wide set. Twenty-five mutations checked, every one red, including the eleven that survived the first commit: seven ranges of the wide set one code point at a time, both halves of the zero rule, the U+FE0F clause and its ASCII exclusion, the lone-surrogate skip, the dangling-joiner strip, the zero-budget guard, all three `divider` measurements, `clipPlain`, `padVisible`'s strict comparison, both halves of the model-visible table, both halves of the line editor, and the graph cap. Full suite in the CI posture: 4206 tests, 0 fail, 1 skipped. Same under the +399 day clock shift. All four guards pass and a run under a fresh TMPDIR leaves nothing behind. DES-ADMIN-VIA-PI-EXTENSION amended again, and two further live sentences that this change made false are corrected with it: the #403 decision's "the clip is by LENGTH", and the LIST entry's account of what the width now frames. No version moved. Signed-off-by: Rob Boerman <robboerman@live.nl>
A second adversarial pass found two UNDER-counts, which is the direction that
runs a line past its pane, and both were in the repair rather than in the
original.
A KEYCAP measured 1 where the renderer draws 2. The previous commit excluded
ASCII from emoji-form promotion, which is right on its own terms: `#`, `*` and
the digits stay one column under U+FE0F alone. Its comment then named the
keycap as the reason, while the keycap itself went on measuring wrong. It takes
three code points, base plus U+FE0F plus U+20E3, and only twelve bases.
THE COUNT AND THE CUT NOW WALK ONE STEPPER, which is the structural half. The
previous commit put the U+FE0F state inside `columnsOf` alone, and
`sliceColumns` and the line editor call that function ONE CHARACTER AT A TIME,
where a base is 1 and its selector is 0. So the cut spent a budget of 2 on a
glyph drawn 3 wide and the pane overflowed. One generator now yields
`{text, cols}` steps: the count sums them and the cut takes whole ones, so the
two cannot disagree, and a selector can no longer be stranded without its base.
The line editor needed the same fix on its EDIT side, which is the sharper
half. `cursor` moved one code unit at a time, so two `left`s put it between the
halves of an astral pair and the next `backspace` deleted ONE HALF. The
surviving half stayed in `value()`, which is the string that gets saved, where
no amount of rendering can repair it. All four movements now step by character.
Two claims were false and are corrected rather than softened:
- "plus every character-with-U+FE0F pair" appeared in the commit body, the PR,
the docblock and the spec row. The test did no such sweep, it had a six-entry
list. The sweep now exists, over every base with U+FE0F and with a keycap, and
it is what would have caught the keycap defect.
- "its one remaining limit" named the ZWJ sequence alone. A skin-tone modifier,
a regional-indicator flag pair, a Hangul jamo cluster and a Devanagari cluster
over-count too. What is left is a class, not one shape, and every member of it
over-counts, so every one draws short inside its border rather than through
it.
Newly pinned, each having survived the previous commit: the two `divider`
measurements whose revert changes only which of the label and the meta is
clipped first, which total-width assertions cannot see; the line editor's
cursor snap, which no test reached because every fixture built a fresh editor
with the cursor past the end; the skill-frontmatter cap, the second of the two
graph caps; and the sweep's own SCOPE, by asserting how many code points it
walked and how large the renderer's wide set is, because narrowing the loop
bound to the BMP left the suite green.
The two guard-weakening mutations that remain unkillable are stated in the test
rather than chased: a loosened assertion is unobservable while the thing it
guards against is absent. Both are shown live by mutating the source instead.
Thirty-four mutations checked, every one red, with zero survivors.
Full suite in the CI posture: 4211 tests, 0 fail, 1 skipped. Same under the
+399 day clock shift. All four guards pass and a run under a fresh TMPDIR
leaves nothing behind.
DES-ADMIN-VIA-PI-EXTENSION's amendment corrected in the two places it
overclaimed, and the revision row with it.
No version moved.
Signed-off-by: Rob Boerman <robboerman@live.nl>
…s cut (#401) A third adversarial pass found the defect class again, in the function introduced to end it. `widthSteps` emitted a lone surrogate as a step of its own, and both consumers DROPPED it and re-joined what was left. Dropping it splices its neighbours back together: a base and the selector on the other side of it become one emoji-form sequence, so the cut emitted two columns for a budget of one. Measured through the real pane, a 24-column box drew at 43 by this module's own count. That is the failure the previous commit's body describes, one level down. ONE RULE, AT THE TWO DOORS WHERE TEXT ENTERS. A lone surrogate is not a character, so `widthSteps` removes them BEFORE it steps. There is then no orphan step, nothing to drop and nothing to splice, and the count describes what will be DRAWN rather than what arrived, which is the property every consumer actually needs. The line editor gets the same rule at its constructor, `insert` and `setValue`. `backspace` and `del` were fixed to step by character in the previous commit, and this pass pointed out that the three doors admitting text were left open: `stripControls` removes C0 and C1, not half a character, and `insert` takes a whole paste. The argument that made the edit-side fix necessary is that `value()` is the string that gets SAVED, and it applies to those doors unchanged. It also makes the window's arithmetic sound, because `cursor` indexes that string. Newly pinned, having survived the previous commit: the keycap's ADVANCE, which no width assertion can see because the enclosing mark is zero columns, so a step that advances by one instead of two emits U+20E3 again and corrupts the glyph while the count stays right. Corrected, all false when written: two comments above ZERO_WIDTH saying Mc is excluded because a spacing mark occupies a column, where the code includes it and the revision row says the renderer's answer wins; and the cursor comment, which described a snap to the step the cursor falls inside where it rounds forward to the next one. FILED RATHER THAN CHASED, under this round's cap of three review rounds: - #417. The never-narrower guarantee is per code point and per the swept pairs, not for every string. The renderer computes a cluster's base AFTER stripping leading non-printing characters, so a cluster beginning with one of 2,616 zero-width characters and continuing with one of exactly four tails has that tail counted twice. It is an under-count and it amplifies with a repeated breaker. The claim is narrowed to what the sweeps actually assert. - #418. Two HTML views still size an SVG chip and cut their text by code unit, the same class this issue answered everywhere else. Thirty-six mutations checked, zero survivors. Full suite in the CI posture: 4214 tests, 0 fail, 1 skipped. Same under the +399 day clock shift. All four guards pass and a run under a fresh TMPDIR leaves nothing behind. No version moved. Signed-off-by: Rob Boerman <robboerman@live.nl>
…terminal (#401) The final review pass found one more instance of the leading-mark class filed as #417, with a different leader and a one-line containment, so it is fixed here rather than filed. A LONE SURROGATE breaks a cluster for the renderer exactly as one of #417's 2,616 zero-width leaders does: it skips the break, computes the next cluster's base, and counts that base twice. `clip` returned its input unchanged whenever it fitted, so an orphan the COUNT had already removed was still in the TEXT handed to the renderer. Measured: `("\ud800\uff9f").repeat(12)` counted 12 here and 24 there, and a 24-column pane drew at 36, a 40-column one at 52. `clip` steps its fitted line now. That makes the rule this module already states true of the text rather than only of the count: half a character is removed where text ENTERS. The #417 leader set is unchanged, all 2,616 of them zero-width characters. Three issue texts corrected with it, each false at HEAD rather than merely incomplete: - #418 said both HTML views already import from `panel.mjs`. `graph-html.mjs` imports nothing, deliberately, with a comment saying it is allowed no dependencies at all and a parity test standing in for the import. That was the suggested route, so the correction matters. - #402's table said a zero-width space is counted as a column and breaks the width math. It is zero columns now, which is what the renderer draws, so the width argument is answered and only the deception argument is left. - #417 gains two measured notes: the line editor's window can begin on a zero-width character and manufacture the shape out of a well-formed value, and both its shapes are a REGRESSION against main for those inputs, which is worth saying so the fix is not deprioritised as a pre-existing residual. Verified at scale against the pinned renderer by the review pass: 64,000 hostile strings through every cutter at every width 0 to 40, and 1,500 random title/body/footer triples through both panes at twelve widths, with no width failure by either measure; and 15,000 random 24-operation editor sequences with no lone surrogate in the value, no wrong render width and no new class. Full suite in the CI posture: 4215 tests, 0 fail, 1 skipped. Same under the +399 day clock shift. All four guards pass, a run under a fresh TMPDIR leaves nothing behind, and reverting the containment turns the new pin red. No version moved. Signed-off-by: Rob Boerman <robboerman@live.nl>
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
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.
Closes #401.
Every width promise in the admin panel was a UTF-16 code-unit count.
clip,pad,styler.cell,divider,clipPlain,visibleLen, both frame builders' top rules, the line editor's window and themodel-visible runs table all sized themselves with
.length, so a framed pane this module reported asexactly 80 columns came out at 90 in the terminal for a CJK job id, and ragged the other way for a
combining mark.
One table, in
panel.mjs, and the styler comes to itThe issue asked whether the two renderers keep one width rule or deliberately stop sharing one. They keep
one. Importing pi-tui's
visibleWidthintostyle.mjswas the alternative, and that module isoverlay-only and already depends on pi, but
panel.mjsownsclipand the monochrome renderer, itspurity pin forbids it reaching the world at all, and the two renderers draw the same geometry. A width
rule that holds in the framed pane and not in the plain one is the shape that holds on one branch of an
if, which #403 had just been about.That shape then turned up inside this fix. With only
frame's top rule pinned, a mutation revertingbox's top rule to.lengthsurvived the whole suite. Both are pinned now.The correction that matters: the table is transcribed from the renderer
The first version of this table was written out of UAX #11 by hand and checked against ten strings. It
passed, and it under-counted 139,820 code points: CJK Extension B through H, Tangut, the Kana
supplement, the Hangul Jamo extensions and every emoji block added since Unicode 13. CJK Extension B is
exactly the repository name this issue is about, so the fix carried the defect it was fixing. In two
classes (an astral character, and a character followed by U+FE0F) it was worse than the
.lengthitreplaced, which had been right there by accident because such a sequence is two code units and two
columns.
The renderer draws these panes, so the renderer is the authority, not the standard.
WIDEis its owndouble-width set transcribed from the pin; the zero rule is
\p{M}plus\p{Cf}plus the fillers; andone piece of state handles U+FE0F, which asks for the emoji form of the character before it and cannot be
answered by a per-character table at all.
Mc is no longer excepted. The first version reasoned from Unicode that a spacing mark occupies a column.
The renderer says otherwise, and between a standard and the thing actually painting the characters, the
painter wins.
The bolt is two sweeps
Ten examples cannot see a 139,820-code-point hole. The test walks every code point there is, and every
character followed by U+FE0F and by a keycap, requiring that the table never measure narrower than the
renderer and that every place it measures wider is a declared departure.
A second adversarial pass is why the pair sweep is real rather than claimed. The first repair's commit,
docblock and spec row all said "plus every character-with-U+FE0F pair"; the test had a six-entry list, and
the hole was one selector further along. A keycap (twelve bases plus U+FE0F plus U+20E3, drawn as one
key) measured 1 against the renderer's 2. That is an under-count, so it is fixed rather than stated.
That pass also found the structural half. The first repair put the U+FE0F state inside
columnsOfalone,and
sliceColumnsand the line editor call that function one character at a time, where a base is 1 andits selector is 0, so the cut spent a budget of 2 on a glyph drawn 3 wide and the pane overflowed. One
generator now yields
{text, cols}steps: the count sums them, the cut takes whole ones, and they cannotdisagree.
The direction is the whole property, and "over-counting is harmless" would be too kind. Both directions
rag a frame, differently. An over-count pads a body line as though it were wider than it is, so the line
comes out short and the right border sits left of the one above it, contained by the pane. An under-count
runs the line past the pane and past the terminal, where it wraps and takes the border with it.
The oracle's version is asserted, not just its presence: pi depends on pi-tui by a range, so a resolve
that is not the lockfile's would answer any 0.80.x, and a table pinned to the wrong renderer is not
pinned.
What is left is a class, not one shape
The table sums steps, so every run the renderer collapses into one glyph comes out wider than it draws: an
emoji ZWJ sequence (6 against 2), and also a skin-tone modifier, a regional-indicator flag pair, a Hangul
jamo cluster and a Devanagari cluster. Saying the ZWJ sequence was the only one was the other claim the
second pass refuted. All of them over-count, so all of them draw short inside the border rather than
through it, and terminals disagree with each other on all of them. The issue also asked whether a grapheme-aware cut should replace the
UTF-16 slice:
sliceColumnswalks code points, so a cut never lands inside a wide character and neverleaves half a pair, and a grapheme-aware count is what the ZWJ residual would need.
Repaired with it, as the same defect class
and a window edge put a bare low surrogate into the live trigger editor
renderRuns, the model-visible table behind/dispatch runs, sized its columns by.length, so oneCJK character in a
local:<basename>target shifted every later column of that rowdangling in front of the ellipsis
cursormoved one code unit at a time, so twolefts put it between the halves of an astral pair andthe next
backspacedeleted one half, leaving the other invalue(), the string that gets savedAlso corrected: three comments and two live spec sentences that overclaimed, the docblock's measurement
table (its numbers were doubled and named two different strings), a dead import with the stale comment
above it, and a clause that was dead because every code point with that property is already in the wide
set.
Checked
Thirty-four mutations, every one red, zero survivors, including every one that survived an earlier commit:
ranges of the wide set one code point at a time, both halves of the zero rule, the U+FE0F and keycap
clauses, the lone-surrogate skip, the dangling-joiner strip, the zero-budget guard, all four
dividermeasurements,
clipPlain,padVisible's strict comparison, both halves of the model-visible table, theline editor's window, cursor snap and all four edit movements, and both graph caps. The sweep's own scope
is pinned too, by asserting how many code points it walked: narrowing its loop bound to the BMP, which is
where the original hole was, had left the suite green.
Two guard-weakening mutations remain unkillable and are stated in the test rather than chased: loosening an
assertion is unobservable while the thing it guards against is absent. Both are shown live by mutating the
source instead.
Full suite in the CI posture: 4215 tests, 0 fail, 1 skipped. Same under the +399 day clock shift. The
temp-dir, dated-fixture, test-count and revision-row guards all pass, and a run under a fresh TMPDIR
leaves nothing behind.
DES-ADMIN-VIA-PI-EXTENSION amended, and the two other live sentences stating the old claim are corrected
with it. DES-PANEL-SEPARATE-FROM-RECEIVER UNCHANGED, checked. The bidi and zero-width residual is
UNCHANGED and still open as #402: those code points sit outside a control-byte class defined by what a
terminal interprets, and they are a reader-deception question rather than a width one.
No version moved: root and worker 1.10.3, receiver 1.5.0, admin 1.10.2.
Carried from earlier in this round:
receiver/src/start.mjsimports@edgehero/pi-dispatch/watch-closer,which the published worker 1.10.3 does not export, so the next release PR must move the receiver's worker
range to the version that ships it.
The review rounds, and what they cost
Three adversarial rounds each confirmed a defect, which is this project's cap, so the fourth pass decided
merge-with-follow-ups rather than starting another cycle. Worth recording, because the pattern is the
finding:
and it under-counted 139,820 code points, including the CJK Extension B that this issue is about. In
two classes it was worse than the
.lengthit replaced.inside the count while the cut and the editor called it one character at a time, so the cut overflowed.
It also found the line editor's edit side creating half a character in the value that gets saved.
spliced its neighbours together, so a cut to one column emitted two. A 24-column pane drew at 43.
containment, which is applied here rather than filed.
Each round's defect was in the previous round's repair, never in the original diagnosis. The structural
answer that ended it was to stop patching sites: one stepper that the count, the cut and the editor all
walk, and one rule about where half a character is removed.
Filed rather than fixed, under the cap: #417 (the renderer counts a cluster's base twice when the cluster
begins with a non-printing character, an under-count that amplifies) and #418 (two HTML views still size
and cut by code unit). Both are named in the spec entry. #402's text is corrected where this change made
it false.