镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

fix(core): prevent embedding cache collisions between distinct texts - #1434

Open
pei711 wants to merge 1 commit into
VoltAgent:mainfrom
pei711:fix/core-embedding-cache-collisions
Open

pei711 wants to merge 1 commit into
VoltAgent:mainfrom
pei711:fix/core-embedding-cache-collisions

Conversation

@pei711

@pei711 pei711 commented Oct 4, 2026 •

Copy link
Copy Markdown

PR Checklist

  • The commit message follows the contribution guidelines.

Bugs / Features

  • Related issue: none; reproduced directly against current main.
  • Tests for the changes have been added.
  • Docs: no public API or configuration changes.
  • Changeset added for @voltagent/core.

What is the current behavior?

The embedding cache uses a 32-bit hash plus input length. Distinct texts such as Aa and BB share a key, so a cached embedding can be returned for the wrong input. Batch processing then treats unseen text as cached, and semantic search returns incorrect matches.

What is the new behavior?

Use the full input text as the cache key. Preserve the existing TTL and LRU behavior, including eviction of an empty-string key. Add single/batch cache regressions and public Memory semantic-search coverage.

Notes for reviewers

  • Before the fix, 9 new regression cases failed; after the fix, both affected test files pass (24 tests).
  • Core type checking, CJS/ESM/declaration builds, strict publint, and built CJS/ESM semantic-search checks pass.
  • Full-repository Biome CI passes with LF checkouts; upstream and patched versions have the same 5 existing warnings. Syncpack passes.
  • Full Core suite on Windows/Node 24.13.1: 1,405 passed, 5 failed, 2 skipped, 1 todo. Clean upstream: 1,395 passed with the exact same 5 sandbox failures. Node 24.19.0: 1,408 passed, 2 failed; both failures also reproduce on clean upstream and concern Windows sandbox command parsing/cleanup.
  • Prepared with AI assistance; all verification used offline adapters and local files.

Summary by cubic

Fixes embedding cache collisions in @voltagent/core so distinct texts can no longer return each other's cached embeddings.

  • Replaced the 32-bit hash plus input-length cache key with the full input text; TTL and LRU behavior, including eviction of an empty-string key, are unchanged.
  • Added single and batch cache regression tests plus public Memory semantic-search coverage.
  • @voltagent/core gets a patch changeset.

Written for commit ec2d35d. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Embedding cache entries are now keyed by the full input text, preventing distinct messages with colliding hashes from sharing embeddings.
    • Cache expiration and least-recently-used eviction continue to work correctly, including for empty-text inputs.

@changeset-bot

changeset-bot Bot commented Oct 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ec2d35d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@voltagent/core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7658eaf8-6d46-453f-9e93-dd853dcb7792
📥 Commits

Reviewing files that changed from the base of the PR and between 72a46c7 and ec2d35d.

📒 Files selected for processing (4)
  • .changeset/quiet-embeddings-match.md
  • packages/core/src/memory/semantic-search.spec.ts
  • packages/core/src/memory/utils/cache.spec.ts
  • packages/core/src/memory/utils/cache.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The embedding cache now keys entries by full input text. New tests cover cache operations and verify distinct embeddings through single-message and batch semantic-search saves.

Changes

Embedding cache identity

Layer / File(s) Summary
Full-text cache keys
packages/core/src/memory/utils/cache.ts, packages/core/src/memory/utils/cache.spec.ts, .changeset/quiet-embeddings-match.md
Cache lookup, storage, expiration, and LRU eviction use full input text. Tests cover colliding inputs, replacement, expiration, eviction, and batch retrieval. The changeset records a patch for @voltagent/core.
Semantic-search regression coverage
packages/core/src/memory/semantic-search.spec.ts
Tests cover single-message and batch saves with distinct inputs. They verify that searching for "BB" returns the second message and that each message retains its corresponding vector.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to ec2d3

The cache-key correction appears ready to merge after normal checks, with no actionable risk identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to ec2d3

The correction prevents different texts from reusing the wrong embedding. Complete input text can now remain in memory after expiry or deletion, however; the significance depends on the application's sensitive-data retention requirements.

Retained concerns

  • Low · security · inferred: With caching enabled, complete message and query text can remain referenced after logical expiry or source deletion. Previously unchanged cleanup and deletion behavior retained only compact keys and a short preview; full-text identity widens the raw-content retention scope. Whether this conflicts with deployment privacy requirements is unresolved.
Security review details

Security Blast Radius

  • inferred — The directly affected retention scope is successfully embedded text cached within one Memory instance. If an application shares that instance across users, its cached contents share that owner; deployment-level sharing is not established.

Trust Boundaries and Controls

  • observed — The correction retains the existing text-to-embedding call path and private cache owner. Cache get, has and statistics do not expose raw keys, and no new caller or authorization transition appears in the changed production code.

Resilience and Maintainability Implications

  • observed — A rejected batch embedding call occurs before cache population. Later vector-store failure can leave derived cache entries, but this ordering is unchanged; the PR does not introduce a new partial-write recovery mechanism.

Hardening Proposals

  • proposed — For deployments with sensitive-text retention requirements, define when cached raw-text references must be released and consider scheduled expiry cleanup or deletion-linked invalidation. A byte budget could separately bound the cost of retaining long accepted inputs.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: preventing embedding-cache collisions between distinct texts.
Description check ✅ Passed The description covers the template sections, explains the current and new behavior, and reports tests, changeset status, and reviewer notes. It also clarifies that no related issue or documentation u…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cubic-dev-ai cubic-dev-ai 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.

No issues found across 4 files

Re-trigger cubic

This branch has not been deployed

No deployments
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.

1 participant