fix: read search hit text from chunk — search() currently drops 100% of results - #83
Open
fabio-bergmann wants to merge 1 commit into
Open
Conversation
`/v4/search` and `/v4/profile` return the text of a hit in `chunk`, but `SupermemoryClient.search()` mapped it as `r.content || r.memory || r.context || ''`. `memory` therefore came back empty, and the `dedupe(mapped, (r) => r.memory)` on the next line dropped every entry whose key was empty — so `total` reported the true hit count while `results` was always `[]`, and every consumer printed "No memories found". Writes were unaffected, so the plugin looked healthy while silently accumulating memories it could never retrieve. `getProfile()` mapped `searchResults` the same way, which took the SessionStart context hook down with it. - extract the fallback chain into `searchResultText()` and add `chunk`. The two call sites had drifted apart (only one listed `memory`); they now share one definition. - key the search dedupe on the existing `searchResultKey()`, which falls back to `id:` — so a hit with no recognised text field is returned rather than discarded. The profile dedupe keeps the raw text as its key to stay deduped against profile facts via the shared `seen` set, with the same id fallback. - cover both with unit tests, using a verbatim `/v4/search` hit. Fixes supermemoryai#81 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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 #81.
/v4/searchreturns the text of a hit inchunk.SupermemoryClient.search()mapped it asr.content || r.memory || r.context || '', somemorycame back empty and thededupe(mapped, (r) => r.memory)on the next line dropped every entry with an empty key.totalreported the true count whileresultswas always[].Writes are unaffected, so the plugin looks healthy while accumulating memories it can never retrieve — and
/supermemory:statusstill says "connected", since it only checks that a key is present.getProfile()mappedsearchResultsidentically, which took the SessionStart context hook down too.Changes
searchResultText()inresult-merge.jsholds the fallback chain, now includingchunk. The two call sites had drifted apart — only one listedmemory— and now share one definition.searchResultKey(), which falls back toid:. A future field rename then costs formatting, not every result. The profile dedupe keeps raw text as its key so hits still dedupe against profile facts through the sharedseenset, plus the same id fallback./v4/searchhit.npm test5 → 8,npm run lintclean.plugin/scripts/*.cjsrebuilt vianpm run build(5 bundles carry the mapper).Note on testability
The mapper had no test coverage because
src/lib/supermemory-client.jscan't berequired —src/lib/validate.jsis committed as minified ESM whilepackage.jsonis"type": "commonjs", so requiring it throwsCannot use import statement outside a module. Extracting the chain intoresult-merge.jsis what makes this regression testable at all. That packaging issue seems worth fixing separately.Two of the three tests genuinely require the new helper; the third passes on
mainalready —searchResultKeyhad the id fallback andmergeSearchResponsesused it. It guards that property rather than proving the bug.Left out
The other hardening idea from #81 — warning when
total > 0 && results.length === 0— adds console output to a module the hooks parse, so I kept it out of this PR. Happy to add it if you want it.Verified against a live container: the same query that returned nothing now returns the memory at ~70% relevance.