fix(source): stop fabricating parse positions; --lang is a fallback in corpus walks (#90) - #100
Conversation
…n corpus walks Two defects from #90, plus a stack overflow the first fix exposed. --- Parse positions past EOF vajra-source serialised the CST to a JSON string and re-parsed it to build the Document. When that re-parse hit serde_json's recursion limit, parse_str reported e.column() as byte_offset — an offset into the 374,597-byte intermediate string, attributed to the 50,646-byte source file: parse error at byte 202923: recursion limit exceeded at line 1 column 202923 in a 968-line file whose longest line is 142 characters. An error position that cannot exist is worse than none: it sends the reader somewhere unverifiable. vajra-core gains parse_value, building a Document from an in-memory value, and vajra-source uses it. No round trip, no intermediate offsets, and no wasted copy of the CST. raw_size_bytes now reports the source length rather than the serialisation length, which is what the field is documented to mean. --- Unbounded recursion, previously masked Removing the round trip removed an accidental guard: serde_json's recursion limit was the only thing stopping node_to_json from overflowing the stack. A 400-level Python file aborted the test process outright. node_to_json now carries a depth and stops at MAX_CONVERT_DEPTH, marking the node `truncated` with its child count rather than emitting something indistinguishable from a leaf. The cap is 120, not 256: each CST level becomes two JSON levels, so a higher cap converts a deep file successfully only for `walk` to reject the whole document at 257 — strictly worse than analysing it with a marked cut. --- --lang overrode every file in a corpus walk A corpus crosses languages, and forcing one grammar onto all of it mis-parsed the rest: `--corpus --lang javascript` over a tree of 42 .js and 6 .py parsed the Python as JavaScript, errored, and dropped it. In a directory walk --lang is now a fallback for unrecognised extensions; a recognised extension wins. It still selects the grammar for a single named file, where it is an instruction rather than a default. On the tree that surfaced this: 1 error and 50 documents -> 0 errors and 51. Not changed: --corpus still requires --input-format source to index source files. Making it implicit would silently stop indexing .json, since the walk selects one or the other, never both. Closes #90
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
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 |
1 participant
Two defects from #90 — plus a stack overflow that fixing the first one exposed.
1. Parse positions that cannot exist
vajra-sourceserialised the CST to a JSON string and re-parsed it to build theDocument. When that re-parse hit serde_json's recursion limit,parse_strreportede.column()asbyte_offset— an offset into the 374,597-byte intermediate string, attributed to the user's 50,646-byte file:152,277 bytes past EOF, at "line 1 column 202923", in a 968-line file whose longest line is 142 characters.
line 1because the intermediate JSON has no newlines.vajra-coregainsparse_value, which builds aDocumentfrom an in-memory value;vajra-sourceuses it. No round trip, no intermediate offsets, and one fewer full copy of the CST.raw_size_bytesnow reports the source length instead of the serialisation length — which is what the field's own doc comment ("size in bytes of the raw input") always claimed.2. Unbounded recursion, previously masked by the bug
Removing the round trip removed an accidental guard. serde_json's recursion limit was the only thing stopping
node_to_jsonfrom overflowing the stack:I could not ship a change that turns an error into a crash.
node_to_jsonnow carries a depth and stops atMAX_CONVERT_DEPTH, marking the nodetruncatedwith its child count rather than emitting something indistinguishable from a genuine leaf.The cap is 120, not 256. Each CST level becomes two JSON levels (a
childrenarray wrapping an object), so a higher cap converts a deep file successfully only forwalkto reject the entire document at depth 257 — strictly worse than analysing it with a marked cut. The first value I tried was 256, and the test caught exactly that.3.
--langoverrode every file in a corpus walkIn a directory walk
--langis now a fallback for unrecognised extensions; a recognised extension wins. It still selects the grammar for a single named file, where the user is pointing at one input and the flag is an instruction rather than a default — there is a test for both halves.Tests
vajra-cli/tests/corpus_lang.rs:an_explicit_lang_does_not_override_a_recognised_extension— zero errors, all three files indexed, and the result is identical to indexing with no--langat allan_explicit_lang_still_applies_to_a_single_named_file— the same.pyfingerprints differently under--lang pythonand--lang javascriptraw_size_bytes_is_the_source_file_size— compared againststd::fs::metadatavajra-source:a_reported_byte_offset_never_exceeds_the_input— the invariant the issue asked for; this is the test that found the stack overflowdeep_nesting_is_truncated_visibly_rather_than_overflowingraw_size_bytes_is_the_source_lengthDeliberately not changed
--corpusstill requires--input-format sourceto index source files. Making it implicit would silently stop indexing.json, because the walk selects one or the other and never both. The existing error message already names the flag to pass.Gate
cargo test --workspaceclean,cargo clippy --workspace --all-features -- -D warningsclean,cargo fmt --checkclean.Found by dogfooding vajra against a trending repo.
Closes #90