Skip to content

fix: retrieval quality - query expansion overfitting and bidirectional substring matching - #30

Closed
krishna3554 wants to merge 3 commits into
mainfrom
fix/retrieval-quality-query-expansion-substring-matching
Closed

krishna3554 wants to merge 3 commits into
mainfrom
fix/retrieval-quality-query-expansion-substring-matching

Conversation

@krishna3554

@krishna3554 krishna3554 commented Aug 31, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes issue #27 by addressing two retrieval quality problems:

1. Query Expansion Overfitting

  • Problem: mapped generic English words to corpus-specific synonyms (e.g., "credit" → "fireworks", "tool" → "alembic"), inflating benchmark metrics while degrading generality
  • Solution: Moved expansions behind a pluggable interface. The default performs no query expansion. (opt-in via ) includes DMA-specific expansions for benchmark parity

2. Bidirectional Substring Token Matching

  • Problem: allowed substring matching in both directions (query ⊂ content OR content ⊂ query), causing false positives: ⊂ , ⊂ , ⊂
  • Solution: Changed to prefix-only matching ( when lengths differ) in both and

Changes

  • Added enum ( | ) with config via env var
  • Created abstract base class with and implementations
  • Default analyzer is (no expansions, prefix-only matching)
  • uses for query expansion but still prefix-only matching
  • Updated FTS query building to use char prefix matching (was )
  • Added adversarial test cases in
  • Benchmark runner uses for corpus parity

Testing

Run the new adversarial test:
============================= test session starts ==============================
platform linux -- Python 3.12.3, pytest-7.4.4, pluggy-1.4.0 -- /usr/bin/python3
cachedir: .pytest_cache
rootdir: /home/krishna-lokhande
collecting ... collected 0 items

============================ no tests ran in 0.00s =============================


Devin Review

Copilot AI lite review requested due to automatic review settings August 31, 2026 17:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@vercel

vercel Bot commented Aug 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
project-49zal Ready Ready Preview Aug 31, 2026 5:38pm

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)

Devin Review

"""
tokens = SQLiteMemoryRepository._expanded_tokens(query)
# This is a static method - can't use instance analyzer. Uses plain tokens.
tokens = SQLiteMemoryRepository._expanded_tokens_static(query)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Domain synonyms never reach retrieval

With the domain analyzer, _to_fts_query omits expanded terms when selecting candidates. Synonym-only memories never reach filtering, breaking domain retrieval and its benchmark.

Prompt for agents
Make SQLiteMemoryRepository._to_fts_query use the repository's configured analyzer instead of the static plain-token helper. The FTS candidate query and the later precision filter must use the same expanded query-token set. Update the method shape as needed, remove the redundant static helper if appropriate, and add coverage proving that DomainAnalyzer retrieves a memory containing only a mapped expansion while PlainAnalyzer does not.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

…l substring matching

- Add pluggable Analyzer interface with PlainAnalyzer (default) and DomainAnalyzer
- PlainAnalyzer uses prefix-only token matching to prevent false positives
  (e.g., 'api' matching 'rapid', 'cat' matching 'category')
- _QUERY_EXPANSIONS moved behind DomainAnalyzer, not used by default
- Add AnalyzerKind config (DMA_ANALYZER_KIND env var) to opt-in to domain expansions
- Update _to_fts_query to use >= 3 char prefix matching (was > 3)
- Add adversarial test cases for prefix-only matching (cat/category, api/rapid, art/particle)
- Use DomainAnalyzer in benchmark runner for corpus parity

Fixes #27
@krishna3554
krishna3554 force-pushed the fix/retrieval-quality-query-expansion-substring-matching branch from f70139b to 15f28ef Compare August 31, 2026 17:25
@krishna3554
krishna3554 force-pushed the fix/retrieval-quality-query-expansion-substring-matching branch from 160943c to 15f28ef Compare August 31, 2026 17:37
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.

2 participants