fix: apply fuzzing tests and related fixes - #287
Draft
DecimalTurn wants to merge 147 commits into
Draft
Conversation
…ith seeded RNG serializeInlineArray and serializeInlineTable used Math.random() to decide single-line vs multi-line formatting, bypassing the SeededRandom instance. Thread the RNG through the serialization chain so that the same seed always produces identical TOML output.
…omments The fuzz harness was not fully deterministic (Math.random() in serialization), so seed numbers in test names were misleading — the same seed would not reproduce the same TOML. The tests themselves remain valuable; only the seed references are removed.
…d 187) Changing a dotted key to a scalar under a section header leaves a conflicting key-value in the output, producing 'Value already defined' on re-parse. Seeds 92/139/176 require the full random TOML context to trigger and are not included.
When a dotted key is truncated (e.g. v.jp -> v) because the value changed from a table to a scalar, any remaining sibling KVs that were children of the old implicit table (e.g. v.e4.c6) must be removed. Otherwise the output defines the same key twice — once as the new scalar and once as the leftover dotted-key children. Fix applies to both bare-KV (branch 1) and inline-item-KV (branch 2) Edit paths.
Full random TOML reduced from ~115 to ~21 lines while preserving the bug. Corruption occurs when inlineTableStart=2 converts nested inline tables to sections, a table section is deleted, an AOT is emptied, a dotted key value is changed, and an empty [g] section follows.
… seeds Prints the generated TOML, format, mutations, patched output and parse status for a given seed. Useful for extracting minimal reproductions from fuzz failures. Usage: npx tsx src/__tests__/inspect-fuzz-seed.ts <seed> [--mutations M]
…ending exit offset When two sections were removed in the same applyChanges batch, the second removal's extra blank-line reclaim used node.loc.start.line without accounting for the first removal's pending exit offset on the previous sibling. This inflated extra, pulling subsequent sections too far up and causing header overlaps in the output. Seed 176 is now fixed — delete a section + empty an AOT no longer corrupts the writer positioning.
…d minimumDecimals When the fuzz harness runs with format options that intentionally change the TOML output (truncateZeroTimeInDates and minimumDecimals), the reparsed JS object can differ from the mutated one in ways that are still correct. deepEqualWithFormat normalises both sides — converting midnight-UTC Dates to date-only ISO strings and rounding numbers to the specified decimal places — before comparing, so these format-driven differences don't cause false-positive roundtrip-mismatch failures.
…is removed When a dotted KeyValue (e.g. y.ic83) inside a table has its last key segment removed, the implicit parent segments no longer have any CST representation. Extend the existing Table/TableArray implicit-parent materialisation to also cover KeyValue nodes with dotted keys. The KeyValue path searches the node's containing table (not just the document root) for remaining siblings sharing the same key prefix. When none remain and the updated JS object has an empty object at that path, an empty table header is generated. Fixes the last .fails test (fuzz #484).
Three new tests covering fuzz-discovered failure categories: - Deleting a middle segment of a root-level dotted KV (a.b.c -> delete a.b, materialises [a]) - Deleting a key from an inline table inside an inline array - Deleting a key inside a nested array where the parent is a scalar All three tests pass as a result of the previous implicit-parent materialisation fix for KeyValue nodes.
Two minimal reproductions from fuzz harness:
- Seed 22 (InlineItem for remove): deleting an element from a nested
inline array inside an inline table. Reproduced with just
t = { arr = [1, [2, 3, 4]] } and delete obj.p.t.arr[1][0].
- Seed 20 (Integer for remove): deleting an element from a nested
array. Reproduced with just a = [ [ 1, 2, 3 ] ] and
delete obj.a[0][0].
Both are caused by findParent returning a leaf node (InlineItem /
primitive) instead of the InlineArray container when the path
traverses through nested inline arrays.
…ed key When removing the last segment of a dotted key inside an array-of-tables entry (e.g. delete-key at 7<L:.b32uvhdz.0.z0ncoh.y.eam), findParent returns the node itself because every prefix of the change path matches the dotted key via prefix-matching. The handler then unwrapped the KV's value (an Integer) as the parent and crashed with 'Unsupported parent type Integer for remove' (fuzz seed 20). Fall back to the node's structural container (findHostContainer) when the one-segment-higher re-resolution still returns the node itself, so the KV is removed from its actual container (the AOT entry) instead.
When the last segment of a dotted key inside an array-of-tables entry is removed (e.g. z0ncoh.y.eam -> z0ncoh.y), the implicit parent was materialised with the full path including the numeric entry index as a key segment, which the parser treats as a literal '0' key inside the entry. Derive the prefix relative to the node's container (stripping the entry key and its index for AOT entries), check remaining siblings against that relative prefix (also covering sub-tables stored as document-level siblings), generate the table with the entry key plus relative prefix, and insert it right after the entry so it stays in scope.
LocalDate has no originalFormat property, so deepClone fell back to new Date(getTime()), producing a plain Date whose toISOString() differs from the LocalDate's date-only override. The library's datesEqual then saw a phantom edit on an untouched value and regenerated it (2057-08-11 -> 2057-08-11T00:00:00.000Z), causing a false roundtrip mismatch. Reconstruct Date subclasses through their own constructor (new Ctor(obj.toISOString())) when originalFormat is unavailable, so the subclass and its toISOString() override survive the clone.
…ntry Reproduces fuzz seed 20 (delete-key at 7<L:.b32uvhdz.0.z0ncoh.y.eam). Without the structural-container fallback this crashes with 'Unsupported parent type Integer for remove'; with it the KV is removed and the emptied parent is materialised as a sub-table inside the AOT entry's scope.
Removing the last segment of a dotted key inside an AOT entry must materialise the emptied parent as a sub-table of the SAME entry: key relative to the entry (no numeric index), inserted before the next entry so it doesn't leak into a later entry's scope. Guards the fix in 67998a2.
…newLine CRLF) Documents that newLine='\r\n' normalises multiline string content newlines to CRLF, so re-parsing the patched output differs from the input object. This is the library's intended no-mixed-line-endings behaviour; the fuzz harness comparison must normalise \r\n vs \n in strings the same way.
When the caller sets newLine, the library normalises ALL line endings in the output — including multiline string content — to the requested style, so re-parsed values legitimately differ by \r\n vs \n. deepEqualWithFormat now treats the two as equivalent inside strings, but only when fmt.newLine is explicitly set; without it the document keeps its original newlines and comparisons stay strict.
…eed 4)
Replacing the first segment of a dotted KV with an object produces adds
under a prefix that no longer exists, e.g. `ak.b.c = date` becomes
`ak = { k99 = [...], k36 = ... }`. After the first add created `ak.k99`,
the second add's findParent matched `ak` as a PREFIX of the dotted key
and unwrapped the KV's value as the parent — inserting `k36` into the
array line, or crashing with "Unsupported parent type 'Integer'" for
scalar values.
Adds now walk change.path prefixes from longest to shortest, skipping
KVs matched by dotted-key prefix (detected by comparing the node's
absolute tree path length against the probe), so consecutive adds land
in the shared ancestor table. restoreMissingKeySegments then re-attaches
the missing segments (`ak`) to each child's key, emitting `ak.k99` /
`ak.k36` dotted KVs.
Also fixes fuzz seed 26; seeds 11, 22, 30 remain.
…uzz seed 11) Deleting the last segment of a ROOT-level dotted KV (e.g. removing nduxrs6pz.ek from nduxrs6pz.ek = 597081) materialises the emptied parent as a [nduxrs6pz] section. The section was inserted at the removed KV's index — the top of the file — and a [table] header claims every key-value below it, so all remaining root KVs got nested under it and the round-trip no longer matched the patched object. The materialised header now goes after all remaining root KVs: before the first existing section header, or at the very end when there is none. AOT-entry materialisation keeps its existing position (right after the entry, inside its scope). Also fixes fuzz seed 30; seed 22 remains.
… seed 22)
Deleting the last segment of a dotted key inside an inline table (TOML
1.1), e.g. removing `a.b` from `p = { a.b = "x", c = 1 }`, emptied the
implicit prefix. The Remove handler's implicit-parent materialisation
only recognised block containers (Document/Table/TableArray) — the
InlineItem node wrapping the dotted KV was removed outright and the
emptied prefix (`a`) was silently dropped instead of being re-emitted as
`a = {}`.
The materialisation now also handles InlineTable containers: when the
caller still wants the prefix as an empty object, a fresh `prefix = {}`
item (parsed from a literal, since parseJS strips empty objects) is
inserted at the removed item's position, inheriting its comma setting.
Two positional hazards are handled on the way:
- the removal registered a pending enter offset on the inline table
(first-item removals register on the parent), which applyWrites would
have applied to the new item too — it is resolved before inserting;
- with bracket spacing enabled the inserted item is re-aligned one
column so `{ a = {}` keeps the original bracket style.
This was the last failing seed of the 0-30 fuzz sweep.
…d 50) Two crashes/corruptions discovered by seed 50: 1. Deleting the last segment of a dotted key inside an inline table that is itself an inline-array element: findHostContainer only returns block containers, so the Remove handler's parent fallback landed on the Document and remove() threw "Node not found in parent for removal". The fallback now searches for the node's true structural container (any container type, descending through values and inline items) before giving up. 2. Splicing an element out of an inline array containing a multiline string: the diff expresses the splice as Moves + a Remove. moveInlineElement re-inserted the multiline item with the writer's span math, which assumes the next sibling sits at the moved node's first-line column. That corrupts every item after the moved one and the closing bracket — the string's closing delimiter was overwritten by the following date. moveInlineElement now resolves removal offsets before re-inserting (so insert() measures final positions), restores the end column of multi-line values on the wrapper InlineItem AND its inner node, and realigns the tail items sequentially after the move so they sit right after the previous item's end, syncing the container end and any wrapper InlineItem's end. Remaining sweep failures (31-100): 52, 70, 73, 79, 92, 98.
…, 98) The harness randomly mutates the parsed JS object, including writes to out-of-bounds array indices. When the array is an array-of-tables (from [[key]] sections), writing a scalar/array/date as a new element is unrepresentable in TOML — [[key]] entries must be tables — so the round-trip comparison can never pass (fuzz seeds 52 and 98 wrote a date into an AOT entry). fuzzOne now collects AOT key paths from the generated CST and skips mutations that would write a non-table value into one (with a bounded retry loop so a skipped mutation cannot spin).
With useTabsForIndentation enabled, toTOML converted leading spaces to
tabs on every emitted line — including the content lines of multiline
string literals, silently changing the value (' A' became '\tA').
Indentation inside a multiline string is part of the value.
The tab pass now skips every line from a multiline string's first
content line through its closing delimiter (the opening-delimiter line
may still carry structural indentation). Fixes fuzz seeds 70, 73
and 79.
… removing first section (seed 65785)
…tive key (seed 67221)
…a surviving section (seed 68244)
…shared-line container (seed 68861)
…eds 80004, 82825, 86547, 86724)
…edits (seed 136865)
There was a problem hiding this comment.
Pull request overview
This PR introduces a deterministic fuzzing harness for patch() and applies a large set of correctness fixes across parsing, diffing, CST mutation/writing, and formatting to prevent round-trip corruption and improve robustness against edge cases surfaced by fuzz seeds.
Changes:
- Add fuzzing utilities/scripts and format-aware fuzz assertions to systematically reproduce and distill
patch()failures. - Harden
patch()/writer/diff logic around pending offsets, dotted-key/path resolution, inline container moves/inserts/removals, and structural edits. - Improve parsing/serialization correctness for specific edge cases (newline-style detection, datetime token merging guard, key-path encoding collisions).
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/writer.ts | Exposes dirty/offset helpers and refines inline insert/remove/shift behavior to avoid stale-loc and multiline edge-case corruption. |
| src/utils.ts | Extends stableStringify to preserve distinct non-finite numbers in diffs. |
| src/tokenizer.ts | Adds newline scan state to detect mixed line endings during tokenization. |
| src/to-toml.ts | Avoids tab-conversion inside multiline string literal content lines. |
| src/to-js.ts | Switches internal key identity to an unambiguous encoding to avoid collisions. |
| src/patch.ts | Large set of fuzz-driven fixes for structural edits, prefix matches, offset flushing, and container/section correctness. |
| src/parse-toml.ts | Threads newline-state through tokenizer; fixes datetime token-merge false positives. |
| src/generate.ts | Fixes generated KeyValue value alignment when value loc starts on a later line. |
| src/find-by-path.ts | Defers strict-prefix dotted-key matches to prefer exact matches. |
| src/diff.ts | Improves array diffing around duplicates/moves/removes to reduce writer-corrupting move chains. |
| src/comment-ownership.ts | Hardens inline-element moves (especially multiline/shared-line layouts) and offset handling. |
| src/tests/update-order.test.ts | Adds a known-failing regression test for an unsupported reorder case. |
| src/tests/to-js.test.ts | Adds regression coverage for empty-key vs literal-dot key-path collision. |
| src/tests/randomizer.ts | Makes randomizer serialization deterministic via seeded RNG (no Math.random). |
| src/tests/parse-toml.test.ts | Adds regression for datetime parsing edge case (quoted key following date-only). |
| src/tests/inspect-fuzz-seed.ts | Adds a CLI helper to inspect a specific fuzz seed’s TOML/mutations/output. |
| src/tests/fuzz-patch.ts | Adds format-aware equality, skips unrepresentable AOT mutations, exports helpers, and avoids side effects on import. |
| src/tests/diff.test.ts | Adds regressions for array diffing with duplicates and multiline-sensitive cases. |
| src/tests/snapshots/diff.test.ts.snap | Updates snapshot to match new diff behavior. |
| scripts/generate-seed-test.ts | Adds helper script to generate scaffold regression tests from seeds. |
| scripts/fuzz-investigate.ts | Adds investigation script for fuzz failures/mismatches with detailed reports. |
| scripts/compare-branch-size.mjs | Adds branch-to-branch dist size comparison script via git worktrees. |
| package.json | Adds compare-branch-size script entry. |
| docs/Fuzz-Testing.md | Documents fuzzing approach, distillation workflow, and tooling. |
| docs/bug-notes/fuzz-sweep-60000-80000-roundtrip-fixes.md | Adds summarized notes for a fuzz-fix sweep window. |
| docs/bug-notes/fuzz-sweep-80000-100000-roundtrip-fixes.md | Adds summarized notes for a fuzz-fix sweep window. |
| docs/bug-notes/fuzz-sweep-120000-160000-roundtrip-fixes.md | Adds summarized notes for a fuzz-fix sweep window. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+646
to
+648
| const cached = absolutePathCache.get(target); | ||
| if (cached) return cached; | ||
| const indexes: Record<string, number> = {}; |
Comment on lines
+107
to
+109
| export function getPendingEnterOffsets(root: Root): Offsets { | ||
| return enter_offsets.has(root) ? enter_offsets.get(root)! : new WeakMap(); | ||
| } |
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.
This branch includes the code for a fuzzing testing suite for the patching function:
The
patchfunction has a hard guarantee to uphold: apply an edit to a TOMLdocument while preserving its comments, whitespace and formatting. But while those are nice to keep, the crucial part remains to make sure that we don't mangle the data as this is what actually matters in the end.
So, there's clearly a need to harden the patch function and a good way to discover bugs remain fuzzing: we generate TOML documents and
mutations at random, apply
patch, and check that the result round-tripscorrectly (see
src/__tests__/fuzz-patch.tsand itsrandomizer.ts).The method
delete key, change value, change type, add/remove array item, etc.).
patch(src, mutatedObj)and verify the output re-parses to theexpected object and preserves the source's comments and formatting.
mutation that still reproduces the bug (see the distillation process below),
and add it as a test marked
.fails.to confirm the seed passes too.
Footnotes
Apparently he never said it, but everybody seems to remember that. ↩