Skip to content

fix(query-analyzer): pick strongest dateparser match, not the leftmost (#2768)#2772

Merged
nicoloboschi merged 2 commits into
mainfrom
fix/query-analyzer-strongest-date
Jul 20, 2026
Merged

fix(query-analyzer): pick strongest dateparser match, not the leftmost (#2768)#2772
nicoloboschi merged 2 commits into
mainfrom
fix/query-analyzer-strongest-date

Conversation

@benfrank241

Copy link
Copy Markdown
Member

Fixes #2768.

Problem

DateparserQueryAnalyzer.analyze took the first valid dateparser.search_dates match and discarded the rest. dateparser over-matches: short common words that are weekday/month abbreviations in some language (we/me/did → a weekday, do → Sunday) come back as bogus dates. When such a false positive appears before the real date, the false positive wins and the query gets a plausible-but-wrong temporal window — worse than none, because temporal_constraint is non-null so nothing downstream can tell extraction failed.

The previous defence was a hard-coded blacklist. Two problems, both confirmed:

  1. It's a moving target and partly dead code. The guard text.lower() not in false_positives or len(text) > 3 re-admits anything >3 chars, so entries like march/will never filter anything — only the ≤3-char words do any work. we simply isn't in the list.
  2. It's version-dependent. dateparser 1.4.1 (the version shipped in the published 0.8.4 image) added we as an English Wednesday abbreviation that survives languages=["en"] scoping; the locked 1.2.2 doesn't parse we at all. So neither the blacklist nor language-scoping is a stable fix.

Reproduction (verified on both versions, RELATIVE_BASE = 2026-07-17, a Friday)

Query Correct main (locked 1.2.2) main (shipped 1.4.1)
what did we discuss in May May 17 ✅ May 17 ❌ Jul 15 (we)
what did we discuss None ❌ Jul 12 (did) ❌ Jul 15 (we)
tell me what we decided on 2026-06-10 Jun 10 ❌ Jul 15 (me beats the date) ❌ Jul 15 (we)

Fix

Replace the blacklist + leftmost selection with a signal score. Each match is scored by the date content it actually carries (a digit is strongest — day/year/ISO; then explicit month/relative words; then weekday/period words). Matches with no signal (bare abbreviations) score zero and are rejected; among the rest the strongest wins, ties broken by longest span. This subsumes the entire blacklist and is independent of language and dateparser version.

All three cases above now resolve correctly on both 1.2.2 and 1.4.1, and the existing analyzer suite (English months/relative/weekday/ago queries + Chinese period extraction, which runs before this path) stays green.

Tests

  • Deterministic unit tests for the scorer: bare false positives (we/me/did/do/wed/sat/…) score 0; real signals (in May, 2026-06-10, yesterday, last week, on Wednesday) score > 0; a digit outscores a weekday word.
  • Behaviour tests through the analyzer for the three issue scenarios (rejection, ranking, real-month-survives). Per the reporters' guidance, these assert analyzer output, never raw dateparser spans, so they hold across dateparser versions.

400 tests pass in test_query_analyzer.py (was 382).

Follow-up (out of scope)

@dimonnld's investigation surfaced a lock drift: the repo uv.lock pins dateparser==1.2.2, but the shipped 0.8.4 image's /app/api/uv.lock pins 1.4.1. If CI runs 1.2.2 while releases ship 1.4.1, this bug class is invisible to the suite by construction. Worth a separate look — this PR's tests are written to be version-independent so they guard the behaviour either way.

Credit to @dimonnld (report + root-cause) and @koriyoshi2041 (independent repro + the ranking/rejection test split).

#2768)

dateparser.search_dates over-matches: short common words that are weekday
or month abbreviations in some language ("we"/"me"/"did" resolve to a
weekday, "do" to Sunday) come back as bogus dates. The analyzer took the
first valid match, so when a false positive appeared before the real date
the query got a plausible-but-wrong temporal window — worse than none,
since the constraint is non-null and nothing downstream can tell that
extraction failed.

The previous defence was a hard-coded blacklist of such words, which is a
moving target (every short word dateparser resolves is a new instance of
the same bug) and was already partly dead code: the `len(text) > 3` escape
hatch re-admitted every multi-character entry, so only the <=3-char words
did any work. The bug also depends on the dateparser version — 1.4.1 (the
version shipped in the published image) added "we" as an English Wednesday
abbreviation that survives `languages=["en"]` scoping, while the locked
1.2.2 does not — so language scoping is not a stable fix either.

Replace the blacklist + leftmost selection with a signal score: each match
is scored by the date content it actually carries (a digit is strongest,
then explicit month/relative words, then weekday/period words). Matches
with no signal (bare abbreviations) score zero and are rejected; among the
rest the strongest wins, ties broken by longest span. This subsumes the
entire blacklist and is independent of language and dateparser version.

Tested (Friday reference date, where these abbreviations resolve):
- "what did we discuss"                   -> no constraint (was 07-12/07-15)
- "tell me what we decided on 2026-06-10" -> 2026-06-10 (was 07-15)
- "what did we discuss in May"            -> May (unchanged, now robust)

Regression tests assert analyzer output, never raw dateparser spans, so
they hold across dateparser versions.
# Conflicts:
#	hindsight-api-slim/tests/test_query_analyzer.py
@nicoloboschi
nicoloboschi merged commit d28b852 into main Jul 20, 2026
1 of 2 checks passed
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.

[BUG] query_analyzer picks the first dateparser match, so a false positive ("we") overrides the real date

2 participants