Add JSONL runtime parsing and module imports#182
Merged
Conversation
- Add `JSONL.parse` and `JSONL.parseChunk` built-ins - Support `.jsonl` module exports and loader integration - Document JSONL behavior and add end-to-end tests
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 3 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds JSONL (newline-delimited JSON) support: a line-aware parser with chunked parsing, a runtime Changes
Sequence Diagram(s)sequenceDiagram
participant Script as Script (caller / test)
participant Builtin as JSONL Builtin
participant Parser as TGocciaJSONLParser
participant JSON as TGocciaJSONParser
Script->>Builtin: JSONL.parse(text or Uint8Array)
Builtin->>Parser: Parse(input)
Parser->>JSON: Parse each non-empty line
JSON-->>Parser: parsed value / parse error
Parser-->>Builtin: TGocciaArrayValue or raise EGocciaJSONLParseError
Builtin-->>Script: return array (or throw SyntaxError with "JSONL line N")
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
Suite Timing
Measured on ubuntu-latest x64. |
Contributor
Benchmark Results274 benchmarks Interpreted: 🟢 78 improved · 🔴 44 regressed · 152 unchanged · avg +0.2% arraybuffer.js — Interp: 🟢 9, 5 unch. · avg +1.3% · Bytecode: 🟢 6, 🔴 2, 6 unch. · avg +1.7%
arrays.js — Interp: 🟢 7, 🔴 3, 9 unch. · avg +0.3% · Bytecode: 🟢 3, 🔴 6, 10 unch. · avg -0.0%
async-await.js — Interp: 🟢 1, 🔴 1, 4 unch. · avg -0.3% · Bytecode: 🟢 5, 1 unch. · avg +4.0%
classes.js — Interp: 🟢 10, 🔴 2, 19 unch. · avg +0.6% · Bytecode: 🟢 2, 🔴 4, 25 unch. · avg -0.7%
closures.js — Interp: 🟢 5, 6 unch. · avg +1.2% · Bytecode: 🔴 7, 4 unch. · avg -3.7%
collections.js — Interp: 🟢 4, 8 unch. · avg +0.7% · Bytecode: 🔴 2, 10 unch. · avg -0.6%
destructuring.js — Interp: 🟢 4, 🔴 3, 15 unch. · avg +0.1% · Bytecode: 🔴 15, 7 unch. · avg -2.5%
fibonacci.js — Interp: 🟢 5, 🔴 1, 2 unch. · avg +1.0% · Bytecode: 🟢 1, 🔴 5, 2 unch. · avg -1.5%
for-of.js — Interp: 🔴 3, 4 unch. · avg -2.2% · Bytecode: 🟢 2, 🔴 3, 2 unch. · avg +0.1%
helpers/bench-module.js — Interp: 0 · Bytecode: 0
iterators.js — Interp: 🔴 7, 13 unch. · avg -1.4% · Bytecode: 🟢 5, 🔴 2, 13 unch. · avg +1.0%
json.js — Interp: 🔴 8, 12 unch. · avg -1.2% · Bytecode: 🟢 3, 🔴 4, 13 unch. · avg -0.7%
jsx.jsx — Interp: 🟢 3, 🔴 7, 11 unch. · avg -0.7% · Bytecode: 🟢 9, 🔴 5, 7 unch. · avg +1.3%
modules.js — Interp: 🟢 3, 6 unch. · avg +1.0% · Bytecode: 🔴 2, 7 unch. · avg -1.1%
numbers.js — Interp: 🟢 9, 2 unch. · avg +2.7% · Bytecode: 🟢 2, 🔴 3, 6 unch. · avg -0.7%
objects.js — Interp: 🔴 2, 5 unch. · avg -1.1% · Bytecode: 🟢 2, 🔴 2, 3 unch. · avg +0.1%
promises.js — Interp: 🟢 3, 9 unch. · avg +0.6% · Bytecode: 🟢 1, 🔴 1, 10 unch. · avg -0.5%
regexp.js — Interp: 🟢 3, 🔴 2, 6 unch. · avg +0.1% · Bytecode: 🔴 5, 6 unch. · avg -1.9%
strings.js — Interp: 🟢 5, 🔴 1, 5 unch. · avg +1.0% · Bytecode: 🔴 3, 8 unch. · avg -1.8%
typed-arrays.js — Interp: 🟢 7, 🔴 4, 11 unch. · avg +0.7% · Bytecode: 🟢 6, 🔴 8, 8 unch. · avg -0.3%
Measured on ubuntu-latest x64. Benchmark ranges compare cached main-branch min/max ops/sec with the PR run; overlapping ranges are treated as unchanged noise. Percentage deltas are secondary context. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/language/modules/jsonl-import.js (1)
12-26: Add one loader-level blank-line/error case here.This file only exercises happy-path imports/re-exports. The new
.jsonlmodule contract also promises ignored blank lines and line-numbered failures for invalid input, and neither path is exercised end-to-end throughTGocciaModuleLoader.As per coding guidelines, JavaScript tests should cover happy paths, edge cases, and error cases.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/language/modules/jsonl-import.js` around lines 12 - 26, Add tests to cover the loader-level blank-line and failure cases: extend the "JSONL module imports" suite by adding one test that imports a JSONL module containing an empty line and asserts the loader (TGocciaModuleLoader) ignores it (e.g., record count unchanged and labels still correct), and add another test that attempts to load a syntactically invalid JSONL module via TGocciaModuleLoader and asserts the thrown error includes the failing line number (or message matching /line \d+/) to verify line-numbered failures; reference the existing test names and variables (firstRecord, count, labels, reExportedFirstRecord, reExportedCount) when adding these new tests so they integrate with the current fixtures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/built-ins.md`:
- Line 20: The docs snippet is out of sync with the engine defaults: ensure the
registration examples reflect TGocciaEngine.DefaultGlobals and
TGocciaGlobalBuiltin by re-adding ggYAML where it was removed and keeping
ggJSONL present; update the enum/example list and the default-set example so
both include ggYAML (and any other builtins present in
TGocciaGlobalBuiltin/DefaultGlobals) so the documentation matches the actual
defaults.
In `@units/Goccia.Builtins.JSONL.pas`:
- Around line 94-104: ReadUint8ArrayBytes currently throws
EGocciaJSONLParseError for non-Uint8Array typed arrays which gets remapped to a
SyntaxError by the public parse/parseChunk wrappers; change the error to an
argument/type error instead (e.g. throw a distinct
EGocciaInvalidArgument/EGocciaTypeError or create EGocciaJSONLInvalidInputError)
so callers can map it to an argument-type exception rather than
ThrowSyntaxError; update TGocciaJSONLBuiltin.ReadUint8ArrayBytes and the other
JSONL input-validation sites referenced (lines ~128-150 and ~153-217) to raise
that argument/type-specific exception (or ensure the public methods do not remap
that specific exception to ThrowSyntaxError) and keep existing parse error
semantics for real parse failures.
---
Nitpick comments:
In `@tests/language/modules/jsonl-import.js`:
- Around line 12-26: Add tests to cover the loader-level blank-line and failure
cases: extend the "JSONL module imports" suite by adding one test that imports a
JSONL module containing an empty line and asserts the loader
(TGocciaModuleLoader) ignores it (e.g., record count unchanged and labels still
correct), and add another test that attempts to load a syntactically invalid
JSONL module via TGocciaModuleLoader and asserts the thrown error includes the
failing line number (or message matching /line \d+/) to verify line-numbered
failures; reference the existing test names and variables (firstRecord, count,
labels, reExportedFirstRecord, reExportedCount) when adding these new tests so
they integrate with the current fixtures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 690c0cd1-257a-4840-bd70-622b24895e51
📒 Files selected for processing (17)
README.mddocs/built-ins.mddocs/design-decisions.mddocs/language-restrictions.mdtests/built-ins/JSONL/parse.jstests/built-ins/JSONL/parseChunk.jstests/language/modules/helpers/event-stream.jsonltests/language/modules/helpers/jsonl-re-exporter.jstests/language/modules/jsonl-import.jsunits/Goccia.Builtins.JSONL.pasunits/Goccia.Constants.PropertyNames.pasunits/Goccia.Engine.pasunits/Goccia.FileExtensions.pasunits/Goccia.JSONL.pasunits/Goccia.Modules.Loader.pasunits/Goccia.Runtime.Bootstrap.pasunits/Goccia.ScriptLoader.Globals.pas
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
units/Goccia.Builtins.JSONL.pas (1)
175-183: Check forNaN/InfinitybeforeTruncto match codebase patterns.The code calls
Trunc(AArgs.GetElement(n).ToNumberLiteral.Value)without first checking if the value isNaNorInfinity. WhileClampOffsetsafely handles extreme integer values, the codebase consistently checksIsNaNandIsInfinitebefore callingTrunc(seeGlobalArrayBuffer.pas,Goccia.Builtins.Math.pas). This aligns with the documented guidance: "Always check the special value flags first and handle each case explicitly."♻️ Suggested pattern
+ if AArgs.GetElement(1).ToNumberLiteral.IsNaN then + StartOffset := 0 + else if AArgs.GetElement(1).ToNumberLiteral.IsInfinite then + StartOffset := TextLength + else StartOffset := ClampOffset( Trunc(AArgs.GetElement(1).ToNumberLiteral.Value), TextLength)Apply the same check to the end offset (lines 181–185).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@units/Goccia.Builtins.JSONL.pas` around lines 175 - 183, The start and end offset extraction in units/Goccia.Builtins.JSONL.pas calls Trunc on AArgs.GetElement(...).ToNumberLiteral.Value without checking special float flags; update the StartOffset and EndOffset branches to first read the numeric value, test IsNaN and IsInfinite on that value (using the same pattern as in GlobalArrayBuffer.pas / Goccia.Builtins.Math.pas), handle those cases explicitly (e.g., set sensible defaults or clamp), and only then call Trunc and pass the result to ClampOffset; reference the existing identifiers AArgs, GetElement, ToNumberLiteral, Trunc, IsNaN/IsInfinite checks, and ClampOffset when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@units/Goccia.Builtins.JSONL.pas`:
- Around line 175-183: The start and end offset extraction in
units/Goccia.Builtins.JSONL.pas calls Trunc on
AArgs.GetElement(...).ToNumberLiteral.Value without checking special float
flags; update the StartOffset and EndOffset branches to first read the numeric
value, test IsNaN and IsInfinite on that value (using the same pattern as in
GlobalArrayBuffer.pas / Goccia.Builtins.Math.pas), handle those cases explicitly
(e.g., set sensible defaults or clamp), and only then call Trunc and pass the
result to ClampOffset; reference the existing identifiers AArgs, GetElement,
ToNumberLiteral, Trunc, IsNaN/IsInfinite checks, and ClampOffset when making the
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: df005ff9-ebe5-43c8-b7dd-94d871c75247
📒 Files selected for processing (5)
docs/built-ins.mdtests/built-ins/JSONL/parse.jstests/built-ins/JSONL/parseChunk.jsunits/Goccia.Builtins.JSONL.pasunits/Goccia.JSONL.pas
✅ Files skipped from review due to trivial changes (2)
- tests/built-ins/JSONL/parse.js
- tests/built-ins/JSONL/parseChunk.js
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/built-ins.md
- Treat NaN and -Infinity as zero offsets - Clamp positive infinity to the input length - Add tests for start and end offset edge cases
This was referenced Apr 5, 2026
Merged
This was referenced Apr 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
JSONLbuilt-in withJSONL.parse(...)andJSONL.parseChunk(...)for newline-delimited JSON runtime use..jsonlimports in the module loader, exposing each non-empty line as a zero-based string export..jsonlmodule imports/re-exports.Fixes #176
Testing
tests/built-ins/JSONL/parse.js.tests/built-ins/JSONL/parseChunk.js.tests/language/modules/jsonl-import.js.Summary by CodeRabbit
New Features
Documentation
Tests