Use IMemoryCache for process-local tool embeddings#3
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 328f7305b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/ManagedCode.MCPGateway/Embeddings/McpGatewayInMemoryToolEmbeddingStore.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR updates the gateway’s process-local tool embedding reuse to be backed by IMemoryCache, adds a dedicated DI registration helper for that store, and cleans up legacy/obsolete paths and documentation to match the new design.
Changes:
- Reimplemented
McpGatewayInMemoryToolEmbeddingStoreon top ofIMemoryCache(including deterministic fingerprint-agnostic fallback behavior) and removed the old internal index helper. - Added
AddMcpGatewayInMemoryToolEmbeddingStore()for DI registration and updated tests/docs/ADRs accordingly. - Renamed “legacy” lexical naming to “heuristic” in runtime search internals and refreshed related test data.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/ManagedCode.MCPGateway.Tests/TestSupport/TestToolEmbeddingStore.cs | Updates test embedding store to track embeddings directly after internal index removal. |
| tests/ManagedCode.MCPGateway.Tests/Search/McpGatewaySearchLexicalTests.cs | Renames a test context key from legacy to compatibility. |
| tests/ManagedCode.MCPGateway.Tests/Search/McpGatewayInitializationTests.cs | Adds DI test coverage for AddMcpGatewayInMemoryToolEmbeddingStore() and shared IMemoryCache. |
| tests/ManagedCode.MCPGateway.Tests/Search/McpGatewayInMemoryToolEmbeddingStoreTests.cs | Updates disposal pattern and adds test for “latest embedding wins” fallback behavior. |
| src/ManagedCode.MCPGateway/Registration/McpGatewayServiceCollectionExtensions.cs | Adds AddMcpGatewayInMemoryToolEmbeddingStore() (registers IMemoryCache + store). |
| src/ManagedCode.MCPGateway/Registration/McpGatewayChatOptionsExtensions.cs | Removes obsolete AddManagedCodeMcpGatewayTools(...) shims. |
| src/ManagedCode.MCPGateway/ManagedCode.MCPGateway.csproj | Adds package reference for Microsoft.Extensions.Caching.Memory. |
| src/ManagedCode.MCPGateway/Internal/Runtime/Search/McpGatewayRuntime.TokenSearch.cs | Renames legacy lexical scoring concepts to heuristic lexical scoring. |
| src/ManagedCode.MCPGateway/Internal/Runtime/Core/McpGatewayRuntime.cs | Renames lexical feature weight constant to heuristic naming. |
| src/ManagedCode.MCPGateway/Internal/Runtime/Core/McpGatewayRuntime.Types.cs | Renames retrieval candidate score field to heuristic naming. |
| src/ManagedCode.MCPGateway/Internal/Embeddings/McpGatewayToolEmbeddingStoreIndex.cs | Removes the old internal embedding index helper implementation. |
| src/ManagedCode.MCPGateway/Embeddings/McpGatewayInMemoryToolEmbeddingStore.cs | Implements the store using IMemoryCache and supports DI-owned vs self-owned cache lifetimes. |
| docs/Architecture/Overview.md | Updates architecture diagrams/text to reflect IMemoryCache backing and module list cleanup. |
| docs/ADR/ADR-0004-process-local-embedding-store-uses-imemorycache.md | Adds ADR documenting the IMemoryCache decision and invariants. |
| README.md | Updates embedding-store guidance to use AddMcpGatewayInMemoryToolEmbeddingStore() and clarifies process-local scope. |
| Directory.Packages.props | Adds package version for Microsoft.Extensions.Caching.Memory. |
| AGENTS.md | Adds explicit repo guidance favoring framework caching primitives and removing legacy leftovers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/ManagedCode.MCPGateway/Embeddings/McpGatewayInMemoryToolEmbeddingStore.cs
Show resolved
Hide resolved
328f730 to
feea947
Compare
|
Addressed the memory-cache review feedback in Changes:
Verified locally:
|
Summary
IMemoryCacheAddMcpGatewayInMemoryToolEmbeddingStore()and update README, architecture docs, and ADRsVerification
dotnet restore ManagedCode.MCPGateway.slnxdotnet build ManagedCode.MCPGateway.slnx -c Release --no-restoredotnet build ManagedCode.MCPGateway.slnx -c Release --no-restore -p:RunAnalyzers=truedotnet test --solution ManagedCode.MCPGateway.slnx -c Release --no-buildroslynator analyze src/ManagedCode.MCPGateway/ManagedCode.MCPGateway.csproj -p Configuration=Release --severity-level warningroslynator analyze tests/ManagedCode.MCPGateway.Tests/ManagedCode.MCPGateway.Tests.csproj -p Configuration=Release --severity-level warningcloc --include-lang=C# src tests