Repository navigation
fix(relative): add the fraction of a negative relative second count - #327
Socialpranker wants to merge 2 commits into
Conversation
`Relative::Seconds` holds a floor decomposition, so -0.25 seconds is `(-1, 750_000_000)`. A `jiff::Span` is sign-uniform, so combining those two fields directly subtracted the fraction instead of adding it and the result was off by `2 * (1 - fraction)` seconds. Rebalance the fraction onto the sign of the whole seconds before building the span.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #327 +/- ##
==========================================
- Coverage 99.34% 97.51% -1.84%
==========================================
Files 21 21
Lines 4123 4261 +138
Branches 136 137 +1
==========================================
+ Hits 4096 4155 +59
- Misses 26 105 +79
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| .try_seconds(seconds) | ||
| .and_then(|span| span.try_nanoseconds(nanoseconds)), | ||
| Relative::Seconds(seconds, nanoseconds) => { | ||
| // `Relative::Seconds` is a floor decomposition: the value is |
There was a problem hiding this comment.
6 lines of comment for 3 lines of code, nobody will read it :) two lines are enough here
| ); | ||
| } | ||
|
|
||
| // Fractional relative seconds, checked against GNU date 9.11 with |
There was a problem hiding this comment.
the header at the top of the file says 8.32, maybe worth aligning the versions to 9.11
Merging this PR will degrade performance by 3.24%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | parse_invalid_input |
59 µs | 61 µs | -3.24% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing Socialpranker:fix-negative-fractional-seconds (8270fba) with main (e25bf15)
|
fair, two lines now. bumped the header to 9.11 too |
2 participants
2026-08-27 12:00:00 -0.25 secondslands 1.5 seconds early. The fraction of anegative relative second count is subtracted instead of added:
The error is
2 * (1 - fraction)seconds, so it grows as the fraction shrinks —-0.000000009 secwas off by very nearly two full seconds. Positive counts werealways correct, and so were the equivalents written with
ago(
+0.25 sec agowas wrong,-0.25 sec agowas right), because only the sign ofthe resulting count matters.
The rule
From the GNU manual, "Relative items in date strings": "The unit of time may be
preceded by a multiplier, given as an optionally signed number… Following a
relative item by the string 'ago' is equivalent to preceding the unit by a
multiplier with value -1." A multiplier of -0.25 on the unit
seconddisplacesthe date by a quarter second backwards; nothing about the sign changes the
magnitude. GNU applies exactly that, keeping the fraction intact rather than
rounding or truncating it — every
%Nabove is exact.Fractions are accepted on
secondonly, in GNU and here alike, so this is thewhole surface of the bug.
The change
Relative::Seconds(i64, u32)is a floor decomposition: the value isseconds + nanoseconds / 1e9with a non-negative fraction, so -0.25 seconds isheld as
(-1, 750_000_000). That representation is correct, and theExtendedDateTimepath (years above 9999) consumes it correctly withchecked_add_seconds, which adds the positive fraction and carries.The in-range path builds a
jiff::Spaninstead, and aSpanis sign-uniform:Span::new().try_seconds(-1).try_nanoseconds(750_000_000)is -1.75s, not-0.25s. The two fields are now rebalanced onto a common sign —
(-1, +750_000_000)becomes(0, -250_000_000)— before the span is built.secondsis negative in that branch, so
seconds + 1cannot overflow.No representation or public API changes, and the extended-year path is
untouched.
How the GNU behavior was established
By running the installed GNU binary (coreutils 9.11) as a black box with
+'%H:%M:%S.%N'so the fraction is visible, plus the manual section quotedabove. I did not read GNU coreutils source.
Testing
cargo test: 440 passed, 0 failed (417 pre-existing, plus 23 newtest_relative_fractional_secondscases whose expectations were taken fromGNU 9.11 transcripts).
entirely fails 12 of 23 cases, flipping the sign of the fraction fails 8,
and an off-by-one on the borrow (
seconds - 1) fails 12.cargo fmt --checkandcargo clippy --all-targets -- -D warnings: clean.fix(offset): treat a numeric offset after a time of day as a zone correction #326: 18 cases moved from differing to identical, 0 regressions
(562 → 544 on
main; stacked on fix(offset): treat a numeric offset after a time of day as a zone correction #326 it is 162 → 144).This is independent of #326 — it reproduces on
mainand the branch is cut frommain— but the two touch the same inputs, so whichever lands second will wanta rebase.
The 144 cases that still differ are one pre-existing class:
<zone> +N <unit> agois accepted here and rejected by GNU. That one looks like a GNU parserartifact rather than a rule to copy — the manual defines
agoas a -1multiplier, GNU itself accepts
UTC +1 year 1 day ago, and inUTC +1 hour +0 sec agoGNU silently ignores theagoinstead of erroring — soI have left it alone.
Disclosure
Prepared with AI assistance (Claude Opus 5, via Claude Code), per the uutils AI
policy. Every GNU behavior quoted above came from running the installed binary
and reading the GNU manual, not from reading GPL source. All testing was run
locally.