fix(memory): tokenize on punctuation and all whitespace in extractWords - #1403
Open
Ajithkumar003-dev wants to merge 1 commit into
Open
fix(memory): tokenize on punctuation and all whitespace in extractWords#1403Ajithkumar003-dev wants to merge 1 commit into
Ajithkumar003-dev wants to merge 1 commit into
Conversation
extractWords split text on the literal space character only and lowercased each piece whole, so punctuation stayed attached to the token and tabs and newlines were never split on at all. "The agent works great!" indexed the token "great!", making a search for "great" miss it, and everything after a newline was folded into a single unsearchable token. The same function tokenizes the query, so punctuated queries failed the same way. Split on runs of non-word runes with strings.FieldsFunc instead, mirroring the \w+ tokenization used by adk-python's in-memory memory service. FieldsFunc never yields empty strings, so the empty-token guard is dropped. Fixes google#569 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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.
What
extractWordsinmemory/inmemory.gonow splits text on runs of non-word runes (strings.FieldsFunc) instead of on the literal space character, so punctuation and non-space whitespace no longer become part of an indexed token.Fixes #569
Why
extractWordsbuilds the keyword index forInMemoryServiceand also tokenizes the incoming query, so both sides were affected:"The agent works great!"indexed the tokengreat!, soSearchMemorywith querygreatreturned nothing."first line\nsecond\tline"indexed the single tokenline\nsecond\tline, making every word after the first newline unreachable. Multi-line model responses are the common case here."Why timed-out?"produced the tokentimed-out?, which matched nothing.Implementation note
The issue proposed
strings.Fields+strings.TrimFunc. I usedstrings.FieldsFuncwith a non-word separator predicate instead, for two reasons:TrimFunconly strips punctuation from the ends of a token, so the comma-separated case the issue lists as affected would still break:regions: us-east1,us-west1stays one token.FieldsFunchandles it.re.findall(r'\w+', text)inin_memory_memory_service.py, keeping the two implementations aligned per the Alignment with adk-python guidance in CONTRIBUTING.md. The predicate accepts Unicode letters, Unicode digits and_, matching Python 3's\w.Happy to switch to the
TrimFuncapproach if maintainers prefer to keep hyphenated and comma-joined tokens intact.FieldsFuncnever yields empty strings, so the oldif s == "" { continue }guard is now dead code and was removed.Testing plan
Three cases added to the existing
Test_inMemoryService_SearchMemorytable, one per failure mode above:find events next to punctuation— coversgreat!andregions: us-east1,us-west1find events separated by non-space whitespace— covers\nand\tfind events for a query containing punctuation— covers query-side normalizationEach was verified to fail against the unfixed
extractWordsbefore the fix was applied, so they are genuine regression tests rather than tests written to match new behavior:With the fix applied, the full package passes, including the pre-existing
find eventsand concurrency tests:Also run locally:
go build ./...(whole module, clean),go vet ./memory/(clean),gofmt(clean).extractWordsis unexported and referenced only within thememorypackage, so the change is self-contained.I could not run
go test -racelocally — it requires cgo and there is no C toolchain on my machine. The change introduces no new concurrency, and the existingTest_inMemoryService_SearchMemory_Concurrentpasses without race enabled; relying on CI for the race run.