perf(memory): Harden hybrid automatic recall - #1250
Merged
Merged
Conversation
Keep parallel vector and lexical RRF for automatic recall, add an HNSW cosine index, push the recall distance cutoff into SQL, and hard-cap each retrieval leg so Postgres work stays bounded. Co-Authored-By: David Cramer <david@sentry.io>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Keep each hybrid retrieval leg at least as large as the caller's requested limit so explicit search cannot under-return when one modality is empty.
Cover the San Francisco / America/Los_Angeles store-then-ask-time path so hybrid automatic recall keeps exact timezone tokens usable at answer time. Co-Authored-By: David Cramer <david@sentry.io>
Timezone-aware current-time answers no longer need bash conversion. The remembered-timezone memory eval now seeds the durable fact and requires systemTime(America/Los_Angeles) without shelling out. Co-Authored-By: David Cramer <david@sentry.io>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4677342. Configure here.
Wire seedMemory through the host embedder so vector rows exist, and phrase the timezone preference so FTS overlaps current-time asks. Assert the seed wrote an embedding before the recall turn.
Nobody sets MEMORY_RECALL_MAX_VECTOR_DISTANCE or recallMaxVectorDistance. Keep 0.45 as a store constant for automatic recall only and drop the env and plugin option plumbing. Co-Authored-By: David Cramer <david@sentry.io>
Oxlint deny-warnings flagged the residual conditional spread after the maxDistance config removal.
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.
Automatic memory recall keeps parallel hybrid retrieval and becomes safer under load.
Both legs still run every time and fuse with RRF. This intentionally rejects the #1244 pattern of skipping lexical once any in-threshold vector hit exists, because one soft semantic neighbor can hide an exact/token memory and the miss path becomes serial.
Bounded hybrid probes
Recall uses a smaller overfetch window than explicit search, hard-caps each leg, requires each leg to fetch at least the caller's requested limit (store max 200), caps the lexical rank window at 200 after GIN filtering, and truncates retrieval queries before embedding/FTS construction. Recall also slightly prefers lexical ranks so exact ids/names/timezones survive soft semantic neighbors.
Postgres-safe vector path
Embeddings now have an HNSW cosine index (
vector_cosine_ops, matching<=>). The recall distance cutoff is applied in SQL instead of post-filtering the top-N in JS, so weak neighbors never consume the candidate budget.Timezone-aware current time
systemTimeaccepts an optional IANA timezone so remembered local-time preferences can be answered without bash conversion. The personal-memory timezone eval seeds San Francisco /America/Los_Angelesand requiressystemTime({ timezone: "America/Los_Angeles" })plus a Pacific answer.Coverage
Adds a hybrid-contract regression for lexical-only timezone facts competing with an acceptable vector distractor, a ranking weight unit test, a search-limit fill regression when one modality is empty, systemTime timezone unit coverage, and the remembered-timezone personal-memory eval.
Fixes #1248
Verification:
@sentry/juniortypecheck passed; systemTime offset math checked against a known PST instant. Not run locally: vitest unit/integration (no Docker/Postgres), memory evals. CI on the new head is the proof path.Requested by David Cramer.
--
View Junior Session [Sentry]