Fix turn-of-year not recognized in search_dates date ranges - #1363
Open
yamilmaud wants to merge 5 commits into
Open
Fix turn-of-year not recognized in search_dates date ranges#1363yamilmaud wants to merge 5 commits into
yamilmaud wants to merge 5 commits into
Conversation
Ensures the turn-of-year fix in 8bcb9ee conforms to the project's ruff formatting rules (verified via `ruff format --check`). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1363 +/- ##
==========================================
+ Coverage 97.10% 97.14% +0.03%
==========================================
Files 235 236 +1
Lines 2904 3046 +142
==========================================
+ Hits 2820 2959 +139
- Misses 84 87 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
parse_found_objects only appends entries with a truthy date_obj (line 197), so results passed to _adjust_year_for_rollovers never contain None dates. Codecov flagged the branch as uncovered; it's dead code, not a missing test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rn-of-year rollover
The previous "curr.month < prev.month and gap < 6 months" heuristic
silently skipped legitimate wide-gap ranges (e.g. "September until
March", a 6-month forward gap) while a naive wider threshold would
instead misfire on unrelated standalone date mentions in running text
that happen to go backwards in month.
Detecting an actual range connector ("until"/"to"/"through"/"till"/"-")
between the two date substrings distinguishes a real range from
coincidental backwards-in-month mentions without relying on an
arbitrary month-count cutoff.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
Could you keep coverage up? |
Exercises _is_range_connector against each supported connector word/dash and several non-range strings, directly documenting and locking in the matching rules used by _adjust_year_for_rollovers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
search_datessplits date ranges like"Closed from 23th December until 8th January."into separate substrings and parses each independently, so a range spanning the new year had both dates land in the same calendar year (the second date should roll over to the next year)._ExactLanguageSearch._adjust_year_for_rollovers, which detects consecutive parsed results without an explicit year, joined by a range connector (until,to,through,till,-/–/—) in the original text, where the later date's month comes chronologically before the earlier date's month — and increments its year by one.Why a connector check instead of a bare month comparison
An earlier version of this fix used
curr.month < prev.month and (curr.month - prev.month) % 12 < 6— a month-gap heuristic. That silently failed to roll over legitimate wide-gap ranges (e.g."from 1st September until 15th March", a 6-month forward gap), and widening or removing the threshold instead caused false positives: unrelated standalone date mentions in running text that merely happen to go backwards in month (e.g. a paragraph mentioning "June ..." then later "May ...", meant to be read within the same year) would get incorrectly bumped to next year.Checking for an actual range connector between the two date substrings (rather than guessing from the month gap alone) correctly distinguishes a real range from coincidental backwards-in-month mentions.
Known scope limits
until/to/through/till/dash) is English-only. Non-English ranges fall back to no rollover (safe, but not fixed) rather than guessing — extending this per-locale is future work.test_search_dates_rollover_skipped_when_substring_position_not_found).Fixes the reported issue:
"Closed from 23th December until 8th January."now parses to(this_year, 12, 23)and(this_year + 1, 1, 8)instead of both falling in the same year.Test plan
tests/test_search.py:test_search_dates_with_prepositions_with_turn_of_year(the originally reported case)test_search_dates_turn_of_year_wide_cross_year_range(3-month gap)test_search_dates_turn_of_year_beyond_six_months(6-month gap, previously broken)test_search_dates_no_rollover_without_range_connector(standalone mentions must not roll over)test_search_dates_rollover_skipped_when_substring_position_not_found(safe fallback on irregular whitespace)pytest tests/— 24023 passed, 6 skipped, 1 xfailedruff check/ruff format --checkclean on changed files🤖 Generated with Claude Code