Skip to content

fix: do not assign weekday that contradicts the computed relative date#649

Open
spokodev wants to merge 1 commit into
wanasit:masterfrom
spokodev:fix/weekday-relative-date-consistency
Open

fix: do not assign weekday that contradicts the computed relative date#649
spokodev wants to merge 1 commit into
wanasit:masterfrom
spokodev:fix/weekday-relative-date-consistency

Conversation

@spokodev

Copy link
Copy Markdown

Problem

A parse of [weekday] [in N weeks] returns a result whose reported weekday contradicts the date it computed. The weekday says one day, the date is another.

Repro (timezone-explicit so it is not a DST artifact):

const chrono = require("chrono-node");
const ref = new Date("2026-06-24T12:00:00Z"); // Wednesday
const res = chrono.parse("Saturday in 3 weeks", ref)[0];

res.start.knownValues;       // { weekday: 6, day: 15, month: 7, year: 2026 } (weekday 6 = Saturday)
res.start.date();            // 2026-07-15T12:00:00.000Z  -> 2026-07-15 is a WEDNESDAY
res.start.isCertain("weekday"); // true
res.start.get("weekday");    // 6 (Saturday), contradicts the Wednesday date

ref (Wed 2026-06-24) + 3 weeks is 2026-07-15, a Wednesday, but the result still carries weekday: 6 (Saturday) from the original "Saturday" token. The result reports a weekday that disagrees with its own date.

Over a sweep of reference dates x {Saturday, Sunday, Monday, Friday} x {in 1/2/3 weeks}, the large majority of certain-weekday results were inconsistent this way. It also reproduces with within, in N days, and a next prefix.

Cause

MergeWeekdayComponentRefiner exists to merge a weekday that labels an explicit date, for example Sunday 12/7/2014 or Tuesday, January 10. Its shouldMergeResults only checks nextResult.start.isCertain("day"), which a relative-offset result such as in 3 weeks also satisfies. The merge then fires and mergeResults overwrites the weekday with the original token, even though ParsingComponents.createRelativeFromReference had already resolved the correct day-of-week for the offset date.

Fix

Skip the merge when the next result is a relative-offset date (it carries the result/relativeDate tag). A relative-offset date already resolves its own day-of-week, so there is no weekday to label. The legitimate case, a weekday in front of an explicitly stated date, is unchanged: those results have no result/relativeDate tag and still merge exactly as before (including the existing behavior where a user-stated weekday is preserved over the computed date, e.g. Tuesday, January 10 in en_month_name_middle_endian.test.ts).

Tests

Added test/en/en_weekday_relative_consistency.test.ts asserting that any result reporting a certain weekday matches the day-of-week of its computed date. It fails on master (weekday 6 on a Wednesday date) and passes with this change. The full existing suite stays green (615 tests) under default TZ, TZ=UTC, and TZ=America/New_York.

When a weekday name is followed by a relative duration (e.g. "Saturday in
3 weeks"), MergeWeekdayComponentRefiner merged the leading weekday onto the
relative-offset result and unconditionally overwrote its weekday. The date is
computed purely from the offset, so the result ended up reporting a weekday
that contradicts its own date (weekday=Saturday on a date that is a Wednesday).

A relative-offset date already resolves its own day-of-week, so the merge
should only fire when the weekday labels an explicitly stated date (the case
this refiner exists for, e.g. "Sunday 12/7/2014"). Skip the merge when the
next result carries the "result/relativeDate" tag, leaving the relative date
self-consistent.
@wanasit

wanasit commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Thanks for the fix. Added one small comment.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 92.859%. remained the same — spokodev:fix/weekday-relative-date-consistency into wanasit:master

@spokodev

Copy link
Copy Markdown
Author

Thanks @wanasit. The inline comment isn't showing on my end, which usually means it's still in a pending review that hasn't been submitted. Could you submit it so the note surfaces? I'll address it right away.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Having a new test file just for this one case is too fragmented.
Could this test be included in the weekday or relative time testing?

@wanasit

wanasit commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Thanks @wanasit. The inline comment isn't showing on my end, which usually means it's still in a pending review that hasn't been submitted. Could you submit it so the note surfaces? I'll address it right away.

Ops. Sorry.

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.

3 participants