Simplify the completed SchemaBinary implementation

[tim-smart]

Final simplification pass over SchemaBinary, reviewing the combined implementation as a whole: the original codec, arena output, integral varints, the parser hot-path work, and both wire modes.

Closes EFF-783

Stacked on agent/codex-engineer/58cacc24 at 0c8163ece5, which is the head containing merged #7370. This branch and PR stay open for the bundle-size pass in EFF-784.

No behaviour changed, and that is checked rather than asserted

A differential harness encoded 34 schema/value pairs in both wire modes (every leaf kind, optional and full presence bitmaps, index signatures, tuples with rest, tagged and untagged unions, Option/Result/Exit/Cause/CauseReason, recursive trees), 12 error cases (missing field, wrong type, truncated frame, leftover bytes, flipped envelope, flipped body byte, in both modes), and byte-by-byte streaming feeds. It ran against the base source and against this branch: 86 lines of hex payloads, decoded values, issue text and issue paths, identical.

The benchmark's payload-size columns are also unchanged, which is the same claim from a second direction.

What was removed

One byte-sequence builder for both hashes. sentinelSetHash carried its own copy of the 32-bit FNV fold and its own u32le encoder, and allocated a one-element Uint8Array per mixed byte. The fingerprint walk already had pushU32 / pushUvarint / pushU64. Those move up next to fnv32 / fnv64, and sentinelSetHash builds one array and hashes it once. Two hashes, one set of encoders.

Exit, Cause, and CauseReason share one compiled node. The exit layout carried loose error and defect children, so both the encoder and the decoder built { _: "cause", error, defect } fresh on every failed Exit. They now share a ReasonLayout compiled once. The fingerprint still hashes the same three children in the same order, so the hash does not move.

Three decode rules that existed in two or three copies. decodeSized, decodeInline, decodeSlot, and decodeExtraPair mirror encodeSized, isInlineSlot, and the extra-key writer. Before this, decodeStructPositional reimplemented decodeArray's inline-slot rule with the null/undefined branch folded differently, and the extra-key loop existed twice, differing only in whether a count or r.pos < r.end bounded it. compileArray also spelled out isInlineSlot's body inline rather than calling it. The encode side has had isInlineSlot since the arena work; the decode side now says the same thing once.

The struct and tuple paths get different helpers on purpose. A struct field carries inline, computed once at layout-compile time, so decodeStructPositional classifies nothing per value and field.inline ? decodeInline : decodeSized is free. A tuple slot has no such flag. Routing it through isInlineSlot and then decodeInline would run packedSize and isSelfDelimiting twice per element, so decodeSlot makes that decision once. Same shared rule, one classification.

One union encoder. encodeUnion and encodeUnionPositional ran the same member scan and differed only in the selector written for the member that matched. They are one function whose two ctx.positional branches sit on the matched arm, so the branch count per encode is what it was.

One missing-required-field surface. missingKeyIssue / throwMissingKeys replace the copy in each struct decoder. Only the Pointer/MissingKey construction is shared; the accumulator stays at the call site as (issues ??= []).push(...), where it is read.

An unreachable guard. IndexSignatureCache threw on a non-positive capacity; the only bounded caller passes a module constant.

What was deliberately left alone

• The union decoders stay split. The default mode skips a member it does not know and resolves to absent; fingerprint mode fails the frame. Folding them would put that fail-closed rule behind a flag, which is exactly the seam that should stay legible.
• decodeArray's uniform loop stays specialised. Schema.Array(S) settles the slot layout and the length rule once outside the loop. Routing it through decodeInline / decodeSized would move packedSize back inside the per-element path.
• Parser and one-shot decode setup stay separate. They differ in what they actually do: incremental buffering and an incremental frame header against a complete input.
• decodeStruct's duplicate-id bookkeeping stays. The 32-bit mask with set fallbacks, and the separate seen and present masks, are each load-bearing: a known union member may decode to absent, so presence cannot reuse the duplicate mask.
• The struct encoders stay separate. Ids and lengths against a presence bitmap and inline slots is a real difference, not a flag.
• astKind and compileDeclaration both switch on the representation id. They return different things (a wire kind, a layout) and merging them would not remove the agreement requirement.

Documentation

The module docstring now states arena ownership and the error surface: which failures are InvalidValue, which are a MissingKey under a Pointer, and that schema-author bugs throw an Error at layout-compile time rather than surfacing as an issue. parser documents why its options are fixed at construction, the 256-entry cache bound and its one-replacement-per-frame rule, and maxFrameSize.

The index-signature key-filter seam raised in the #7370 review is now written down at matchIndexSignature: a key predicate is a check, checks never reach the wire or the fingerprint, so a reader cannot distinguish "the writer sent a key I filter out" from "the writer used a different schema", and such a key is dropped where a mismatched field id fails the frame.

Options no longer links {@link layoutFingerprint}, which is not exported.

Benchmarks

nix develop -c pnpm --dir packages/effect exec node benchmark/schema/SchemaBinary.ts, Node 26.7.0 on Linux x64.

Sequential before/after runs showed a consistent negative skew that turned out to be machine drift, so the numbers below are seven interleaved before/after pairs (stash, run, restore, run), which cancels it. Medians per row, 104 SchemaBinary rows across one-shot encode/decode and all three streaming feeds:

median	+0.30%
mean	+0.42%
worst	-1.6%
best	+2.1%

Nothing outside ±2.1%, which is this benchmark's run-to-run spread.

Two real regressions were found and fixed rather than absorbed. The first interleaved pass showed small record / parser / single frame at -2.8% with non-overlapping distributions across three pairs (802k/809k/803k against 780k/780k/784k). The cause was a checkMissingKeys(layout, issues) call added on every struct decode, where the original code had an inline issues !== undefined check. The one-shot path hid it because the Schema transformation wrapper dilutes it; the parser path does not. Guarding at the call site so only a failing decode calls out restored it: that row is now +0.3% pooled, and the case that had regressed reads +1.9% in the pair immediately after the fix.

The second came out of review: the tuple slot double-classification above. It hides in the aggregate because the repo benchmark's only tuple case is Array(Tuple([Number, Number, Boolean])), where the actual field decoding dwarfs the classification. Isolating it needs both a slot shape that is nothing but classification and enough elements to swamp the per-call cost of the Schema wrapper. At 1024 elements, Array(Tuple([Boolean, Boolean])) decode, six interleaved rounds, ns per element:

	base	8661864dca	head
tuple[bool,bool]	144.09	147.74 (+2.5%)	143.75 (-0.2%)
tuple[num,num,bool]	189.84	190.06 (+0.1%)	186.92 (-1.5%)
tuple[null,bool]	151.41	151.89 (+0.3%)	151.73 (+0.2%)
tuple[str,num]	171.59	172.51 (+0.5%)	172.69 (+0.6%)

decodeSlot puts the two-boolean case back on base. The other three shapes never left it, which is the expected result: they do enough real work per element that one redundant packedSize does not register.

The aggregate benchmark was re-run interleaved against base after both fixes, four pairs, and is unchanged: median +0.00%, mean -0.04%, range -2.3% to +3.2%.

The benchmark markdown is unchanged. Its size columns are exactly right, and its rates still describe this tree within noise; re-running fresh machine-local numbers over it would be churn.

Verification

• nix develop -c pnpm vitest run --project effect test/unstable/encoding: 117 passed
• nix develop -c pnpm vitest run --project effect: 8710 passed, 47 skipped
• nix develop -c pnpm lint and nix develop -c pnpm check: clean
• Differential byte/error/stream harness against base: identical (86 lines)
• Tuple slot harness against base: 15 tuple shapes covering every slot class (zero-width, packed, self-delimiting, length-prefixed), each in both modes, truncated at every byte offset, plus trailing-byte and arity errors. 428 lines, identical
• Benchmark: above

One test added: both Exit branches round-trip in fingerprint mode, where the failure branch has no length prefix to resynchronise on, and the Exit and Cause fingerprints stay distinct despite the shared node.

For EFF-784

isCyclic pulls Chunk, HashMap, HashSet, and Redacted into the module purely for cycle detection. That is a bundle-size question rather than a simplification one, so it is left for the next stage rather than guessed at here.

🤖 Generated with Claude Code

[changeset-bot]

⚠️ No Changeset found

Latest commit: c0163d9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR