Gracefully handle missing scope in UserMemoryStore (fixes #564)#566
Gracefully handle missing scope in UserMemoryStore (fixes #564)#566eanzhao merged 2 commits intofeature/lark-botfrom
Conversation
Channel conversation LLM runs inside Orleans actors with no HTTP auth context. The scope resolver returns null, causing ActorBackedUserMemoryStore to throw InvalidOperationException on every turn and log a warning. - Add TryResolveScopeId/TryResolveWriteActorId that return null instead of throwing - ReadProjectedStateAsync returns null when no scope is available - GetAsync returns UserMemoryDocument.Empty, BuildPromptSectionAsync returns empty string — no warning logged - Write operations (Save/Add/Remove) still throw since they only run from Studio API with auth context Closes #564
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6765f706a9
ℹ️ 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".
| if (actorId is null) | ||
| return null; |
There was a problem hiding this comment.
Preserve failure signal when scope is missing for deletes
Returning null from ReadProjectedStateAsync when scope resolution fails changes RemoveEntryAsync behavior from throwing to silently returning false (because it treats state is null as "entry not found"). In authenticated write paths, this masks a scope-resolution/configuration error as a normal miss, so callers can falsely believe deletion succeeded semantically (no such entry) instead of seeing the actual routing failure and handling it.
Useful? React with 👍 / 👎.
VerdictThe fix correctly addresses issue #564 — However, there are a few things that should be addressed before merging: 1.
|
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## feature/lark-bot #566 +/- ##
====================================================
+ Coverage 72.01% 72.02% +0.01%
====================================================
Files 1255 1255
Lines 90723 90724 +1
Branches 11877 11880 +3
====================================================
+ Hits 65331 65344 +13
+ Misses 20706 20699 -7
+ Partials 4686 4681 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 5 files with indirect coverage changes 🚀 New features to boost your workflow:
|
…, remove dead try-catch
Summary
ActorBackedUserMemoryStorethrowsInvalidOperationExceptionwhen no HTTP auth context is available (Orleans actor / channel conversation)TryResolveScopeId/TryResolveWriteActorIdthat return null instead of throwingReadProjectedStateAsyncreturns null when scope is unavailable →GetAsyncreturnsEmpty→BuildPromptSectionAsyncreturns empty string silentlyVerification
dotnet build— 0 errorsdotnet test test/Aevatar.AI.Tests— 500 passed, 0 failedCloses #564