Receiver: ts-proto generated codec replaces the hand-rolled reader/writer - #20
Merged
Conversation
…iter The JS receiver decoded frames with a hand-written protobuf Reader and a per-op tag switch (receiver/src/proto.ts + frames.ts), and encoded event payloads with a hand-written Writer. docs/design.md said the codec was pbf; it never was — the hand-rolled codec was chosen silently in a dispatch and the design record was not updated. Measured over the bench producers' real captured streams (19 of them, every op; harness kept untracked under bench/decoders): ts-proto generated readers over @bufbuild/protobuf/wire decode in ~0.7x the time of the hand-rolled decoder, and a hand-driven hybrid on the same runtime is no faster. protobuf.js generated is 4x slower. pbf is fastest but silently wrong on sint64 above 2^53, has no bounds checks, and cannot represent proto3 `optional` presence (ns/parent/anchor). So: generated readers and writers everywhere, no hand-written wire code. Codegen is `buf` from npm under Deno spawning the ts-proto plugin via `deno run` — no Node, no protoc (`just proto-ts`); generated code is committed and drift-gated in `just check`. FrameSink and its types are unchanged. Framing keeps partial-frame buffering, MAX_FRAME_BYTES, and the wait-vs-malformed distinction for a truncated length prefix; each frame is bounded by a post-decode position check, so a nested length that overruns the frame (or a decode that stops early on a zero tag) is a malformed frame in both modes. Strict mode walks the generated messages' `_unknownFields` with the same message-name errors. sint64 outside Number.isSafeInteger now throws instead of losing bits. The unanchored `gen/` .gitignore rule (for host/desktop/gen) is anchored so receiver/src/gen can be tracked.
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.
What
Replaces
receiver/src/proto.ts(hand-rolled protobuf Reader/Writer) and the per-op tag switch inframes.tswith ts-proto generated readers and writers over@bufbuild/protobuf/wire. Event payload encoding uses the generated writer too. No hand-written wire code remains in the receiver.Why
docs/design.mdsaid the JS codec was pbf; it never was — the hand-rolled codec was chosen silently in a dispatch and the record was not updated. Evaluating the options against the receiver's actual needs:readSVarintis silently wrong above 2^53 (2^53+1 → 2^53), no bounds checks (a truncated string clamps silently), and its generated readers cannot represent proto3optionalpresence — the schema uses that forns/parent/anchor, and real streams containInsertBefore.parent = 0.@bufbuild/protobuf/wire: exact sint64 (forceLong=numberthrows on unsafe values), bounds-checked,optional→undefined, +3–4 KB gzipped.Measured over the bench producers' real captured streams (19, every op) into the same
FrameSink(harness kept untracked underbench/decoders/; three runs, geomean stable to ±0.04):The hand-rolled decoder loses on allocation (two
Readers, a tuple per tag, a closure per frame), not on the wire format. Generated readers everywhere is both fastest-correct and the least hand-written code; the live receiver after this PR measures ~0.8x the old decoder's time on the same harness (withunknownFields=truefor strict mode).Codegen
buffrom npm runs under Deno and spawns the ts-proto plugin throughdeno run— no Node, no protoc (buf.yaml,buf.gen.yaml,just proto-ts).receiver/src/gen/*.tsis committed;just checkregenerates into a temp dir and diffs (just proto-drift). The unanchoredgen/.gitignorerule (forhost/desktop/gen) is anchored so the generated code can be tracked.Semantics preserved / changed
FrameSinkand its exported types are unchanged.push,MAX_FRAME_BYTES, truncated length prefix = wait vs malformed = error (5-byte u32 bound, now tested at frame level)._unknownFieldswith the sameunknown field N in <Message>errors.SetProperty.intoutsideNumber.isSafeIntegernow throws instead of silently losing bits.Gates
just check(incl. drift gate),deno task test(128),deno fmt --check,cargo test -p stream-dom-guest,just site,just e2e,just bench-wire(byte-identical wire shapes — the encoder side is Rust). Equivalence gate: the live decoder's sink-call trace is identical to a frozen copy of the old decoder over all 19 corpus streams. Independent review: one blocking finding (per-frame bound lost with the shared reader), fixed with tests.deno lint receiverhas one pre-existing failure inpolicy-desktop.ts:476(no-control-regex), untouched here.Automerge armed (merge commit).