Skip to content

fix(parser): handle N3 verb edge cases - #719

Merged
jeswr merged 3 commits into
rdfjs:mainfrom
jeswr:codex/fix-n3-verb-edge-cases
Sep 2, 2026
Merged

fix(parser): handle N3 verb edge cases#719
jeswr merged 3 commits into
rdfjs:mainfrom
jeswr:codex/fix-n3-verb-edge-cases

Conversation

@jeswr

@jeswr jeswr commented Sep 1, 2026

Copy link
Copy Markdown
Member

Summary

  • keep the N3 has, is, and of verbs from preempting longer valid prefixed names, including across stream chunks
  • scope inverse-predicate state to the active predicate-object list: preserve it across commas and reset it across semicolons and annotation boundaries
  • apply inverse direction consistently when closing blank-node property lists and formulas, and when constructing triple terms and reified triples
  • add regression coverage for the cases identified in the Copilot review of #694

This is a follow-up to #694.

Verification

  • 6,876 tests pass
  • 100% statement, branch, function, and line coverage
  • ESLint passes
  • Node, IIFE browser, and ESM browser builds pass

Copilot AI lite review requested due to automatic review settings September 1, 2026 20:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The changes are cohesive, direction handling is consistently applied via a single quad creation path, and the added tests directly cover the reported edge cases and nesting/punctuation boundaries.

Pull request overview

This PR tightens N3 parsing/tokenization around the has, is, and of verbs to avoid keyword preemption of longer prefixed names (including across stream chunks) and to correctly scope/apply inverse-predicate state across punctuation and nested constructs (blank node property lists, formulas, triple terms, and reified triples), with regression tests added for the edge cases.

Changes:

  • Add lexer logic to prefer longer prefixed names over N3 verb keywords when the verb is a prefix-start (including split chunk scenarios), while still recognizing numeric-boundary cases like has1.
  • Refine parser inverse-predicate state handling to persist across commas, reset across semicolons/annotation boundaries, and apply inversion consistently when closing nested scopes and constructing triple terms / reified triples.
  • Add regression coverage for inverse-predicate scoping and verb/prefix tokenization edge cases.
File summaries
File Description
src/N3Lexer.js Adds _matchN3Verb to avoid verb keyword tokenization when a longer prefixed name is possible, including across stream chunks.
src/N3Parser.js Centralizes direction-aware quad creation/emission and resets/scopes inverse-predicate state across punctuation and nested constructs.
test/N3Lexer-test.js Adds lexer regression tests for numeric boundaries and keyword-like prefixes (including chunk-splitting).
test/N3Parser-test.js Adds parser regression tests for inverse scoping across , / ;, blank node property lists, formulas, triple terms, reification, and annotations.
Review details
  • Files reviewed: 4/4 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.

@jeswr

jeswr commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

Performance follow-up for the inverse-handling concern:

I benchmarked main (ffb71f9) against this branch with Node 24.19.0. The harness alternated base/candidate order, used four fresh-process pairs with 20 samples per process, and included A/A (main versus main) controls. Parser workloads streamed 30,000 quads per sample; the isolated _emit workload created 2,000,000 quads per sample.

The initial implementation routed every _emit through _createQuad and showed a 5–8% paired-median slowdown in isolated emission. Commit 99d1e11 now installs the direction-aware _createQuad and _emit implementations only on N3 parser instances. For non-N3 formats, ordinary _emit remains the same direct factory call as main; blank-node and formula tails use a four-argument alias to that same method. This centralizes inverse swapping without putting inverse checks or a fifth argument on non-N3 emission paths.

Latest results (negative values are faster):

Workload Aggregate median Paired median A/A paired control
Isolated _emit -1.9% -0.7% +0.1%
Turtle -4.3% -2.9% +4.9%
N-Triples +0.7% +3.4% +2.3%
Turtle with blank-node property lists -4.8% -4.7% +3.6%
Turtle with RDF 1.2 triple terms -0.1% -2.2%

There is no measured non-N3 regression beyond the process/JIT/GC noise visible in the A/A controls. The only positive aggregate result is N-Triples at +0.7%; its paired result is 1.1 percentage points above its +2.3% A/A control, while its ordinary emission code is source-identical to main.

Verification on the revised branch: 6,876 tests pass with 100% statement/branch/function/line coverage; ESLint and the Node, IIFE, and ESM builds pass.

@jeswr
jeswr merged commit 30c108e into rdfjs:main Sep 2, 2026
55 checks passed
@jeswr
jeswr deleted the codex/fix-n3-verb-edge-cases branch September 2, 2026 15:30
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 2.7.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants