Skip to content

feat(viz): number mbtx source lines in both HTML renders - #1108

Merged
bobzhang merged 5 commits into
mainfrom
feat/mbtx-line-numbers
Aug 28, 2026
Merged

feat(viz): number mbtx source lines in both HTML renders#1108
bobzhang merged 5 commits into
mainfrom
feat/mbtx-line-numbers

Conversation

@bobzhang

Copy link
Copy Markdown
Collaborator

Summary

An mbtx run's compiler diagnostics cite line:column positions into the submitted source, but neither the desktop transcript card nor the static viz export numbered the program — following a diagnostic meant counting lines by hand.

  • New shared @syntax_html.moonbit_numbered_source helper: each highlighted line gets a right-aligned gutter, space-padded to the last line's number width so per-row grids stay aligned across digit boundaries (line 9 → · 9, line 10 → 10).
  • Both mbtx render paths (desktop card, viz export) use it. The gutter reuses the read card's grid styling under shared moonbit-line / moonbit-gutter / moonbit-code classes and is user-select: none, so copied text stays pure source.
  • The desktop card stops clamping the source through truncate_output: a clamp would misnumber every line after its skip marker, and card_fields' 128k-unit ceiling already bounds the payload. Empirically (171 recorded calls), sources top out at 37 lines / ~1.5k UTF-16 units against the 101-line / 20k-unit truncation thresholds, so the clamp had never fired.

Display-only: nothing under agent_tool/mbtx/, the protocol, or prompts changed.

Test plan

  • New helper test covers 1-based numbering, digit-boundary gutter padding, and token classes.
  • Desktop card and viz export tests extended with gutter assertions.
  • moon check clean; full moon test passes (2065 native + 34 js).

🤖 Generated with Claude Code

https://claude.ai/code/session_01EgSh3EMeThZdTLxfyHjC9a

bobzhang and others added 3 commits August 28, 2026 15:13
An mbtx run's compiler diagnostics cite line:column positions into the
submitted source, but neither the desktop transcript card nor the static
viz export numbered the program - following a diagnostic meant counting
lines by hand.

Add a shared @syntax_html.moonbit_numbered_source helper that pairs each
highlighted line with a right-aligned gutter, space-padded to the last
line's number width so the per-row grids stay aligned across digit
boundaries. Both mbtx render paths use it; the gutter reuses the read
card's grid styling under shared moonbit-line/gutter/code classes and is
user-select: none, so copied text stays pure source.

The card also stops clamping the source through truncate_output: a clamp
would misnumber every line after its skip marker, and card_fields'
128k-unit ceiling already bounds the payload (recorded calls top out at
37 lines / ~1.5k units against 101-line / 20k-unit thresholds).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EgSh3EMeThZdTLxfyHjC9a
One entry point with `numbered? : Bool = false` instead of a separate
moonbit_numbered_source: the transcript card and viz export opt in, the
composer's approval body keeps the un-numbered default - there are no
diagnostics to match at approval time, and its comment now records that
choice instead of claiming identity with the transcript render.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EgSh3EMeThZdTLxfyHjC9a
Codex review of #1108 flagged that dropping truncate_output left the
numbered render unbounded: the 128k-unit card ceiling still admits tens
of thousands of two-character lines, and the viz export path had no
ceiling at all.

Bound it inside the numbered renderer, where numbering stays honest:
past 501 lines the head 400 and tail 100 rows keep their true numbers
around one unnumbered "lines skipped" row. Recorded programs top out
near 40 lines, so the clamp is display-invisible in practice.

Also per that review, add the missing regression coverage: a >101-line
program renders every numbered line in both the card and the export (so
reintroducing the transcript clamp would fail), one enormous line stays
a single numbered row, and the degenerate clamp elides the middle under
true tail numbers. The numbered branch moves to a private helper so the
consolidated entry point stays a thin dispatch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EgSh3EMeThZdTLxfyHjC9a
@bobzhang

Copy link
Copy Markdown
Collaborator Author

Codex CLI review (xhigh reasoning)

An initial review pass requested two changes, addressed in c95a227:

  1. Unbounded numbered render for degenerate payloads — the 128k-unit card ceiling still admitted tens of thousands of two-character lines, and the viz export path had no ceiling at all. Resolved: the numbered renderer now clamps past 501 lines to head 400 + tail 100 rows with true line numbers around one unnumbered "lines skipped" row (.moonbit-skip), so cost is bounded and numbering never lies.
  2. Missing regression coverage for the dropped clamp — resolved with tests: a 600-line clamp case asserting true tail numbers 501–600 (401 absent, 501 rows total), a 25k-unit single line staying one numbered row, and 120-line programs keeping every numbered line with no skip marker in both the desktop card and the viz export.

Re-review verdict, verbatim:

No remaining issues found.

The clamp arithmetic is correct:

  • Up to 501 lines render in full.
  • Beyond that, rows 1–400 are followed by one unnumbered marker and the final 100 rows.
  • For 600 lines, exactly 401–500 are skipped and the tail retains true numbers 501–600.
  • The rendered structure is capped at 501 .moonbit-line rows.

The desktop and viz 120-line tests exercise their real rendering paths and reject the old line clamp; the 25k-unit test protects the single-line behavior. CSS and call sites are consistent across desktop and export.

Sign-off: c95a227 resolves both prior review requests, and I found no remaining correctness issues in origin/main...HEAD.

Codex's read-only sandbox couldn't execute the test suite; run separately: moon check clean, full moon test 2065/2065 native plus the touched packages on js (editor/viewer/html 5, transcript component 54, viz 31 js + 14 native), all passing.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EgSh3EMeThZdTLxfyHjC9a

bobzhang and others added 2 commits August 28, 2026 15:49
The numbered? flag made one name return two structurally different DOM
shapes - a single code element or a fragment of gutter-grid rows - and
the caller must know which it asked for anyway to style it. Two named
functions state that honestly; the tokenization stays shared through
moonbit_source_lines. Rendering behavior is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EgSh3EMeThZdTLxfyHjC9a
A fresh Codex review of #1108 flagged that the degenerate clamp decided
what to keep only after moonbit_source_lines had materialized fragments
for every line, and that the skip row's empty gutter collapsed its own
grid's max-content column, shoving the marker out from under the code.

Tokenization now takes a keep predicate: every line is still lexed so
state flows across the skipped middle, but only kept lines inflate
tokens and build spans. The skip row's gutter pads to the shared width,
asserted by the clamp test. Rendered output is otherwise unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EgSh3EMeThZdTLxfyHjC9a
@bobzhang

Copy link
Copy Markdown
Collaborator Author

Fresh Codex CLI review (xhigh reasoning) — second pass

An independent fresh-eyes pass at 14a8039 requested two fixes, addressed in 95d3459:

  1. Clamp decided after building every line's VNodes — tokenization now takes a keep predicate: every line is still lexed so tokenizer state flows across the skipped middle, but only kept lines inflate tokens and build spans.
  2. Skip row's empty gutter collapsed its grid column — the marker row's gutter now pads to the shared width, pinned by a test assertion.

Final verdict, verbatim:

No blocking issues found.

The clamp and numbering are correct: 501 lines render fully; 502 lines render 1–400, a "2 lines skipped" row, then 403–502. Lexer state advances before the keep predicate. HTML remains escaped, gutter CSS is consistent across desktop and static export, and no agent, protocol, or prompt files changed.

git diff --check passed, direct 501/502 boundary probes passed, and the current HTML renderer test bundle passed all five tests.

Sign-off: I approve PR #1108 at HEAD 95d3459260d870ea2b83fc8e7569b9a62ce6fbca.

Run separately (Codex's sandbox is read-only): moon check clean, full moon test 2065/2065 native, touched packages green on js. Merging.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EgSh3EMeThZdTLxfyHjC9a

@bobzhang
bobzhang merged commit 9c27bc9 into main Aug 28, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant