Repository navigation
fix: post-mortem follow-ups for TOON 4.4 and jsonx fast path - #231
Merged
Merged
Conversation
The JSONL batch path passed caller hooks to the speculative joined parse, and the per-line retry passed them again, so a hook ran up to three times on a value that failed. Skip both speculative attempts when a custom decoder or hook is supplied, matching loads(); a rejected value is now reported once. Also reject bytes with an explicit TypeError instead of leaking the preprocessing scanner's internal error, and pin the fast path structurally (plain JSON never reaches _preprocess) rather than by timing. Bump jsonx to 1.0.2.
Unknown encode/decode option keys were silently ignored, so a porting typo such as the reference's indent_size produced a different document with no error. Reject them with TypeError, keep the explicit lengthMarker migration error, accept str/bytes/bytearray on decode (UTF-8, with invalid input reported as ToonDecodeError), and state the non-ValueError contract in the ToonDecodeError docstring. Bump toon to 0.6.1.
The benchmark workflow filtered bench-toon out of the extras and installed toon_format from an unpinned git+ URL with '|| true', so a failed or drifted reference silently removed the toon rows from the artifact. Install the bench-toon extra in a required step (matching lint-test) and assert toon rows exist, the same guard s3 already has. Correct the AGENTS note: the pin is a git commit, not a PyPI availability claim.
Pass -rs to the jsonx and toon targets so a missing reference library is reported as a skip with its reason instead of looking like a short green run.
Oaklight
force-pushed
the
fix/postmortem-followups
branch
from
October 22, 2026 22:18
a2e894c to
9c0ed9b
Compare
The class reports jsonx and stdlib side by side; the fast path itself is pinned structurally in test_jsonx_fastpath.py, so these rows are a visible measurement rather than a timing assertion.
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
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.
What
Follow-ups from the post-mortem review of
a37e656(TOON 4.4, #227) and5afde23(#225), requested from elena, clementine and milo in #dev-zerodep.loads_lines()now opts out of the batch fast-path when a custom decoder or hook is supplied — previously a hook ran up to three times on a value in a JSONC line, and a hook'sJSONDecodeErrorrejection was retried.loads()rejects bytes with an explicitTypeErrorinstead of leaking the scanner's internal error. The fast path is now pinned structurally (plain JSON never reaches_preprocess) instead of by the dropped timing assertion.TypeError(a porting typo such as the reference'sindent_sizeno longer silently changes the document);decode()acceptsstr/bytes/bytearray(UTF-8, invalid input →ToonDecodeError); theToonDecodeErrordocstring states the non-ValueErrorcontract.bench-toonpin in a required step and assertstoon/rows in the artifact, instead of an unpinnedgit+install with|| true. AGENTS wording corrected.test-jsonx/test-toonpass-rsso a missing reference library is visible as a reasoned skip rather than a short green run.Not changed
ToonDecodeErrorstays a plainException(notValueError): deliberate contract, now documented in the class docstring and the docs.httpserverunix-socket failures (longTMPDIR) andtabulateUnicode failures seen in the reviewers' runs are environment-specific, not attributable to these merges.Verification
pytest jsonx toon config --ignore-glob="*/test_*_benchmark.py": 336 passed, 2 skipped.make test-jsonx: 88 passed;make test-toon: 152 passed (conformance included).pre-commit run --all-files: ruff, ruff-format, ty, complexipy pass.zerodep version-check: all modules up-to-date.Docs (module pages + changelog, EN and ZH) are committed on
docs_en/docs_zh.AI was used to assist with this change.