fix(lexer): report precise reusable source ranges - #721
Conversation
|
@ericprud, would you mind reviewing this together with This is the lower-level lexer change that the provenance work will build on. In particular, The provenance PR then consumes these corrected line/column ranges to describe |
There was a problem hiding this comment.
🔵 Needs a closer look
The changes modify core lexer coordinate semantics and streaming edge-case handling in ways that are difficult to exhaustively validate from the diff alone.
Pull request overview
This PR refines Lexer token coordinates to represent exact half-open lexical source ranges (with end excluding separator whitespace and endLine for multiline tokens), and resets lexer state so a single Lexer instance can be safely reused after successful or failed tokenization.
Changes:
- Update
src/N3Lexer.jsto track physical line-relative columns (including indentation and BOM), avoid synthetic EOF lookahead extending token ranges, and supportendLinefor multiline literals. - Add/adjust lexer tests to cover BOM handling, multiline literal coordinates (including stream-split and CR/LF variants), CRLF split across stream chunks, comment stability, and lexer reuse after errors.
- Update parser tests to reflect the new coordinate contract (notably
endnow excluding trailing whitespace).
File summaries
| File | Description |
|---|---|
| test/N3Parser-test.js | Updates expected parser error-context token ranges to match the new half-open/lexical coordinate contract. |
| test/N3Lexer-test.js | Expands and adjusts coordinate-focused tests (BOM, multiline, CRLF chunk splits, synthetic lookahead, reuse-after-error) to validate the new behavior. |
| src/N3Lexer.js | Implements precise, reusable, line-relative coordinate tracking (start/end, optional endLine), including CRLF boundary handling and state reset per tokenization. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
f575982 to
3698e7f
Compare
55b773e to
0fb0c2a
Compare
0fb0c2a to
27a5c0b
Compare
Check whether a comment consumes the full chunk before inspecting its final character, using the existing CR code. Reuse the matched comment and normalized prefix lengths, and simplify the empty-buffer column assignment. Preserve CRLF buffering and exact token coordinates. Validated with 6,944 tests at 100% coverage, lint, Node/browser builds, and generated coordinate and streaming checks. Benchmarks on Node 24 and 25 did not establish a general end-to-end speedup.
|
🎉 This PR is included in version 2.7.12 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Make lexer coordinates describe exact, half-open lexical source ranges and make one
Lexerinstance safe to reuse after completed or failed tokenization. This extracts the general lexer work from jeswr#20, including the coordinate foundation from jeswr#16, so it can land before the provenance parser is rebased.Rebased on main
0777d2e, including the merged #726 direction markers, #727 separator dispatch, #728 literal delimiters, and #729 fixed-token matching. The general separator optimization remains in #727; this PR adds coordinate bookkeeping to that scanner and retains its direct character dispatch and native scanning of long runs.lineand optionalendLineare one-based physical source lines.startandendare zero-based, end-exclusive UTF-16 columns, matching JavaScript string indexes.endexcludes separator whitespace; that whitespace counts toward the next token'sstart.endLine, withendrelative to that final line.This corrects indentation and multiline columns, comment ranges, synthetic EOF lookahead extending token ranges, and CRLF split across chunks. Each tokenization resets syntax markers, previous-token, literal-search, input, and line state. Parsed RDF output is unchanged; directly exposed token coordinates and parser error context are affected.
Raw captures provide constant-time lexical lengths for IRIs, blank nodes, and prefixed names. The IRI length adds two for
<and>. Multiline tokens are built with their final fields at the emission site. JavaScript slicing bounds input consumption, while raw lexical lengths keep emitted ranges within the actual token.The separator scanner carries consumed indentation into the next line's column count. Comment ranges start at
#and exclude the line ending and surrounding whitespace. A CR at the end of a stream chunk remains buffered until it can be distinguished from CRLF, both for ordinary line endings and comments. Pending comments and whitespace-only chunks retain their physical columns.Validation: 6,944 tests pass with 100% statement, branch, function, and line coverage, plus ESLint, Node/browser IIFE/ESM builds, and diff checks. Existing tests from all merged optimizations are retained alongside the range and reuse tests. Additional validation covers 1,000 generated documents, 7,073 independently calculated token coordinates, 28,475 two-chunk stream splits, 100 one-character streams, and 28 lexer-reuse pairs. Complete token objects and callback timing match the rebased #721 head
27a5c0b. Independent review found no actionable issues in the follow-up changes.Performance follow-up (
e99b7db): cache the matched comment length, check whether it reaches the end of the buffer before inspecting its last character withcharCodeAtand the existingCRconstant, reuse normalizedprefix.length, and simplify the empty-buffer column assignment. CRLF buffering and all token coordinates are preserved. These remove redundant work; the end-to-end speed differences are inconclusive.Benchmarks compared main
0777d2e, the original rebased PR27a5c0b, and the final cleanup on Node 25.1.0/macOS arm64. The table gives paired geometric-mean wall-time changes and exploratory 95% intervals; positive is slower. The two comparisons were run separately. Dense N-Triples parsing did not reproduce the earlier general +11% slowdown. The LF-comment lexer fixture still shows overhead relative to main in this run; this does not establish a corresponding full-parser regression.The broader candidate that moved multiline bookkeeping into token emission was rejected: the combined patch slowed multiline lexing by 10.2% wall time (95% interval +3.2% to +17.6%) and 8.7% CPU time (+2.6% to +15.2%) relative to the original PR, with additional slowdowns in streamed dense lexing and ignored-comment lexing. Multiline emission retains its original structure.
Method: production Babel builds; 8,000 triples per fixture (16,000 for prefixed Turtle and 1,500 for the small-chunk control); 64 KiB stream chunks, plus an 8-character CRLF boundary control; fresh process per variant/workload/round with rotating order, at least 500 ms warm-up and 750 ms measurement. Five paired rounds covered 18 baseline workloads and 13 final-cleanup workloads. A three-round Node 24.12.0 check of eight selected workloads also found no conclusive end-to-end improvement from the final cleanup. Full semantic digests match main, and complete token digests match the original PR. Tests, builds, and benchmarks ran separately. Allocation and GC variation is substantial in some synchronous workloads, so these synthetic results cannot establish a universal speedup or absence of regression.
The exact published cleanup also passes the full 6,944-test suite at 100% coverage, lint, Node/browser builds, and the generated coordinate and streaming checks above. No further within-scope optimization is recommended from this evaluation.