Repository navigation
Keep long REPL output readable - #990
Merged
Merged
Conversation
The REPL's width measurement bounded each render by element count only. The engine has a second budget — every space-separated part of every text in a render is kept in a cache it refuses from, and that cache holds 16,384 of them however few elements carry them. Rebuilding the instance gives it the same cache, so this is the refusal the renderer's recovery cannot absorb, and it reached the screen as the error that ends it. A long Output did it two ways: one recorded line held more parts than any batch size bounds, and a batch inside the element budget still asked for more parts than the cache holds. Both now bound the request rather than the document — a batch is bounded by parts as well as elements, and a line is measured as far as the region could still hold rather than to its end. Measured on the retained record the reported crash came from: 9.84 MB, 4,029 lines, longest line 90,294 parts, now prepared whole as 120,650 rows with every logical line recovered exactly and no row wider than its region.
PR #990: Keep long REPL output readable3 files, +183 / -7 Scope✅ PR scope looks good. Structural✅ No structural bloat detected. Slop✅ Slop indicators look low. Static AnalysisOxlint: 6 diagnostics across 1 file (1 rule) no-shadow (6): packages/cli/src/repl/fitting.ts CorrectnessNo extraneous code patterns detected. |
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.
Why
Long REPL output crashed the screen. An entry whose recorded output held a large
document — the repository's own
architecture.md, read and emitted by a Sidekickentry — could not be drawn, and reopening the retained history that held it
failed the same way. The command exited rather than showing the reading.
What changes
Long REPL output stays readable. A reading is still measured through the layout
engine and still kept whole; what changed is that a measuring render now stays
inside the engine's text budget as well as its element budget.
Before:
The pane exited with status 1 before any frame was drawn, so the output was
unreachable and so was the entry beside it.
After: the reading prepares and draws. Every logical line is recoverable from
the rows it was cut into, character for character, and no row is wider than the
region it was cut for.
How it works
The engine has two budgets per render, and only one of them was bounded.
WIDTHS_PER_RENDER = 1024bounded how many elements one measuring rendercarries. The second budget is text: every space-separated part of every text in
a render is kept in a cache the engine refuses from, and that cache holds 16,384
of them however few elements carry them. Measured on a freshly built engine, one
text of 16,384 parts, 1,366 texts of twelve, and 820 texts of twenty all refuse,
while 16,383 parts go through either way.
This is the one refusal the renderer's recovery cannot absorb.
measure()rebuilds the measuring instance and asks again, but a replacement instance is
given the same cache, so a render that overran it overruns the replacement too —
which is why the error reads even after rebuilding it. The recovery itself is
working: preparing all 8,951 lines of
architecture.mdrebuilds the instanceseven times and succeeds each time.
Two call sites overran the text budget:
fitLinemeasured the whole lineto decide whether it fits in one row. A retained entry's recorded output
holds lines far past the budget — the journal behind the reported failure
holds one of 90,294 parts — so the first measurement of that single line
refused. This is the reported trace.
row at once sent 1,024 rows per render. Prose rows carry more than sixteen
parts each, so it asked for more parts than the cache holds while asking for
fewer elements than the element budget allowed.
Both now bound the request rather than the document:
WORDS_PER_RENDERandwordsIn()count what a text costs the engine, the waythe engine counts it.
widths()closes a batch on either budget, and a textthat outgrows the budget alone still goes in a render of its own so the
refusal names that text rather than the loop never advancing.
windowOf(text, room)is the prefix of a line the region could still hold:room + 2parts. Every part after the first is preceded by a space, soroom + 2of them are already wider thanroomcolumns, and text does notnarrow when more of it is appended — so that prefix settles the question the
whole line was asked. It ends at a space, which is a grapheme boundary of its
own.
fitLineasks the engine about that prefix, andcutAtsearches itsgrapheme boundaries.
Review guide
Start with:
packages/cli/tests/repl-source-reading.test.ts, row S7Then review:
WORDS_PER_RENDERandwordsIn()— the budget and its unitwidths()— the batch now closes on either budget and always advanceswindowOf(), and its two callersfitLineandcutAtarchitecture.md— the settled width-measurement invariant now states bothbudgets
Look carefully at:
to > fromis what letsan over-budget text through alone instead of looping on it.
boundaries and token slicing are untouched, and the finished row is still
measured as the row it became and refused if it does not fit.
What must stay true
engine accept it — checked by S7 and S6, which recover every logical line's
exact text from the rows it was cut into.
measured as fitting; a line held back from that question is cut rather than
assumed to fit.
and per-row run assertions.
renderer.tsis not in this diff: twoengines, only the drawing one owning committed display state, and only a
capacity refusal replacing an instance.
reading is a projection of what was recorded.
How to verify it
Row S7 a reading with more words than one measuring render is still measured
whole, both arms through the real
entryReadingoutput path, the real fitterand a real engine:
budget is still measured whole, and fails on the pre-fix implementation with
the reported trace frame for frame. It would catch a fix that bounded only the
batch.
budget stays inside the text budget, and fails pre-fix at the pass that
measures every finished row. The lines are deliberately distinct: the engine
caches one entry per exact text, so identical lines cost it one line's parts
and prove nothing — an earlier draft of this arm passed pre-fix for exactly
that reason.
Both arms also check that no row is wider than its region and that each row's
runs are exactly its text, so a width answered for the wrong candidate shows up.
Measured against the real retained record the reported crash came from —
9,842,113 characters, 4,029 lines, longest line 90,294 parts:
exactly, widest row 127 in a 127-column content region.
That journal was opened for reading only. One record was copied out for the
probe; nothing was written back, no entry was replayed and no provider work was
resent.
Also passing: the whole
repl-source-readingsuite (4 passed, 47 steps) andrepl-presentation-style+repl-layout-admission+repl-terminal(38passed, 132 steps).
Scope
Included
fitting.ts, and a bounded per-line candidate.Intentionally unchanged
renderer.ts. Its two-engine split and its capacity recovery are correct andwere not the defect; the recovery is what the error message reports reaching
its limit.
reading would be a product decision, not a repair of this crash.
New abstractions
wordsIn()andwindowOf()are two module-private helpers infitting.ts,each with one purpose and callers in this file only. They sit beside the
existing
WIDTHS_PER_RENDERbound and introduce no public surface.Risks and limitations
seconds and 48 engine rebuilds for its first preparation. It is correct and
complete, and
ReplFitStoreholds the answer afterwards, but the first frameof a reading that large is slow. This is a known non-blocking limitation of
this PR, not deferred work it needs: bounding what a reading prepares at all
is a separate product question.
rows stay correct and complete; only the wrap point moves, and the previous
behavior for such a line was the crash this fixes.
Delivery status
Required CI and branch protection remain the merge gate and have not run yet for
this head. Lint, typecheck, JSR publishability and the three runtime shard
matrices are owed, and
greenwith them. The evidence above is the focusedevidence the feedback round ran; it is not a statement that CI passed or that
this is ready to merge.
One affected-test selection is unresolved rather than failed:
deno task test packages/cli/tests/repl-*.test.tswas running against thisexact tree when its owning process was torn down, so it carries no exit status.
It had reported no failure across 56,717 bytes of log at the point it was cut
off. It was deliberately not re-run — CI's shard matrices own the exhaustive
pass. Recorded in
.reviewer/render-capacity-20261025/logs/41-repl-suite-disposition.txt.Scope confirmation