ReviewOS

home-lang/home

YAML block scalars: phase 2 consumes the whole indent run where Bun stops at content_indent

#711
Closed glennmichael123 opened this 9 days ago ·
9 days ago

Follow-up to #708. After 91fc0f716 (explicit indicator) and 81ae379c8 (chomping), js/bun/yaml/yaml-block-scalar-matrix.test.ts is 941 pass / 143 fail — up from 598 at the start of the session. Pinned Bun is 1084 / 0, so the remaining 143 are real gaps.

Remaining failures by group

group~countwhat the group covers
ws66whitespace-only and leading-whitespace content
empties33trailing/interior empty lines
fold30folded (>) line joining
adv8unicode / BOM in content
tab3tabs after the content indent
crlf3CRLF line endings

Representative failure — ws/>2+/only2/w4:

input     "- >2+\n    \n    \n"
expected  ["  \n  \n"]
received  [""]

The structural divergence

Phase 1 now matches Bun. Phase 2 does not.

Bun (~/Code/bun/src/parsers/yaml.rs, phase-2 0x20 arm) consumes at most content_indent spaces and then hands the next character to the normal content path:

let mut indent = Indent::from(0);
while indent.is_less_than(ctx.content_indent) && next == 0x20 {
    indent.inc(1); self.inc(1);
}
self.line_indent = indent;
let nc = next;
ctx.cur_more_indented = matches!(nc, 0x20 | 0x09);
__c = nc;
break;

Home (packages/runtime/src/parsers/yaml.zig:2939-2974) consumes the entire space run, and appends everything past content_indent to the text inline, duplicating the folding logic:

var indent: Indent = .from(0);
while (self.next() == ' ') {
    indent.inc(1);
    if (ctx.content_indent.isLessThan(indent)) {
        switch (folded) { ... append ' ' with leading_newlines handling ... }
    }
    self.inc(1);
}

Two different algorithms. Home's passes 941 cases, so it is not simply wrong — but it has no counterpart in Bun's live code, and the classes it gets wrong (whitespace-only lines, folded joining, interior empties) are exactly the ones where "append the overflow immediately" differs from "stop at the indent and let append decide".

Bun also carries two ctx fields Home lacks that this arm sets: cur_more_indented and prev_more_indented (yaml.rs:4897 — "space/tab beyond content_indent? Breaks adjacent to such lines"). Those drive folding decisions, which is likely why the fold group is a distinct cluster.

Suggested approach

Replace Home's phase-2 space arm with Bun's shape rather than patching cases:

  1. Add cur_more_indented / prev_more_indented to LiteralScalarCtx.
  2. Make the space arm stop at content_indent, set line_indent, set cur_more_indented, and fall through to the content path.
  3. Move the overflow-space handling out of the scan loop into append, where Bun does it.

This is intricate control flow and worth adversarial review before landing — a partial port here is how the CSS nesting budget in #705 ended up relocating an abort rather than fixing it.

Refs #66

9 days ago

Tried the minimal version of the suggested fix — swapping only the phase-2 space arm to Bun's shape (consume at most content_indent, drop the inline overflow append, let the content path handle the rest). It makes things worse: 941 pass / 143 fail → 848 / 236, −93 passing. Reverted; nothing landed.

So the piecemeal swap is not viable, and the "suggested approach" in the issue body needs all three steps or none:

Home's inline overflow append is doing work that Bun distributes across state the ctx does not carry. Removing it hands the overflow spaces to append, but Home's append has no cur_more_indented / prev_more_indented to decide fold behaviour with, so folded joining and the leading-newline flush both go wrong in cases that pass today. The 93 newly-failing cases are the ones Home's inline handling was getting right by accident of doing it early.

Order that would actually work:

  1. Add cur_more_indented and prev_more_indented to LiteralScalarCtx and set them where Bun does (phase-1 tail after content_indent is fixed, and the phase-2 space arm).
  2. Move the overflow-space handling from the scan loop into append, gated on those flags, matching yaml.rs.
  3. Only then bound the phase-2 consumption at content_indent.

Doing 3 without 1 and 2 is what I measured above.

This is worth adversarial review before landing rather than another empirical attempt — the same shape as #705, where a partial port of a two-part mechanism relocated a crash instead of fixing it.

9 days ago

The complete phase-two port is now implemented and verified. 3fc137809 added Bun's previous/current more-indented state, moved overflow-space handling into append, and bounded phase-two indentation consumption; bae99f883 completed the header-boundary invariants.

Fresh current-main validation:

  • unchanged js/bun/yaml/yaml-block-scalar-matrix.test.ts: 1,084 passed, 0 failed;
  • the whitespace-only, empty-line, folded, Unicode/BOM, tab, and CRLF groups tracked here are all green;
  • no test edits, skips, timeout changes, or parser workarounds were used.

Closing this focused phase-two issue as completed. #737 is also closed; parent parity tracking continues in #66.

Sign in to comment on this issue.