Skip to content

feat(runtime): add ByteStream for binary parsing + MIDI example - #188

Merged
tinovyatkin merged 2 commits into
mainfrom
feat/binary-parsing-bytestream
Jul 24, 2026
Merged

feat(runtime): add ByteStream for binary parsing + MIDI example#188
tinovyatkin merged 2 commits into
mainfrom
feat/binary-parsing-bytestream

Conversation

@tinovyatkin

@tinovyatkin tinovyatkin commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds first-class support for binary / byte-oriented parsing, closing #181.

  • ByteStream — a new byte-oriented CharStream (src/byte_stream.rs, exported from lib.rs). Each byte is one symbol in 0..=255, the stream index is the byte offset, and lookahead returns the byte value. Grammars use the Latin-1 convention (BYTE : ' ' .. 'ÿ';) that ANTLR's reference runtimes use for binary input — but without the Vec<char> + offset-table transcoding InputStream incurs on non-UTF-8 bytes.
  • A worked MIDI example — a Standard MIDI File grammar + a small SemanticHooks "superClass" + an integration test that parses a real .mid over a ByteStream.

ByteStream design

Generic over B: AsRef<[u8]> (default Vec<u8>) so one type maps onto every Rust IO shape:

Source Call Cost
Owned bytes ByteStream::new(vec) moves
Network/read buffer you hold ByteStream::new(&buf[..]) zero-copy borrow
File / socket / stdin / Cursor ByteStream::from_reader(r)? one drain via io::Read

Because the bytes aren't text, text() renders a matched span as lowercase hex. The ASCII fast path (contiguous_ascii) is deliberately not implemented — it indexes a 128-wide DFA row, so bytes >= 0x80 route correctly through the generic wide_rows path instead. (Verified against src/atn/lexer_dfa.rs.)

MIDI example (tests/fixtures/antlr4-rust-gen/midi-binary/)

Length-prefixed formats ("read N, then consume N bytes") are data-dependent, so a pure CFG can't frame them — the same constraint ANTLR's bencoding grammar solves with a lexer superClass. Here that role is a ~20-line MidiHooks implementing SemanticHooks:

  • begin_chunk reads the MThd/MTrk declared length (raw bytes via la() lookbehind) and arms a countdown.
  • lexer_before_token emits a synthesized END_OF_CHUNK once the body is consumed, via enqueue_token + pop_mode.

The bare {beginChunk();} lexer action lowers to a typed hook method through a --sem-patterns [[helper]] entry (kind = "lexer-action", lower = "hook"). Scope is deliberately small: MThd/MTrk chunks, VLQ delta-times, note-on/off, set-tempo, end-of-track. Grammar adapted (simplified) from milnet2/midi-grammar by Tobias Blaschke (BSD-3-Clause, attributed in the grammar header and README).

The twinkle.mid fixture is validated by file(1) as "Standard MIDI data (format 0) using 1 track at 1/96"; make_fixture.py keeps its bytes auditable.

Test

midi_binary_grammar_parses_standard_midi_file_over_byte_stream (in tests/antlr4_rust_gen_cli.rs) generates the recognizer at test time via antlr4-rust-gen (no ANTLR jar, no network), compiles a crate defining MidiHooks, and parses the fixture over a ByteStream — asserting 0 syntax errors and the exact token-type sequence.

Verification

  • cargo test --locked --features codegen — all pass (278 + 3 + 732 + 26).
  • cargo clippy --locked --all-targets --all-features -- -D warnings — clean.
  • Touched files cargo fmt-clean; no unrelated formatting churn.

Closes #181.

Summary by CodeRabbit

  • New Features

    • Added ByteStream support for parsing binary data from byte arrays or readers.
    • Added byte-oriented lookahead, seeking, EOF detection, and lowercase hexadecimal token text.
    • Added support for length-prefixed binary framing through generated lexer hooks.
    • Added a complete Standard MIDI File parsing example and integration coverage.
  • Documentation

    • Documented binary parsing workflows, stream handling, framing hooks, and the MIDI example.

Introduce `ByteStream`, a byte-oriented `CharStream` for parsing binary
formats. Each byte is one symbol in `0..=255` and the stream index is the
byte offset, so grammars can use the Latin-1 convention (`BYTE: ' '..'ÿ'`)
that ANTLR's reference runtimes use for binary input, without the transcoding
and per-byte allocation `InputStream` incurs on non-UTF-8 bytes.

`ByteStream<B = Vec<u8>>` is generic over `AsRef<[u8]>` to map onto Rust IO
primitives: `new(vec)` owns, `new(&buf[..])` borrows a network buffer
zero-copy, and `from_reader(r)?` drains any `std::io::Read` (files, sockets,
stdin, `Cursor`). Because the bytes are not text, `text()` renders a matched
span as lowercase hex. The ASCII fast-path (`contiguous_ascii`) is
deliberately not implemented: it feeds a 128-wide DFA row, so bytes >= 0x80
route through the generic path instead.

Add a worked binary-parsing example under
tests/fixtures/antlr4-rust-gen/midi-binary/: a Standard MIDI File grammar
(MThd/MTrk chunks, VLQ delta-times, note and meta events) plus a small
`SemanticHooks` chunk-framing "superClass" that counts down each chunk's
declared byte length and synthesizes END_OF_CHUNK -- the data-dependent
"read N, then N bytes" pattern ANTLR's bencoding grammar solves the same way.
The bare `{beginChunk();}` lexer action lowers to a typed hook via a
--sem-patterns helper. An integration test generates the recognizer from the
committed grammar and parses a real .mid fixture over a ByteStream with zero
syntax errors.

The grammar is adapted (simplified) from milnet2/midi-grammar by Tobias
Blaschke (BSD-3-Clause). README gains a "Binary and Byte-Oriented Parsing"
section documenting the approach.
@github-actions

Copy link
Copy Markdown

Copy/Paste Detection

Found 1 duplication(s) across 3 changed Rust file(s) (threshold: 100 tokens).

Show duplications

Found a 22 line (132 tokens) duplication in the following files:

  • Starting at line 965 of tests/antlr4_rust_gen_cli.rs
  • Starting at line 1065 of tests/antlr4_rust_gen_cli.rs
        "parser grammar Delegate;\ndelegated: {isTypeName()}? ID;\n",
    )
    .expect("delegate grammar should be writable");
    fs::write(&tokens, "lexer grammar Tokens;\nID: [a-z]+;\n")
        .expect("token grammar should be writable");

    let output = run_antlr4_rust_gen(&[
        root.as_os_str(),
        tokens.as_os_str(),
        OsStr::new("-I"),
        temp.path().as_os_str(),
        OsStr::new("--out-dir"),
        out.as_os_str(),
    ]);
    assert!(
        output.status.success(),
        "stdout: {}\nstderr: {}",
        utf8(&output.stdout),
        utf8(&output.stderr)
    );
    let parser = fs::read_to_string(out.join("root.rs")).expect("parser should be emitted");
    assert!(parser.contains("pub trait RootHooks"), "{parser}");
```rust

</details>

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@tinovyatkin, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 12 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1b4070bb-f765-4904-84dc-2470f47e4fad

📥 Commits

Reviewing files that changed from the base of the PR and between 8a5422b and b789807.

📒 Files selected for processing (4)
  • src/byte_stream.rs
  • tests/fixtures/antlr4-rust-gen/midi-binary/LICENSE-midi-grammar
  • tests/fixtures/antlr4-rust-gen/midi-binary/MidiLexer.g4
  • tests/fixtures/antlr4-rust-gen/midi-binary/MidiParser.g4
📝 Walkthrough

Walkthrough

Adds a public ByteStream for byte-oriented ANTLR parsing, implements binary stream and token-text behavior, and adds a MIDI lexer/parser example using semantic hooks for chunk framing with end-to-end integration coverage.

Changes

Binary parsing support

Layer / File(s) Summary
ByteStream API and runtime behavior
src/byte_stream.rs, src/lib.rs
Adds owned and borrowed byte streams, reader construction, byte-based lookahead and seeking, hexadecimal token text, interval mapping, EOF handling, unit tests, and crate-root exports.
MIDI grammar and fixture
tests/fixtures/antlr4-rust-gen/midi-binary/*
Adds MIDI lexer modes and event rules, parser productions, semantic-hook configuration, fixture generation, and the twinkle.mid binary fixture.
Generated MIDI integration test
tests/antlr4_rust_gen_cli.rs, README.md
Generates the MIDI parser with typed hooks, frames chunks using lookbehind and synthesized tokens, validates parsed token types, and documents binary parsing usage.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ByteStream
  participant GeneratedMidiLexer
  participant MidiLexerHooks
  participant CommonTokenStream
  participant GeneratedMidiParser
  ByteStream->>GeneratedMidiLexer: provide byte lookahead
  GeneratedMidiLexer->>MidiLexerHooks: invoke begin_chunk and lexer_before_token
  MidiLexerHooks->>GeneratedMidiLexer: emit END_OF_CHUNK
  GeneratedMidiLexer->>CommonTokenStream: enqueue tokens
  CommonTokenStream->>GeneratedMidiParser: provide MIDI tokens
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly highlights the main changes: ByteStream binary parsing support and the MIDI example.
Linked Issues check ✅ Passed The PR implements binary-file parsing support with ByteStream and a worked MIDI example, matching #181's request.
Out of Scope Changes check ✅ Passed All changes support binary parsing or the MIDI example; no unrelated code changes stand out.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/binary-parsing-bytestream

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown

Claude Code is working…

I'll analyze this and get back to you.

View job run

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown

📊 Source Code Metrics (this PR vs main)

File Cyclomatic Cognitive Functions LLOC MI
src/byte_stream.rs 44 🆕 15 🆕 22 🆕 98 🆕 11.27 🆕
src/lib.rs 4 ⚪ 3 ⚪ 1 ⚪ 7 ⚪ 33.59 (main: 33.91) 🔴

Generated by mehen v1.7.0 — the code quality watcher.

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.38554% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/byte_stream.rs 96.38% 6 Missing ⚠️

📢 Thoughts on this report? Let us know!

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8a5422b0fa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/byte_stream.rs
Comment thread src/byte_stream.rs Outdated
Comment thread src/byte_stream.rs Outdated
Comment thread tests/fixtures/antlr4-rust-gen/midi-binary/MidiLexer.g4 Outdated
- byte_stream: clamp interval `stop` before the `+1` in `text` and
  `byte_interval`, mirroring `InputStream`, so a `usize::MAX` span (e.g. an
  EOF-token interval) can no longer overflow. Empty/out-of-range now returns
  `None`/`""` consistently.
- byte_stream: implement `position_summary` by scanning the raw bytes. Without
  it, `BaseLexer::position_at` fell back to iterating the hex `text()`, so N
  bytes counted as 2N columns and `0x0A` newlines were invisible — corrupting
  line/column for split or synthesized tokens.
- byte_stream: keep the module-doc example grammar-agnostic (`FooLexer`, per
  AGENTS.md), pointing to the MIDI fixture for the worked example.
- midi fixture: carry the upstream milnet2/midi-grammar BSD-3-Clause notice
  verbatim (LICENSE-midi-grammar) and reference it from both grammar headers,
  satisfying the source-redistribution condition.

Adds unit tests for the usize::MAX clamp and raw-byte position summaries.
@tinovyatkin

Copy link
Copy Markdown
Contributor Author

@codex review

All four findings from the previous review are addressed in b789807:

  • Clamp before +1text/byte_interval now clamp stop to len - 1 first (mirrors InputStream); regression test added.
  • Raw-byte position_summary — implemented; no more hex-text column doubling / invisible newlines; test added.
  • Grammar-agnostic runtime docs — module example is now FooLexer/FooParser per AGENTS.md, pointing to the MIDI fixture.
  • Upstream BSD-3 noticeLICENSE-midi-grammar carries the full upstream copyright + conditions + disclaimer, referenced from both grammar headers.

@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown

Claude finished @tinovyatkin's task in 12m 16s —— View job


Code Review — PR #188: ByteStream for binary parsing + MIDI example

Verdict: ✅ No blocking issues found. Checked for bugs and CLAUDE.md compliance.

  • Gather context (diff, CLAUDE.md, source files)
  • Review src/byte_stream.rs (core new code)
  • Review MIDI grammar/test/fixtures
  • Check CLAUDE.md compliance
  • Validate candidate findings
  • Post consolidated review

I reviewed with four independent passes (two CLAUDE.md-compliance, two bug/correctness), cross-checking ByteStream against the reference InputStream in src/char_stream.rs. This is a clean, well-tested addition and the four prior Codex findings (clamp-before-+1, raw-byte position_summary, grammar-agnostic docs, upstream BSD-3 notice) are all present.

What I verified

src/byte_stream.rs — correct

  • la() (131-149) is a faithful mirror of InputStream::la (char_stream.rs:181-198): the positive branch cursor.checked_add((offset - 1).cast_unsigned()) is safe because offset > 0 ⇒ offset - 1 ≥ 0; the negative branch routes isize::MIN (checked_neg) and under-cursor lookbehind (checked_sub) to EOF instead of panicking. la(isize::MIN) → EOF, no panic.
  • text() (171-194) guards len == 0 before len - 1, clamps stop to len - 1 before the inclusive slice, and the start > stop guard prevents any out-of-range bytes[start..=stop] — including the stop == usize::MAX EOF-span case. Hex capacity (stop - start + 1) * 2 is exact and {byte:02x} renders correct lowercase.
  • symbol_at uses i32::from(byte)0..=255 with no sign extension (high byte 0x80128, pinned by a test).
  • byte_interval clamps before +1 (so stop + 1 ≤ len, no overflow); position_summary scans raw bytes and counts 0x0A. Leaving contiguous_ascii unimplemented is load-bearing and correct — the default None forces bytes ≥ 0x80 through the wide-row path instead of the 128-wide ASCII DFA.

MIDI example — correct

  • make_fixture.py byte math checks out: MThd body = 6 bytes, MTrk track body = 4 + 4 + 7 + 4 = 19 bytes, total file 14 + 27 = 41 bytes (matches the committed fixture). The begin_chunk big-endian length decode over la(-4..-1) yields the declared 6 / 19, and the asserted token-type vector matches ANTLR's numbering order (END_OF_CHUNK=1 from tokens{}, then rules in file order, fragments excluded).
  • The lexer's DELTA↔EVENT mode split correctly resolves the status-byte / VLQ-continuation-byte ambiguity in 0x80..0xFF.

CLAUDE.md compliance — clean

  • cargo fmt --check is clean on the three touched .rs files. src/byte_stream.rs keeps grammar-specific material out of the runtime module (MIDI appears only in module docs pointing at the fixture, and in #[cfg(test)]), satisfying AGENTS.md. No non-deterministic snapshots; no hand-grouped data needing #[rustfmt::skip].

Optional (non-blocking) nit

src/byte_stream.rs:251 — the #[allow(clippy::disallowed_methods)] // insta assertion macros unwrap internal I/O. on the tests module is unnecessary: this module calls no insta macros and no .unwrap() (only .expect()/.unwrap_or(), neither of which is in .clippy.toml's disallowed-methods), so the lint never fires. It's harmless — an unfulfilled allow won't fail CI — but the comment is slightly misleading. Safe to drop the attribute, or leave it; either is fine. Fix this →

Nice work on the design writeup and the auditable fixture generator.
· branch feat/binary-parsing-bytestream

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: b7898072be

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@tinovyatkin
tinovyatkin merged commit 60a92be into main Jul 24, 2026
19 checks passed
@tinovyatkin
tinovyatkin deleted the feat/binary-parsing-bytestream branch July 24, 2026 16:19
@ophiarch ophiarch Bot mentioned this pull request Jul 24, 2026
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.

Parsing binary files

1 participant