fix: resolve date related issues - #316
Merged
Merged
Conversation
A Date holds milliseconds, but TOML fractional seconds can carry any number of
digits, so stringify(parse(doc)), parse(doc, { temporal: true }) and a patched
untouched value all dropped everything past the millisecond: 07:32:00.123456
came back as 07:32:00.123.
fmtMs now emits the whole source fraction when the value's millisecond still
matches its leading digits, and keeps the source's digit count when the value
did change (09:15:30.5 stays one digit, a six-digit source keeps six digits of
the new value).
datesEqual compares dates by instant and TOML kind instead of rendered text.
Text comparison says .5 and .500 differ, and says a source with sub-millisecond
digits differs from the equal Date that cannot hold them, so an untouched value
read as an edit. diff-lite had its own copy of that comparison and now shares
this one.
LocalTime defaults originalFormat to value, like the other three date classes.
Reconstructing a LocalTime with a single argument, as the fuzz helpers do,
silently dropped the fraction; seeds 16552 and 17339 caught it.
Un-skips the copied-input test: a structuredClone'd document keeps
17:13:19.580912 because the source text still pins the digits.
The suite only exercised smol-toml's default TomlDate output on a handful of spellings. Two gaps: - Spellings smol accepts but re-emits differently: sub-millisecond fractions (truncated to milliseconds), a space separator (re-emitted with T), lowercase t and z, explicit +00:00 and -00:00 offsets and TOML 1.1 HH:MM local times (re-emitted with seconds). Asserts the unedited round trip is byte-for-byte for patch and patch-lite, that an unrelated edit leaves every row verbatim, and that edits keep the source separator, offset style and case. - useLegacyDate: false, which returns Temporal objects instead of TomlDate, had no coverage at all. Asserts the emitted types, a byte-for-byte round trip, that stringify keeps the sub-millisecond precision, edits of all four kinds, and that patch-lite rejects Temporal with TypeChange as docs/Patch-Lite.md documents.
The harness only collected scalar leaves, so its comment about skipping date/time values was accurate: collectEditableLeaves excludes Date and the replacement generator only produced scalars. Adds collectDateLeaves plus a same-kind date generator that moves a date-only value by whole days and a time or datetime by hours, reusing createDateWithOriginalFormat so the source class, separator, offset style and fractional-digit count are kept. Scalars and dates are now picked from the same edit loop, preferring dates so they are not drowned out by scalar-heavy documents. A 3000-seed sweep passes with zero failures. The top comment and the sweep test's comment now describe the real behaviour, and a unit test pins the two leaf collectors.
createDateWithOriginalFormat returned an already-LocalTime value untouched, so
a time took its fractional-digit count from how the caller built the object
rather than from the document. Every other kind rebuilds from the source text.
LocalTime was therefore the only class that lost the source's precision on an
edit:
time = 07:32:00.123456 + LocalTime('09:15:30.123') -> 09:15:30.123
dt = 1979-05-27T07:32:00.123456 + LocalDateTime(...) -> 00:00:00.123456
The time branch now rebuilds from the source like the other kinds, so a
millisecond that matches the source fraction's leading digits keeps the rest of
the source fraction, and a different millisecond is written with the source's
digit count capped at the three digits a Date holds.
One smol-toml compat expectation changes with it: a source written without a
fraction that is edited to a value with 500 ms now writes .5 instead of .500.
That is what createDateWithOriginalFormat already did for LocalDateTime and what
date-format-precision.test.ts pins, so the old expectation was recording the
bypass rather than the intended behaviour.
minimumDecimals only ever applied to numbers, so there was no way to ask for a
consistent fractional-second precision across a TOML document. Dates and times
now have their own option, so the two can be set independently:
stringify(parse('n = 1.5\nt = 07:32:00\n'), { minimumDecimals: 3 })
// 'n = 1.500\nt = 07:32:00\n'
stringify(parse('n = 1.5\nt = 07:32:00\n'), { minimumTimeDecimals: 3 })
// 'n = 1.5\nt = 07:32:00.000\n'
The floor is applied where a date is rendered: generateDateTime and
generateTemporalDateTime take the count from parseJS, and preserveFormatting
pads the value patch() rewrites. Padding only, so digits already written are
kept and a date-only value, which carries no time, is left alone. A Temporal
time written without seconds gains ':00' before its fraction.
Untouched rows are never re-rendered, so they keep their bytes, exactly as the
option behaves for floats. The option defaults to 0, is never auto-detected and
is compared by isDefaultFormat, so a format that only sets it still takes the
fast default path. patch-lite accepts no formatting options by design, so it is
unaffected.
fmtMs treated the source's fractional-digit count as a hard limit and sliced the millisecond to fit it. A source that wrote one digit and a value of 750 ms therefore produced `.7`, which is 700 ms, so patch silently changed the data: t = 07:32:00.5 + 09:15:30.750 -> t = 09:15:30.7 (700 ms, not 750) The count is now a floor. The fraction widens to the shortest spelling that represents the millisecond exactly, so the example becomes `.75` and re-parses to 750 ms, while a source with wider padding keeps it (`.500` becomes `.750`). The branch that preserves sub-millisecond source digits is unchanged, and a source with no fraction still gets the minimal spelling. All four classes share fmtMs, and the patch path already did this for LocalDateTime and OffsetDateTime, so those were losing precision the same way. Three assertions in date-format-precision.test.ts pinned the truncation and are updated, and sub-millisecond-precision.test.ts gains per-kind widening cases plus a check that the written instant re-parses to the requested value.
Variants 1-3 could not reach the fraction formatting path: their source fractions are always six digits, which is wide enough to write any millisecond exactly, and their replacement dates are whole seconds. A source with one fractional digit and an edit to 750 ms therefore wrote `.7` and changed the value with every variant passing. Variant 4 closes that gap: - `shortenFractionalSeconds` rewrites source fractions down to one or two digits before parsing, which is the state where a truncating edit changes the value; - replacement dates carry a time of day and a random millisecond, and `dateFocus` aims most mutations at date leaves and always gives them a date value. An untargeted draw reached a date leaf so rarely that a 400-seed sweep found nothing even with the bug present; - `fullFormats` randomizes the options the earlier variants never set (`minimumTimeDecimals`, `escapeSequenceUpperCase`). Dates compare by instant for this variant, so a value written with fewer digits than it needs fails the round trip instead of passing as a formatting difference. With the previous truncation reintroduced, `fuzz-run4.ts` fails at seed 10 after 11 seeds; with the fix in place, 2001 seeds pass. Seed 10 is pinned in `fuzz4.test.ts` next to a 250-seed sweep, and `pnpm run fuzz4` runs a wider range. Every new draw happens only when the variant is enabled, so variants 1-3 keep their exact corpora.
`distill-seed.ts` replays the mutations it writes into the generated test, so it needs the same source transform, mutation mode and format as the harness whose seed it is distilling. Variant 4 adds those three. Its date comparison is by instant: the text form reports a value and the same instant written with a different number of fractional digits as a difference, which would refuse a seed the harness just flagged. Verified by distilling seed 10 down to a single mutation while the truncation is present, and by distilling a known variant 3 seed unchanged. The usage strings and the contributing notes mention variant 4, and the patch-lite section of docs/Fuzz-Testing.md no longer claims the lite harness skips date values.
DecimalTurn
changed the title
Benchmark comparisonThis branch measured against Fixture Benchmark ResultsTime per iteration. Lower is better. Parse, smol-toml spec example (toml-spec-example.toml)
Parse, smol-toml 5MB mixed (5mb-mixed.toml)
Parse, iarna spec example v0.4.0 (0A-spec-01-example-v0.4.0.toml)
Stringify, smol-toml spec example (toml-spec-example.toml)
Stringify, smol-toml 5MB mixed (5mb-mixed.toml)
Stringify, iarna spec example v0.4.0 (0A-spec-01-example-v0.4.0.toml)
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Critical and moderate date-precision and TOML-kind handling issues remain unresolved.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
This pull request improves TOML date/time precision handling and adds configurable fractional-second padding.
Changes:
- Preserves sub-millisecond date fractions.
- Adds
minimumTimeDecimalsformatting support. - Expands compatibility tests, fuzzing, and documentation.
| File | Description |
|---|---|
src/utils.ts |
Date equality and TOML-kind classification |
src/toml-format.ts |
Adds formatting options |
src/patch.ts |
Preserves date formatting during patches |
src/parse-js.ts |
Propagates formatting options |
src/index.ts |
Updates default-format detection |
src/generate.ts |
Formats generated dates |
src/diff-lite.ts |
Centralizes date comparison |
src/date-format.ts |
Preserves and pads fractional seconds |
src/__tests__/toml-format.test.ts |
Formatting tests |
src/__tests__/sub-millisecond-precision.test.ts |
Precision regression tests |
src/__tests__/smol-toml-compat.test.ts |
Compatibility tests |
src/__tests__/patch.test.ts |
Copied-date regression tests |
src/__tests__/patch-lite.fuzz.test.ts |
Date-leaf fuzz coverage |
src/__tests__/minimum-time-decimals.test.ts |
Formatting-option tests |
src/__tests__/fuzz4.test.ts |
Variant-4 fuzz tests |
src/__tests__/fuzz-patch4.ts |
Precision fuzz harness |
src/__tests__/fuzz-patch.ts |
Fuzz generation enhancements |
src/__tests__/fuzz-patch-lite.ts |
Date edit fuzzing |
src/__tests__/date-format-precision.test.ts |
Fraction-width tests |
src/__tests__/comment-ownership.test.ts |
Comment ownership coverage |
scripts/fuzz-run4.ts |
Fuzz runner |
scripts/distill-seed.ts |
Seed distillation support |
scripts/distill-and-append-seed.ts |
Variant usage updates |
README.md |
Date precision and API documentation |
package.json |
Adds the fuzz4 command |
docs/Patch-Lite.md |
Patch-lite date behavior |
docs/Fuzz-Testing.md |
Fuzz variant documentation |
docs/Formatting.md |
minimumTimeDecimals documentation |
docs/Dates.md |
Date precision documentation |
docs/Comment-Ownership.md |
Ownership clarification |
CONTRIBUTING.md |
Fuzz workflow documentation |
CHANGELOG.md |
Release notes |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The curated stringify gate compares every fixture against smol-toml with one shared 25x budget. The iarna spec example is the smallest mixed-type document in that set, so its time is mostly the fixed per-document overhead and its ratio moves much more between runners than the other two fixtures. Two CI runs measuring the same pair of builds put it at 18.3x and 25.2x, so one unlucky runner crossed the 25x line and failed this pull request even though the build was not slower. The iarna corpus carried its own 32x stringify budget before the corpora were merged into one file; that allowance is restored here. check-thresholds now reads an optional per-fixture table for an operation: [fixtures.stringify.perFixture] "0A-spec-01-example-v0.4.0" = 32 A fixture without an entry of its own falls back to the operation budget, and a fixture with no budget at all stays ungated, so no other budget changes.
This reverts commit 413d6a4.
Two review points from the pull request. An edited value treated the source's digit count as a floor on the written fraction, so a source of `.123456` edited to 500 ms came back as `.500000`. The rule now matches the one numbers already use: a source that wrote zeros past its own significant digits declared padding and keeps that width (`.500` becomes `.750`, `.500000` becomes `.750000`), while significant digits the new value cannot fill are dropped (`.123456` edited to 500 ms becomes `.5`). `TomlFormat.minimumTimeDecimals` is how a document asks for a wider fraction. `.123456` and `.123999` are both 123 ms to `getTime()`, so `datesEqual` called them equal, `valuesEqualIterative` returned the untouched document and the edit was dropped. Fractions are now compared past the millisecond when both values spell those digits out, a replacement with its own sub-millisecond fraction is written the way it spelled it (all four kinds, stringify and patch-lite too), and a millisecond-precision replacement still leaves the source's digits alone.
A floor above the digits a value wrote pads them, so "left alone" only holds at or below that count: 07:32:00.123456 with minimumTimeDecimals: 9 serializes as 07:32:00.123456000. The clause about minimumDecimals is dropped as obvious.
Six digits is a value, not a ceiling: the floor pads whatever wrote fewer. Covers 7 and 9 in the unit test, a nine-digit floor across every kind in stringify, and edits whose fraction collapses or comes from a Temporal value in patch.
patch-lite accepts Temporal.PlainDate, PlainTime, PlainDateTime and ZonedDateTime in the updated object, matching the full patch(). A Temporal value switches the existing side to Temporal parsing so an unchanged value is not rewritten, the Temporal type decides the kind and precision, and the source separator and spelled-out zero offset are preserved. Bundle: 44.3 -> 45.2 kB minified, 13.5 -> 13.9 kB gzipped. The gzipped budget is raised from 14 to 15 kB to keep headroom.
DecimalTurn
force-pushed
the
dev-smol2
branch
from
September 27, 2026 14:58
d38405e to
92e20bc
Compare
DecimalTurn
marked this pull request as ready for review
September 27, 2026 14:59
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.
Reworks how fractional seconds are handled across
parse(),stringify()andpatch(), andbrings
patch-litelevel with the full API for date/time values (including Temporals).below the millisecond is applied instead of silently dropped.
declared it with zeros — the rule numbers already follow for decimals.
minimumTimeDecimalsformatting option raises the written fractional-second digits.patch-liteaccepts Temporal objects, so both entry points take the same inputs.