Retrieve and inject Foundry static (user-profile) memories - #1125
PratikDhanave (PratikDhanave) wants to merge 2 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.
|
Scope: user-visible behavior (Foundry memory retrieval semantics), tests Changed Go contract: Upstream evidence reviewed:
Result: findings reported. This PR correctly ports the Python provider's item-less "static memory" fetch-once/cache/prepend behavior, closing a real parity gap where Go previously never retrieved static (e.g. One divergence was flagged inline: on a failed static search, Go leaves the session "uninitialized" so it retries the item-less search on every subsequent turn, whereas Python's
|
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.
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.