Share term normalization between memory and SQLite indexes - #325
Open
Bernhard Merkle (bmerkle) wants to merge 2 commits into
Open
Bernhard Merkle (bmerkle) wants to merge 2 commits into
Bernhard Merkle (bmerkle) wants to merge 2 commits into
Conversation
The memory term index only lowercased terms while the SQLite one also stripped, NFC-normalized and collapsed whitespace, so lookups, get_terms() and size() disagreed between backends. Add a shared normalize_term() and use it in both term indexes and both property indexes (name and value are normalized separately). Also merge, rather than overwrite, terms that collide after normalization in the memory deserialize, and use the shared function for the fuzzy related-terms lists. Fixes microsoft#322 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Copilot started reviewing on behalf of
Bernhard Merkle (bmerkle)
September 28, 2026 22:27
View session
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Removal paths and SQLite deserialization do not consistently preserve normalized keys.
Review effort: Balanced
Findings: 4
Open (6)
What changed in this PR
Centralizes term normalization to align memory and SQLite index behavior.
Changes:
- Adds shared Unicode, whitespace, and case normalization.
- Applies normalization across term, property, and fuzzy indexes.
- Adds backend parity tests and collision-aware deserialization.
| File | Description |
|---|---|
src/typeagent/knowpro/common.py |
Defines shared normalization. |
src/typeagent/knowpro/add_messages.py |
Normalizes fuzzy-index terms. |
src/typeagent/knowpro/conversation_base.py |
Normalizes incremental related terms. |
src/typeagent/storage/memory/semrefindex.py |
Normalizes keys and merges collisions. |
src/typeagent/storage/memory/propindex.py |
Normalizes property names and values separately. |
src/typeagent/storage/sqlite/semrefindex.py |
Uses shared normalization. |
src/typeagent/storage/sqlite/propindex.py |
Uses shared property normalization. |
tests/test_term_normalization.py |
Adds cross-backend parity tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Normalize prop_name in remove_property (memory and SQLite) - Keep the empty normalized term through SQLite deserialize - Don't embed empty normalized terms in the incremental fuzzy update - Update the _collect_related_terms_for_fuzzy_index docstring - Annotate the test fixtures; add tests for removal and empty-term round trip Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This branch has not been deployed
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 #322.
The memory
TermToSemanticRefIndexonly lowercased terms, while the SQLite one also stripped, NFC-normalized and collapsed whitespace. Lookups,get_terms()andsize()therefore disagreed between backends.normalize_term()inknowpro/common.py(the SQLite behavior, promoted to the shared definition) and use it in the memory and SQLite term indexes and property indexes. Property name and value are normalized separately, so leading whitespace in a value is handled.deserializenow merges terms that collide after normalization instead of overwriting.add_messages,conversation_base) use the shared function too.tests/test_term_normalization.pyruns parity checks against both backends.Behavior change: memory-backed index keys are now normalized with the SQLite rules (as the issue suggested).
🤖 Generated with Claude Code