fix: do not assign weekday that contradicts the computed relative date - #649
fix: do not assign weekday that contradicts the computed relative date#649spokodev wants to merge 1 commit into
Conversation
|
Thanks for the fix. Added one small comment. |
|
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. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Moved the case into en_merging_relative_dates.test.ts and deleted the standalone file. That suite already covers the merge-weekday-onto-relative-date path this fixes, so it fits there. No worries on the delay.
Ops. Sorry. |
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.
6211a6f to
0e1e492
Compare
Problem
A parse of
[weekday] [in N weeks]returns a result whose reportedweekdaycontradicts the date it computed. The weekday says one day, the date is another.Repro (timezone-explicit so it is not a DST artifact):
ref(Wed 2026-06-24)+ 3 weeksis2026-07-15, a Wednesday, but the result still carriesweekday: 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 anextprefix.Cause
MergeWeekdayComponentRefinerexists to merge a weekday that labels an explicit date, for exampleSunday 12/7/2014orTuesday, January 10. ItsshouldMergeResultsonly checksnextResult.start.isCertain("day"), which a relative-offset result such asin 3 weeksalso satisfies. The merge then fires andmergeResultsoverwrites the weekday with the original token, even thoughParsingComponents.createRelativeFromReferencehad 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/relativeDatetag). 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 noresult/relativeDatetag 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 10inen_month_name_middle_endian.test.ts).Tests
Added
test/en/en_weekday_relative_consistency.test.tsasserting 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, andTZ=America/New_York.