Uh oh!
There was an error while loading. Please reload this page.
Fix calendar-aligned elapsed durations - #376
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6d8328fd-448c-41bc-8426-102150ede25f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6d8328fd-448c-41bc-8426-102150ede25f
There was a problem hiding this comment.
Pull request overview
Updates elapsed durations to use calendar-aligned month and year anniversaries.
Changes:
- Adds calendar-based duration handling.
- Corrects affected expectations and adds regression cases.
Show a summary per file
| File | Description |
|---|---|
src/duration.ts | Adds calendar-aligned elapsed-time logic. |
test/duration.ts | Updates month-duration expectations. |
test/relative-time.js | Adds month/year regression coverage. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Suppressed comments (1)
src/duration.ts:167
- Skipping the time comparison for every span of at least 12 months silently drops lower units even at second or millisecond precision. The added 2022-01-01 10:00 to 2023-01-01 09:00 case is only 364 days and 23 hours, yet this returns exactly
-P1Y. Year spans should also require the time to match at the requested precision, or retain the remainder relative to the calendar anchor.
// Treat matching calendar days at least a year apart as anniversaries even
// when their times differ, rather than leaking fixed-month remainder days.
const isAnniversary = Math.abs(calendarMonths) >= 12
if (!isAnniversary && !hasSameTimeAtPrecision(date, reference, precisionIndex)) return
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6d8328fd-448c-41bc-8426-102150ede25f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6d8328fd-448c-41bc-8426-102150ede25f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6d8328fd-448c-41bc-8426-102150ede25f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6d8328fd-448c-41bc-8426-102150ede25f
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6d8328fd-448c-41bc-8426-102150ede25f
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6d8328fd-448c-41bc-8426-102150ede25f
There was a problem hiding this comment.
Review details
Suppressed comments (2)
src/duration.ts:211
- Negative end-of-month spans still miss the calendar correction. With
now = 2023-02-28anddate = 2023-01-31, the-1-month anchor clamps to Jan 28, this branch backs it off to zero months, andelapsedTimefalls through to-P28D; the reverse direction returnsP1M. This leaves the advertised Jan 31 → Feb 28 behavior unfixed for past durations. Decompose from the chronologically earlier endpoint and apply the elapsed sign afterward (including the equivalent leap-day case).
const candidateOvershot =
calendarMonths !== 0 && !candidateAligned && (calendarMonths > 0 ? anchor > date : anchor < date)
if (candidateOvershot) {
wholeMonths += calendarMonths > 0 ? -1 : 1
anchor = applyCalendarMonths(reference, wholeMonths)
src/duration.ts:224
- The remainder is measured after applying months, but
applyDurationapplies days before months for negative durations. Consequently the newly expected-P11M25Dfrom 2022-10-24 to 2021-10-30 reapplies to 2021-10-29, one day before its target. Derive negative fields in the same operation order asapplyDuration, or adopt one consistent month-then-remainder order in both functions so generated durations represent the actual endpoint.
const sign = Math.sign(date.getTime() - reference.getTime())
const remainder = isCalendarAligned ? 0 : Math.abs(date.getTime() - anchor.getTime())
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| const sign = Math.sign(date.getTime() - reference.getTime()) | ||
| const remainder = isCalendarAligned ? 0 : Math.abs(date.getTime() - anchor.getTime()) |
liuliu-dev
left a comment
There was a problem hiding this comment.
copilot's comment seems valid 👀
I added a test:
test('past elapsed durations with a day remainder reapply to their target',()=>{constnow=newDate('2022-10-24T14:46:00.000Z')constinput=newDate('2021-10-30T14:46:00.000Z')constduration=elapsedTime(input,'millisecond',now.getTime())assert.equal(applyDuration(now,duration)?.toISOString(),input.toISOString())})it failed:
❌ duration > elapsedTime > past elapsed durations with a day remainder reapply to their target
AssertionError: expected '2021-10-29T14:46:00.000Z' to equal '2021-10-30T14:46:00.000Z'
+ expected - actual
-2021-10-29T14:46:00.000Z
+2021-10-30T14:46:00.000Z
Summary
Fix elapsed durations that incorrectly gain extra days or prematurely cross into a year because months are currently approximated as fixed 30-day periods.
Fixes#262 and incorporates the regression cases proposed in #263.
Problem
elapsedTimederives larger units from elapsed milliseconds:That approximation is useful for short durations, but it does not represent calendar boundaries. It causes results such as:
2 months, 1 day1 year, 5 days1 year, even though the anniversary has not occurredApproach
The change keeps the existing fixed-duration path for shorter, non-aligned intervals, while applying a calendar correction when:
The correction:
This produces same-sign
Durationfields while respecting real calendar boundaries.Precision behavior
Sub-day differences are discarded only when excluded by
precision; duration formatting no longer silently rounds them away.For example, from
2022-01-01T10:00Zto2023-01-01T09:00Z:11 months, 30 days, 23 hours1 yearformatremains responsible for presentation, whileprecisioncontrols the smallest represented unit.Notable edge cases covered
Testing
npm run build