Skip Ohm.js parsing large bodies#8739
Conversation
The v2 parser ran the ohm grammar character-by-character over the entire file, including large opaque body/script/docs/tests content the grammar returns verbatim — which can dominate parse time. Hoist each text block out behind a sentinel before matching, parse the small skeleton, then splice the real content back into the AST, with a safe fallback to a full parse if anything is unexpected. Applied from the bruno-parser-performance working tree. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add tests for the skip-ohm-parse path in bruToJsonV2: - a ~1MB JSON body parses in <1s (the full grammar parse is ~3s and grows super-linearly) and the body round-trips exactly, demonstrating why the optimization is needed - large pretty-printed JSON output matches a plain parse, with no sentinel placeholder leaking into the result - multiple text blocks (body/script/docs) in one file are all restored - CRLF line endings inside a text block are handled - duplicate text blocks (valid .bru) drive the fallback path: only the reducer's surviving block keeps its sentinel, so the splice count no longer matches and the parser falls back to a full parse, still producing the stock result Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WalkthroughThe parser now hoists text blocks into sentinels before building the AST, restores their contents afterward, and falls back to full parsing when optimization validation fails. Tests cover performance, formatting, multiple blocks, CRLF input, and fallback behavior. ChangesText block parsing optimization
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Parser
participant TextBlockExtractor
participant ASTBuilder
participant ASTSplicer
Parser->>TextBlockExtractor: Extract text blocks into sentinels
TextBlockExtractor-->>Parser: Return skeleton and block map
Parser->>ASTBuilder: Parse skeleton
ASTBuilder-->>Parser: Return AST
Parser->>ASTSplicer: Restore text block contents
ASTSplicer-->>Parser: Return spliced AST or replacement count
Parser->>ASTBuilder: Full-parse original input on validation failure
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/bruno-lang/v2/tests/bruToJson.spec.js (1)
1171-1202: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood fallback coverage, but only exercises one of the two documented fallback triggers.
This test covers the "sentinel count mismatch" trigger (duplicate blocks dropped by the reducer).
bruToJson.js's own comment also calls out a second, distinct trigger — an opener nested inside anexampleblock hitting thecatchbranch when the skeleton fails to parse (or lands the sentinel inside a larger string rather than as a standalone value). Worth a companion test for that path since it's explicitly documented as a known edge case.🧪 Suggested additional test
it('falls back to a full parse when a text-block opener appears nested inside an example block', () => { const input = `get { url: https://example.com } example { docs { nested, opaque to the grammar } } `; const output = parser(input); // Whatever the exact shape, it must match a plain full parse exactly. expect(output).toEqual(parser.__proto__ ? output : output); });(Adjust the exact assertion to whatever
exampleblock parsing produces for this input — the key property to pin is that hoisting doesn't change the result versus a stock parse.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-lang/v2/tests/bruToJson.spec.js` around lines 1171 - 1202, Add a companion test in the bruToJson parser test suite for the fallback path where a text-block opener is nested inside an example block and skeleton parsing fails or produces a non-standalone sentinel. Use a representative input containing an example block with nested docs content, then assert the optimized parser output matches the equivalent full-parse result, preserving the existing duplicate-block sentinel-mismatch test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/bruno-lang/v2/tests/bruToJson.spec.js`:
- Around line 1171-1202: Add a companion test in the bruToJson parser test suite
for the fallback path where a text-block opener is nested inside an example
block and skeleton parsing fails or produces a non-standalone sentinel. Use a
representative input containing an example block with nested docs content, then
assert the optimized parser output matches the equivalent full-parse result,
preserving the existing duplicate-block sentinel-mismatch test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 68625180-424f-425b-9266-a6cb7b8fcf1e
📒 Files selected for processing (2)
packages/bruno-lang/v2/src/bruToJson.jspackages/bruno-lang/v2/tests/bruToJson.spec.js
1 participant
Description
The bru parser (bru-lang) is not performant when parsing large .bru files, especially due to large text blocks. For example, including Base64 data in an HTTP POST request makes the .bru parser crawl.
Since this text is not actually parsed by Ohm.js and is just forwarded as a string (
outdentString(sourceString)), we can do a preprocessing step to remove large text blocks from .bru files, then parse with Ohm, and re-insert the text blocks. This improves parsing of small files slightly, and large files by many orders of magnitude. The parser falls back to the old (slow) method whenever necessary, adding a negligible time penalty in those cases.Contribution Checklist:
Potentially related issues:
#8529
#2416
#5840
Benchmarks / proof:
Also adds tests to measure parsing speed, and ensure the new parsing behavior is identical to the old.
Summary by CodeRabbit
Performance
Bug Fixes