Add: a cell model, so a wide glyph occupies the two columns it is drawn in - #66
Merged
Conversation
Three defects that surfaced during the last triage but were out of that PR's scope. Each verified before being planned. watch had no unknown-flag guard: anything flag-shaped it did not recognise stayed in the argv and became the command. My first probe of this was worthless — a non-tty run refuses on "stdin is not a TTY" before parsing anything, so the refusal was the TTY check and proved nothing. Driven through a real PTY.spawn, `rune watch --timeout 5 -- echo hi` exited 127 with the child never running. run has guarded this since it grew flags; watch never did, which made it the worse of the two, because run at least says something. The guard now lives in Command.flag_error and is shared rather than copied: the two had already drifted once, with run growing the inline-value branch and watch having no guard to grow it in. --grep ignored --since. `filter` was handed the sliced text and then called transcript.grep, which searched the whole transcript, so a read from a cursor recorded after the first line still returned that line and grep_matches counted it. A caller paging with --since=<last cursor> got the whole history on every page under a count that looked like it had filtered. --max-output did not bound clean_stdout/clean_stderr at all: a 200-byte budget returned 10,506 bytes across four fields. Now 1,012. An adversarial pass found two merge-blockers in this work and was right about both. First, the watch guard shipped with NO test — reverting all three guard files left the suite fully green, so CI could not tell the fix from its absence, including its own headline case. Six tests now cover it, including a drift guard and the case it must not lose, a child's own flags surviving. Second, moving unknown_flag_error and INLINE_VALUE_ERROR out of RunCommand left the pty_runner spec documenting them and added six exports nothing documented; specsync reported two hard errors, both now gone. It also caught a false claim in my own comment: watch's VALUE_FLAGS said it "cannot drift from the parser" while --log is appended by hand, because its pattern is inline in scan_head. Corrected to what is true. One reported loss was checked and deliberately left: the guard covers the leading position only, so `rune watch echo hi --log=/tmp/x` still writes rune's own log to the child's path. Measured byte-identical before and after, so it is pre-existing and recorded in invariant 20 rather than fixed here. Note an API change: RunCommand::INLINE_VALUE_ERROR still resolves through inheritance but now carries a second placeholder, so format(..., name:) alone raises KeyError. It has never appeared in a tagged release. 584 examples, 0 failures. Controls: watch guard reverted fails 4 of 33, grep reverted fails 1 of 2, stream bounds reverted fails 1 of 3. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rf59AtQmJcodUJ6vXDZNY
…wn in The last of the five renderer gaps, and a correction of my own conclusion about it. A cell model was built once before and reverted, and the spec recorded that it had been "measured worse than the gap". That was wrong. The A/B compared two working trees and misattributed which output came from which side. Re-measured against three explicit revisions on the same 56,928-byte grok capture that had emitted a CJK table: cc8bb3c (one column) "東h京 Tokyo" "大 阪 Osaka" ad76e22 (one column) "東h京 Tokyo" "大 阪 Osaka" cell model "東京 Tokyo" "大阪 Osaka" The one-column model corrupts real agent output and always did: an agent positions its columns assuming two per CJK glyph, and a renderer counting one puts every later write in the wrong place. I had it backwards, reverted a correct fix, and wrote the mistake into the contract. I also ruled out the four renderer fixes shipped since — alt screen, DECAWM, IRM, charsets — by disabling each in turn on top of main. The corrupted rows are byte-identical in all four cases. It is the column arithmetic. Rows are now Arrays of cells: nil, a String of one graphic plus its combining marks, or CONTINUATION for the right half of a wide glyph. That makes two things true the String could not. A cell holds any number of characters without moving the cells after it, which is what makes combining marks work at all — appending a mark to a String row put every later index off by one and the next graphic overwrote it. And the pair invariant can be restored after the fact, in one heal pass, instead of being taught to twelve operations that each slice the row. The first attempt tried the latter and lost. pad/padded_line are gone: assigning past the end of an array fills with nil and a nil cell renders blank. That padding was the mechanism by which a column index became an index into text. 591 examples, 0 failures. Controls: forcing the width table to one column fails 4 of 136 parser examples; making heal a no-op fails 2. Stacked on leif/watch-flag-guard so the change records do not collide. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rf59AtQmJcodUJ6vXDZNY
3 tasks
0xLeif
force-pushed
the
leif/screen-cell-model
branch
from
August 18, 2026 17:36
5f4d77b to
ba810bb
Compare
# Conflicts: # .specsync/change-sequence.json
CodeQL flagged .sub(/\n+\z/, '') in Screen#to_s as a polynomial-regex risk on uncontrolled input (PR #66 review). Measured rather than trusted: a single quantifier anchored at \z has no backtracking ambiguity to exploit, and both a trailing-newline run and an adversarial no-match input scale linearly to 800,000 bytes. The alert looks like a false positive for this specific pattern. Fixed anyway, and with the better fix regardless of whether the alert was real: to_s already holds the rendered rows as an array before joining them, so popping trailing empty rows off that array gives the same result without a regex on the render path at all. Verified equivalent on a 5-row scrolling grid and on the all-blank and single-line edge cases. 594 examples, 0 failures. This also carries the branch through: PR #65 was squash-merged, which gave main a new commit SHA for content that used to be ac38dba. The branch had been rebased onto the new main, which orphaned CHG-0070's recorded verification commit and left specsync check refusing both reopen (content unchanged, so not stale by its own check) and outright acceptance (recorded commit unreachable). Undone by merging origin/main into the pre-rebase commit instead of rebasing, which keeps the accepted commit reachable in history — the standard fix for exactly the trap this repo's own CLAUDE.md warns about with --amend, encountered here via rebase instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rf59AtQmJcodUJ6vXDZNY
0xLeif
force-pushed
the
leif/screen-cell-model
branch
from
August 18, 2026 18:23
ba810bb to
8893c35
Compare
0xLeif
added a commit
that referenced
this pull request
Aug 18, 2026
Carries three things: the merge needed to keep this stacked PR's specsync evidence reachable after upstream squash-merges, a re-anchor I forgot to commit in the previous push, and a real race CI found on the resulting tree. The merge itself needed real conflict resolution, not just a repeat of the mechanical squash-SHA trap. screen.rb, its spec and both spec docs had genuinely diverged: my branch predates the CodeQL regex fix (#66), so origin's side won on to_s and the resync/Indic tests were additive on both sides. One correction caught before it landed: I initially took origin's copy of character_width.rb wholesale, which would have silently dropped this branch's own Indic-marks fix — caught by grep and the full suite before committing, not after. CI then found what local runs did not. Ruby 3.4's job failed a test this PR added: `abandon` sends SIGKILL and immediately writes state: 'failed', but SIGKILL is asynchronous, so `list` right after could still see the supervisor as alive and report 'running' — describe deliberately recomputes state from real process liveness rather than trusting the record, precisely so a supervisor killed without its cooperation is never reported as-is, which is exactly the case a not-yet-dead abandon target is. stop hit this identical shape once already, and the fix is already written down at its own await_death call: "SIGKILL is asynchronous... the very next command saw the session as running." abandon had the same fire-and-kill shape without the wait. It now calls await_death before recording the failure, matching stop exactly. Passed 3/3 full local runs before this was understood as a real race rather than the flake noted in #68's own PR body — worth correcting: that was this bug, present since abandon was written, and CI's slower runner is what surfaced it. 607 examples, 0 failures. specsync 32/32 files, 7431/7431 LOC. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rf59AtQmJcodUJ6vXDZNY
0xLeif
added a commit
that referenced
this pull request
Aug 18, 2026
…SCII found (#68) * Fix: watch was executing the flags it did not recognise Three defects that surfaced during the last triage but were out of that PR's scope. Each verified before being planned. watch had no unknown-flag guard: anything flag-shaped it did not recognise stayed in the argv and became the command. My first probe of this was worthless — a non-tty run refuses on "stdin is not a TTY" before parsing anything, so the refusal was the TTY check and proved nothing. Driven through a real PTY.spawn, `rune watch --timeout 5 -- echo hi` exited 127 with the child never running. run has guarded this since it grew flags; watch never did, which made it the worse of the two, because run at least says something. The guard now lives in Command.flag_error and is shared rather than copied: the two had already drifted once, with run growing the inline-value branch and watch having no guard to grow it in. --grep ignored --since. `filter` was handed the sliced text and then called transcript.grep, which searched the whole transcript, so a read from a cursor recorded after the first line still returned that line and grep_matches counted it. A caller paging with --since=<last cursor> got the whole history on every page under a count that looked like it had filtered. --max-output did not bound clean_stdout/clean_stderr at all: a 200-byte budget returned 10,506 bytes across four fields. Now 1,012. An adversarial pass found two merge-blockers in this work and was right about both. First, the watch guard shipped with NO test — reverting all three guard files left the suite fully green, so CI could not tell the fix from its absence, including its own headline case. Six tests now cover it, including a drift guard and the case it must not lose, a child's own flags surviving. Second, moving unknown_flag_error and INLINE_VALUE_ERROR out of RunCommand left the pty_runner spec documenting them and added six exports nothing documented; specsync reported two hard errors, both now gone. It also caught a false claim in my own comment: watch's VALUE_FLAGS said it "cannot drift from the parser" while --log is appended by hand, because its pattern is inline in scan_head. Corrected to what is true. One reported loss was checked and deliberately left: the guard covers the leading position only, so `rune watch echo hi --log=/tmp/x` still writes rune's own log to the child's path. Measured byte-identical before and after, so it is pre-existing and recorded in invariant 20 rather than fixed here. Note an API change: RunCommand::INLINE_VALUE_ERROR still resolves through inheritance but now carries a second placeholder, so format(..., name:) alone raises KeyError. It has never appeared in a tagged release. 584 examples, 0 failures. Controls: watch guard reverted fails 4 of 33, grep reverted fails 1 of 2, stream bounds reverted fails 1 of 3. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rf59AtQmJcodUJ6vXDZNY * Add: a cell model, so a wide glyph occupies the two columns it is drawn in The last of the five renderer gaps, and a correction of my own conclusion about it. A cell model was built once before and reverted, and the spec recorded that it had been "measured worse than the gap". That was wrong. The A/B compared two working trees and misattributed which output came from which side. Re-measured against three explicit revisions on the same 56,928-byte grok capture that had emitted a CJK table: cc8bb3c (one column) "東h京 Tokyo" "大 阪 Osaka" ad76e22 (one column) "東h京 Tokyo" "大 阪 Osaka" cell model "東京 Tokyo" "大阪 Osaka" The one-column model corrupts real agent output and always did: an agent positions its columns assuming two per CJK glyph, and a renderer counting one puts every later write in the wrong place. I had it backwards, reverted a correct fix, and wrote the mistake into the contract. I also ruled out the four renderer fixes shipped since — alt screen, DECAWM, IRM, charsets — by disabling each in turn on top of main. The corrupted rows are byte-identical in all four cases. It is the column arithmetic. Rows are now Arrays of cells: nil, a String of one graphic plus its combining marks, or CONTINUATION for the right half of a wide glyph. That makes two things true the String could not. A cell holds any number of characters without moving the cells after it, which is what makes combining marks work at all — appending a mark to a String row put every later index off by one and the next graphic overwrote it. And the pair invariant can be restored after the fact, in one heal pass, instead of being taught to twelve operations that each slice the row. The first attempt tried the latter and lost. pad/padded_line are gone: assigning past the end of an array fills with nil and a nil cell renders blank. That padding was the mechanism by which a column index became an index into text. 591 examples, 0 failures. Controls: forcing the width table to one column fails 4 of 136 parser examples; making heal a no-op fails 2. Stacked on leif/watch-flag-guard so the change records do not collide. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rf59AtQmJcodUJ6vXDZNY * Fix: a failed launch was reported as success, and three defects non-ASCII found Four defects from two sources, each verified before being planned. A launch that never happened reported success. `start` with a command not on PATH returned status "ok", state "exited", exit_code 127 — so a caller checking the field whose entire job is to say whether the call worked saw success. It was documented as "check state instead", which is the wrong shape of answer: an envelope should not need a footnote to be read correctly. Reported from a 22-minute real drive where it cost an hour. Only 127 fails, deliberately. `start -- true` exits 0 immediately and is a successful launch of a program with nothing to do; treating any prompt exit as failure would break every short-lived child. The session record is kept rather than deleted — start failing loudly is the fix, and removing the transcript that shows why would trade one quiet failure for another. An error that was confidently wrong. A session started in one directory and read from another got "No such session", and that error told the reader to run `rune session list`, which is scoped to their own project and returns nothing, actively confirming the wrong conclusion. rune knew where it was the whole time. The message now names the project. This has caught three readers, two of whom had read the guide's warning first, which is when a documented gotcha stops being a documentation problem. Two defects the native-language translation round found, by conducting each rune session entirely in the language being translated rather than sending English: CharacterWidth::ZERO covered Latin, Greek, Cyrillic, Hebrew, Arabic and Thai and omitted every Indic script, so हिन्दी was charged six columns for six codepoints. My own table, written hours earlier. Fixed as the Mn/Me subset, not every Indic mark: U+093F is a spacing mark and takes a column in wcwidth and xterm, so zeroing all of them would be wrong in the other direction. हिन्दी is now five columns, matching xterm rather than the three a shaping engine draws. ScreenRenderer.resync searched with String#index (characters) and sliced with byteslice (bytes). On "日本語テキスト\e[1mAFTER" the ESC is at character 7 and byte 21; it cut at 7, returning "\xAA\x9Eテキスト\e[1mAFTER" — both splitting a character and failing to drop the remainder it exists to drop. One claim from the same report did NOT reproduce and is not fixed: that a send over ~1024 bytes jams the session while rune answers settled:true. Measured at 600/1100/4096/20000 bytes — no jam below 20k, and at 20k the follow-up is refused with status:error and recovers by itself in ~10s, which is what ROADMAP already records. The reported shape looks like a probe reading clean_output without checking status. Also lands the six in-language guide translations and their report. 604 examples, 0 failures. Controls: launch_failure reverted fails 1 of 3, resync byte index 2, Indic ranges 2, project lookup 2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rf59AtQmJcodUJ6vXDZNY * Merge origin/main and fix a CI-only abandon race CodeQL's review missed Carries three things: the merge needed to keep this stacked PR's specsync evidence reachable after upstream squash-merges, a re-anchor I forgot to commit in the previous push, and a real race CI found on the resulting tree. The merge itself needed real conflict resolution, not just a repeat of the mechanical squash-SHA trap. screen.rb, its spec and both spec docs had genuinely diverged: my branch predates the CodeQL regex fix (#66), so origin's side won on to_s and the resync/Indic tests were additive on both sides. One correction caught before it landed: I initially took origin's copy of character_width.rb wholesale, which would have silently dropped this branch's own Indic-marks fix — caught by grep and the full suite before committing, not after. CI then found what local runs did not. Ruby 3.4's job failed a test this PR added: `abandon` sends SIGKILL and immediately writes state: 'failed', but SIGKILL is asynchronous, so `list` right after could still see the supervisor as alive and report 'running' — describe deliberately recomputes state from real process liveness rather than trusting the record, precisely so a supervisor killed without its cooperation is never reported as-is, which is exactly the case a not-yet-dead abandon target is. stop hit this identical shape once already, and the fix is already written down at its own await_death call: "SIGKILL is asynchronous... the very next command saw the session as running." abandon had the same fire-and-kill shape without the wait. It now calls await_death before recording the failure, matching stop exactly. Passed 3/3 full local runs before this was understood as a real race rather than the flake noted in #68's own PR body — worth correcting: that was this bug, present since abandon was written, and CI's slower runner is what surfaced it. 607 examples, 0 failures. specsync 32/32 files, 7431/7431 LOC. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rf59AtQmJcodUJ6vXDZNY --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The last of the five renderer gaps — and a correction of my own earlier conclusion about it.
I had this backwards, and wrote the mistake into the contract
A cell model was built once before and reverted, and
parsers.spec.mdinvariant 17 recorded that it had been "measured worse than the gap". That was wrong. The A/B compared two working trees and misattributed which output came from which side.Re-measured against three explicit revisions, on the same 56,928-byte grok capture that had emitted a CJK table:
The one-column model corrupts real agent output and always did. An agent positions its columns assuming two per CJK glyph; a renderer that counts one puts every later write in the wrong place. I reverted a correct fix on a bad reading, then recorded the error as a documented limitation — which would have told the next reader not to attempt the thing that works.
I also ruled out the four renderer fixes shipped since — alternate screen, DECAWM, IRM, charsets — by disabling each in turn on top of
main. The corrupted rows are byte-identical in all four cases. It's the column arithmetic.The design decision that matters
Rows are now
Arrays of cells:nil, aStringholding one graphic plus its combining marks, orCONTINUATIONfor the right half of a wide glyph.That makes two things true the
Stringcould not:Stringrow put every later index off by one, so the next graphic overwrote it. Measured under the old model, a decomposedérenderede.healpass, rather than taught to the twelve operations that slice a row. The first attempt tried the latter and lost: insert, delete, erase and scroll can each separate a glyph from its continuation, and a repainting TUI uses exactly those. One rule in one place is also the rule a new operation cannot forget.pad/padded_lineare gone. Assigning past the end of an array fills withniland anilcell renders blank — that padding was the mechanism by which a column index became an index into text.Behaviour
Test Plan
specsync check32/32 files, 7325/7325 LOCharnesses/renderer_gaps.rbheala no-op fails 2Invariant 17 is rewritten to record the measurement that reverses the earlier claim, and says plainly that the error was a mislabelled comparison rather than a bad measurement.
🤖 Generated with Claude Code
https://claude.ai/code/session_018rf59AtQmJcodUJ6vXDZNY