Skip to content

Fix calendar-aligned elapsed durations - #376

Draft
francinelucca wants to merge 3 commits into
mainfrom
francinelucca-fix-relative-time-tests
Draft

Fix calendar-aligned elapsed durations#376
francinelucca wants to merge 3 commits into
mainfrom
francinelucca-fix-relative-time-tests

Conversation

@francinelucca

Copy link
Copy Markdown
Contributor

Summary

  • calculate whole month and year durations from calendar boundaries instead of fixed 30-day months
  • remove the resulting stray days from exact month and year anniversaries
  • include the regression cases from add test cases for superfluous days #263 and correct existing expectations that encoded the same bug

Fixes #262.

Testing

  • npm run build
  • full browser test suite (642 tests)

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/duration.ts Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 6d8328fd-448c-41bc-8426-102150ede25f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stray days added to duration

2 participants