Repository navigation
Parse "at 22h" and "à 22h" as a time of day, not as hours ago - #1441
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1441 +/- ##
=======================================
Coverage 99.18% 99.18%
=======================================
Files 239 239
Lines 4052 4054 +2
=======================================
+ Hits 4019 4021 +2
Misses 33 33 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
AdrianAtZyte
marked this pull request as ready for review
September 24, 2026 10:59
serhii73
added a commit
that referenced
this pull request
Oct 5, 2026
- The comment above the new tests gave a wrong reason for using a Wednesday: the seven weekdays resolve to seven different dates whatever the current date. - The parse() rows with a non-breaking hyphen and a no-break space could not tell \W+ from [\s-], because sanitize_date normalises both before the simplifications run. Only search_dates sees the raw text, so the rows that pin \W+ (an en dash and a non-breaking hyphen) are search_dates rows now. - Nothing pinned that "après" and "avant" only exclude the "midi" that directly follows, or that "12 midi" keeps its day: add "après-demain à midi", "avant-hier midi" and "le 12 midi". - Rows inserted in the middle of test_translation and test_search_and_parse renumbered 390 existing test IDs. The new rows are in new test methods instead, so no existing test ID changes. - The strict xfails only tolerate an AssertionError, and say that the French entries are needed besides the support for "next <weekday>". Also move the rule to the end of the simplifications of fr.yaml. Their order does not matter for it, and this keeps it away from the place where the open #1441 inserts its own rules. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
1 task done
serhii73
added a commit
that referenced
this pull request
Oct 19, 2026
* Parse "midi" as noon in French (#459) French had no entry for "midi", so a date string with it was not understood: "mardi midi", "mardi à midi" and "demain à midi" returned None, and search_dates dropped the time of "demain à midi". Add a simplification that reads "midi" as 12:00, like English does for "noon". "après-midi" (the afternoon) and "avant midi" (before noon) are not noon, so they are excluded, also with an article in between ("après le midi") and with any run of non-word characters between the words. A hyphen counts as a word boundary, so without the exclusion a plain "midi" would match inside "après-midi", and "demain après-midi" would be read as a time zone offset of -12:00. "<weekday> prochain" is not covered: it needs support for "next <weekday>", which master lacks even in English (#573). The examples of the issue that need it are strict xfails. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Tighten the tests of French "midi" - The comment above the new tests gave a wrong reason for using a Wednesday: the seven weekdays resolve to seven different dates whatever the current date. - The parse() rows with a non-breaking hyphen and a no-break space could not tell \W+ from [\s-], because sanitize_date normalises both before the simplifications run. Only search_dates sees the raw text, so the rows that pin \W+ (an en dash and a non-breaking hyphen) are search_dates rows now. - Nothing pinned that "après" and "avant" only exclude the "midi" that directly follows, or that "12 midi" keeps its day: add "après-demain à midi", "avant-hier midi" and "le 12 midi". - Rows inserted in the middle of test_translation and test_search_and_parse renumbered 390 existing test IDs. The new rows are in new test methods instead, so no existing test ID changes. - The strict xfails only tolerate an AssertionError, and say that the French entries are needed besides the support for "next <weekday>". Also move the rule to the end of the simplifications of fr.yaml. Their order does not matter for it, and this keeps it away from the place where the open #1441 inserts its own rules. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Make the French "midi" exclusion follow the engine's boundaries Two cases slipped past the exclusion of "après" and "avant", both found by comparing master and the branch on large corpora: - search_dates always strips the accents of the text before it simplifies it, whatever NORMALIZE says, but with NORMALIZE off the rules keep their accents, so the "après" of the lookbehind never matched. "Réunion demain après-midi" lost its "demain" and "Livraison après midi" became noon. Write the rule as apr[eè]s, which the normalised rule set turns into apr[ee]s. - Every rule is wrapped in a boundary that counts "_" as a separator, so "après_midi" is a standalone "midi", but \W+ did not cross the "_" and it became noon. Use [\W_]+, like the boundary. Add the rows that fail without these changes, and rows for what the exclusion must not do: "après-demain à midi" and "avant-hier midi" (the existing relative expressions that contain the words it keys on), and "2026-10-12 midi", next to "le 12 midi" for the missing "12 " prefix. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
serhii73
approved these changes
Oct 20, 2026
serhii73
left a comment
serhii73
left a comment
Collaborator
There was a problem hiding this comment.
Code review (claude code-review, at 9308276; probes run on the PR applied to its merge-base and on master; I did not run the PR's own suite). at 22h and à 22h as a time of day work, and the regressions below are on unusual inputs, so I'm approving. I'd fix them with a lookahead before merge:
- Year or ISO date followed by
Nh ago. The rule\b(\d{4},? |\d{4}-\d{2}-\d{2} )(\d{1,2})h\brewrites2026 5h agoto2026 5:00 ago, leavingagodangling, so2026 5h ago,1999 3h ago,1000 3h agoandil y a 2026 3hreturn None where master parsed them as hours ago. Adding(?!\s+ago)(and the French equivalent) would keep them. at Nhrule. It checks only the word boundary, soat 22h ago,at 24handat 25hreturn None where master gave an hours-ago datetime; limiting the hour to 0-23 and excluding a trailingagowould fall back correctly. The French(?<!\by )\bà (\d{1,2})h\bhas the same gaps (il y a à 22hreturns None).- Coverage. The rule exists for English and French only (
à 22hwithlanguages=['en']is still read as hours ago), and the tests have no negative cases for the inputs above or forsearch_dates(see you at 22h tomorrowchanges from 2026-09-26 10:00 to 2026-09-25 22:00, which is the intended time of day).
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.
Fixes #943, fixes #1147.
It only really fixes 1 part of #1147, but the rest are tracked by other issues: