Repository navigation
Retrieve and inject Foundry static (user-profile) memories - #1125
PratikDhanave (PratikDhanave) wants to merge 4 commits into
Conversation
The memory provider only searched with the current turn's items, so a scope's static (e.g. user-profile) memories - returned by an item-less search - were never retrieved. It also returned early when a turn had no search items, skipping memory injection entirely. Fetch static memories once per session with an item-less search, cache them on the session, and prepend them to the contextual memories on every turn, matching the Python provider. Without a session (nowhere to cache) the item-less search is skipped rather than repeated each turn.
github-actions
Bot
added
area:provider
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new cache is unsynchronized and does not isolate cached memories by provider or scope.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
Extends the Foundry memory provider to retrieve and cache static memories per session, then prepend them to contextual memories.
Changes:
- Added item-less static-memory retrieval and session caching.
- Added two-turn regression coverage for caching and injection.
| File | Description |
|---|---|
provider/foundryprovider/memory.go |
Implements static-memory retrieval and caching. |
provider/foundryprovider/memory_test.go |
Tests static-memory injection and reuse. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| memoryStaticInitStateKey = "foundrymemory.staticInitialized" | ||
| memoryStaticMemoriesStateKey = "foundrymemory.staticMemories" |
| if ctx := messagesString(messages); !strings.Contains(ctx, "static profile") || !strings.Contains(ctx, "contextual memory") { | ||
| t.Fatalf("first-turn context = %q, want static + contextual", ctx) | ||
| } |
github-actions
Bot
added
kind:code
This comment has been minimized.
This comment has been minimized.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Generated by Go API Consistency Review Agent · copilot · auto · 88 AIC · ⌖ 10.5 AIC · ⊞ 9.2K
| Options: &azaiprojects.MemorySearchResultOptions{MaxMemories: p.config.MaxMemories}, | ||
| }) | ||
| if err != nil { | ||
| // Leave the session uninitialized so the next turn retries. |
There was a problem hiding this comment.
This staticMemories helper leaves the session uninitialized on a failed item-less search so the next turn retries. The Python reference this PR states it's porting (python/packages/foundry/agent_framework_foundry/_memory_provider.py, before_run) always sets state["initialized"] = True in a finally block regardless of success, caching an empty static-memory list and never retrying the item-less search again for that session (test_static_memories_only_retrieved_once covers this).
So Go will keep re-issuing the item-less static search every subsequent turn after a transient failure, while Python treats the static fetch as one-shot best-effort per session. Since the PR explicitly aims to match the Python provider's caching semantics, consider setting the initialized flag (with an empty cached list) even on failure to match Python, or note this retry-on-failure behavior as an intentional Go-specific divergence.
Qualify the static-memory session-state keys with the memory store name and resolved scope so a session shared across providers, or a scope callback whose result changes between turns, cannot inject another scope's static memories. Also assert prepend order in the test.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by Go API Consistency Review Agent · copilot · auto · 76.1 AIC · ⌖ 7.93 AIC · ⊞ 9.2K
| result, err := p.client.SearchMemories(ctx, p.memoryStoreName, scope, &azaiprojects.MemoryStoresClientSearchMemoriesOptions{ | ||
| Options: &azaiprojects.MemorySearchResultOptions{MaxMemories: p.config.MaxMemories}, | ||
| }) | ||
| if err != nil { |
There was a problem hiding this comment.
The retry-on-failure behavior diverges from the upstream Python provider.
Here, when the item-less static search fails, initKey is left unset so every subsequent turn re-issues the static search until it succeeds (per the "Leave the session uninitialized so the next turn retries" comment).
Upstream python/packages/foundry/agent_framework_foundry/_memory_provider.py (FoundryMemoryProvider.before_run) does the opposite: the try/except/finally block sets state["static_memories"] = [] on failure and unconditionally sets state["initialized"] = True in the finally clause, so a failed static search is attempted only once per session and the provider silently proceeds with no static memories thereafter — it never retries on later turns.
This is a meaningful parity gap: a persistently failing/unavailable memory store causes the Go provider to retry the item-less search on every turn indefinitely (extra latency/cost per turn), while Python fails once and moves on. Consider caching an "attempted" flag (with empty static memories) on failure, matching the Python finally semantics, unless the indefinite-retry behavior is an intentional Go-specific choice.
…memories # Conflicts: # go.mod # go.sum # provider/foundryprovider/memory.go
github-actions
Bot
removed
the
kind:dependencies
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by Go API Consistency Review Agent for #1125 · copilot · auto · 78 AIC · ⌖ 15.6 AIC · ⊞ 13.9K
| if err != nil { | ||
| // Leave the session uninitialized so the next turn retries. | ||
| p.log(ctx, slog.LevelError, "foundrymemory: failed to search static memories", "memory_store", p.memoryStoreName, "error", err) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Parity issue: static-memory failure retry policy diverges from the Python provider.
On a failed item-less static search, this leaves the session un-initialized ("Leave the session uninitialized so the next turn retries"), so every subsequent turn re-issues the static search until it succeeds.
The upstream Python provider does the opposite and does so deliberately. In _memory_provider.py, before_run wraps the static search in try/except/finally and sets state["initialized"] = True in the finally block — i.e. regardless of success or failure — with the explicit comment "Mark as initialized regardless of success to avoid repeated attempts". state["static_memories"] is set to [] on failure. This is also asserted by the upstream test test_static_memories_only_retrieved_once (and the failure-path test around line 293 in test_foundry_memory_provider.py), which expect exactly one static search per session even when it errors.
Go's current behavior (retry on every turn after a failure) means a persistently-failing or slow memory store is hit with a static search on every single agent turn for the life of the session, which is a different reliability/cost profile than upstream intentionally chose.
Suggested fix: set initKey to true (and cache an empty/nil static-memories slice) in the err != nil branch as well, so a failed static fetch is treated as "initialized" and not retried, matching the Python provider's documented behavior.
…memories # Conflicts: # provider/foundryprovider/memory.go
API consistency review
Changed Go contractNo exported API changes. Upstream evidence reviewed
AssessmentThe Go change matches the Python contract: static memories come from a single item-less search per session, are cached, and are prepended on each turn. There is no feature gate upstream, so unconditional behavior is consistent. Go differs from Python in minor ways: it retries the static fetch after a failure, skips it when there is no session, and can inject static memories on turns with no input text. These look intentional and low impact. .NET parity was not verified, so this review covers Python only.
|
github-actions
Bot
added
the
parity-approved
The Foundry memory provider only ran a search with the current turn's items and returned early when a turn had no items. So a scope's static memories (e.g.
user_profile), which the service returns from an item-less search, were never retrieved or injected.The Python provider fetches static memories once per session with an item-less
search_memoriescall, caches them in state, and prepends them to the contextual memories on every turn.Change
staticMemories), cache them on the session (same session-state pattern ashostedsession.go), and prepend them to the contextual memories each turn. Contextual search still runs only when the turn has items. Without a session there is nowhere to cache, so the item-less search is skipped rather than repeated every turn.Itemsis optional on the search options, so an item-less search is valid.Test
TestMemoryProviderInjectsStaticMemories: over one session, the first turn issues an item-less static search + a contextual search and injects both; the second turn reuses the cached static memory (no second item-less search) and still prepends it. Fails before the change (static memory absent), passes after.