fix: strip date filters at the start of a query - #1396
Open
LHMQ878 wants to merge 1 commit into
Open
Conversation
DateFilter.defilter matched a required leading whitespace before the date
filter, so a filter at position 0 was never removed. Anything other than
whitespace before the filter had the same effect, which covers start of
query, "(" and ",".
get_filter_terms uses the bare date_regex, so the two methods disagreed
about whether a filter was present. defilter_query then returned the query
unchanged, and routers/helpers.py recovers filters by subtracting the
defiltered query from the original, so filters_in_query came out empty and
the date range never reached apply_filters. The raw dt>="..." text was also
left in the query passed to extract_questions and embed_query.
For example, `dt>="last week" what changed in my project` searched the whole
knowledge base with the filter text still in the embedded query, while the
same filter written at the end of the query worked.
Dropping the \s+ prefix leaves the existing "remove multiple spaces" pass to
tidy up. Across 280 prefix/filter/suffix combinations, every output that
differs is one the old expression failed to strip; no case where it already
worked changes.
Fixes khoj-ai#1395
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
Fixes #1395.
DateFilter.defilterrequired whitespace before the date filter, so adtfilter at the start of a query was never stripped. Becauserouters/helpers.pyrecovers the filters by subtracting the defiltered query from the original, that leftfilters_in_queryempty and the date range never reached the database query — while the rawdt>="..."text stayed in the question sent to the LLM and the embedding model.Root cause
A filter at position 0 has nothing before it, so
\s+can't match.get_filter_termsuses the baredate_regexwith no such requirement, so the two methods disagree about whether a filter is present. Measured onmaster(ae229ca):get_filter_termsdefilterhead dt:"1984-01-01" tail["dt:'1984-01-01'"]'head tail'dt:"1984-01-01" tail["dt:'1984-01-01'"]'dt:"1984-01-01" tail'dt:"1984-01-01"["dt:'1984-01-01'"]'dt:"1984-01-01"'head,dt:"1984-01-01"["dt:'1984-01-01'"]'head,dt:"1984-01-01"'head (dt:"1984-01-01")["dt:'1984-01-01'"]'head (dt:"1984-01-01")'Any non-whitespace character before the filter suppresses the removal, so start-of-query,
(and,are all affected. Two adjacent filters hit it too: the first one's trailing space is consumed as the second one's required leading\s+, so only one is removed —Impact
routers/helpers.py:1329reconstructs the filters by string subtraction:When
defilterreturns the query unchanged,filters_in_queryis"". That string is the only thing appended to each inferred search query (line 1371) and the only thingEntryAdapters.apply_filterssees, so the date range is dropped. Measured end to end:Two user-visible effects for one input: the date filter isn't applied, and
dt>="last week"is left in the text passed toextract_questionsandembed_query. Neither is surfaced — the query just behaves as though no date filter were typed.Fix
Drop the
\s+prefix and match the filter itself. The existing "remove multiple spaces" pass on the next line already tidies up the whitespace the substitution leaves behind, which is why nothing else needed to change.Behavior parity
I enumerated all 280 combinations of 10 prefixes (
"","head ","head","head, ","head,","head (","(","head\n","head\t"," ") × 4 filter forms (dt:,dt>=,dt<,dt==) × 7 suffixes:Every output that differs is a case the old expression failed to strip. There is no combination where the old code worked and the new code changes the result.
Tests
Four tests added to
tests/test_date_filter.py:test_defilter_removes_date_filter_mid_query— the cases that already worked, so they can't silently regress.test_defilter_removes_date_filter_at_start_of_query— the reported bug, including two adjacent filters and a query that is nothing but a filter.test_defilter_removes_date_filter_after_non_space_character—,and(.test_defilter_removes_all_terms_it_reports— the invariant that ties the two methods together: nothingget_filter_termsfinds may survivedefilter.Red on the unmodified tree:
With the fix:
test_defilter_removes_date_filter_mid_querypasses either way by design — it pins the behavior this change must preserve rather than guarding the regression.Verification
ruff checkandruff format --checkclean on both files, run withruff==0.12.3to match the pin in.pre-commit-config.yaml.trailing-whitespace/end-of-file-fixerhooks).WordFilter.defilterhas no leading-whitespace requirement and handles all these positions correctly, so it needed no change — I verified this rather than assuming it.FileFilter.defilterhas a separate bug (it never strips-file:"..."), already covered by Fix exclude file filters leaking into query in FileFilter.defilter #1345. I deliberately left it alone to avoid conflicting with that PR; this change touches onlyDateFilter.dtfilters istests/test_online_chat_actors.py, whose date tests assert on LLM output and never calldefilter.Disclosure: I could not run the full
pytestsuite locally — it needs Postgres with pgvector plus the--all-extrasinstall. I rantests/test_date_filter.pyagainst both the pristine and fixed trees, and the end-to-end and 280-combination probes above, all against the realDateFiltermodule.