Skip to content

Features/aii/gtr 13556 - #1455

Open
yuyixg wants to merge 5 commits into
SciSharp:masterfrom
yuyixg:features/AII/GTR-13556
Open

yuyixg wants to merge 5 commits into
SciSharp:masterfrom
yuyixg:features/AII/GTR-13556

Conversation

@yuyixg

@yuyixg yuyixg commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

No description provided.

marsyusms and others added 2 commits October 9, 2026 11:30
… instance

SharpCache keyed reference-type arguments by GetHashCode(), i.e. object identity.
RuleEngine.Triggered builds a new AgentFilter on every call, so every trigger missed
and parked another full copy of all agents in the static MemoryCache for 10 minutes
(OneBrain pods saw-toothing at 80-88% of their 3 GiB limit). perInstanceCache
entries were parked there too, one dead copy per DI scope.

- Build argument keys from content: strings (length-prefixed, SHA-256 when long),
  primitives, ICacheKey, otherwise a SHA-256 of the JSON. An argument without a
  stable key bypasses the cache instead of falling back to identity.
- AgentFilter implements ICacheKey.
- perInstanceCache entries live on the instance (ConditionalWeakTable) and are
  collected with it; ClearCacheAsync still clears them.
- Callers that mutated now-shared cached results work on copies:
  RoutingAgentHook (routable agent fields) and AgentController.GetAgent (inherit).
- GetAgent no longer flags the shared AgentSettings.LlmConfig as inherited.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- RoutingAgentHook: one conversation's filled fields are removed from a copy,
  not from the cached routable agents the next conversation gets.
- AgentController.GetAgent: inheriting and resolving redirect names leave the
  cached agent untouched.
- perInstanceCache entries are collected together with their scope, and
  Utilities.ClearCache still clears them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@qodo-code-review

qodo-code-review Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

Grey Divider

Sorry, something went wrong

We weren't able to complete the code review on our side. Please try again manually by commenting /agentic_review on this PR.

Grey Divider

Qodo Logo

@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Prevent SharpCache agent memory growth and shared-result mutations

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Key cache entries by argument content so repeated agent queries reuse results instead of retaining
 copies.
• Keep per-instance entries with their service instances and copy shared results before
 request-specific changes.
• Add regression tests for cache retention, invalidation, key behavior, and caller isolation.
Diagram

graph TD
  Trigger["Rule triggers"] --> Service["Agent service"] --> Cache["SharpCache"] --> Shared[("Shared cache")] --> Copies["Copying callers"]
  Filter["AgentFilter key"] --> Cache
  Cache --> Instance[("Instance entries")]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Canonical serialization for AgentFilter keys
  • ➕ New filter fields would be included without manually extending GetCacheKey.
  • ➕ Unambiguous encoding could reduce delimiter-related key collisions.
  • ➖ Serialization and hashing add work to a frequent query path.
  • ➖ Canonical handling of property and collection order would need an explicit policy.

Recommendation: Keep the content-key and instance-owned storage strategy: it addresses both sources of retained entries without changing agent query behavior. Consider unambiguous encoding for AgentFilter fields as a follow-up; its manually assembled key depends on delimiter assumptions.

Files changed (9) +611 / -24

Bug fix (6) +166 / -24
Agent.csExpose a complete shallow copy of Agent +6/-0

Expose a complete shallow copy of Agent

• Adds ShallowCopy for callers that must preserve every Agent member while replacing collections they intend to modify.

src/Infrastructure/BotSharp.Abstraction/Agents/Models/Agent.cs

SharpCacheAttribute.csUse content keys and instance-owned cache entries +119/-20

Use content keys and instance-owned cache entries

• Replaces identity-based argument keys with stable content representations and bypasses caching when a key cannot be produced. Moves per-instance entries to weakly associated storage, retains expiry, and clears those entries during cache invalidation.

src/Infrastructure/BotSharp.Abstraction/Infrastructures/SharpCacheAttribute.cs

AgentFilter.csGive AgentFilter a content-based cache key +16/-1

Give AgentFilter a content-based cache key

• Implements ICacheKey using pagination and every filter field so equivalent new filters can share agent-query results.

src/Infrastructure/BotSharp.Abstraction/Repositories/Filters/AgentFilter.cs

AgentService.GetAgents.csAvoid mutating the shared default LLM configuration +2/-1

Avoid mutating the shared default LLM configuration

• Clones the default LLM configuration before marking an agent's inherited configuration, leaving AgentSettings unchanged.

src/Infrastructure/BotSharp.Core/Agents/Services/AgentService.GetAgents.cs

RoutingAgentHook.csIsolate conversation-specific routable fields +14/-0

Isolate conversation-specific routable fields

• Copies routable agents and their field lists before removing fields already present in a conversation, preserving cached results for later conversations.

src/Infrastructure/BotSharp.Core/Routing/Hooks/RoutingAgentHook.cs

AgentController.csApply inheritance to a copy of the cached agent +9/-2

Apply inheritance to a copy of the cached agent

• Copies the fetched agent, its mutable collections, and routing rules before adding inherited items and resolving redirect names for the response.

src/Infrastructure/BotSharp.OpenAPI/Controllers/Agent/AgentController.cs

Tests (3) +445 / -0
BotSharp.Core.UnitTests.csprojReference OpenAPI from the core unit tests +2/-0

Reference OpenAPI from the core unit tests

• Adds the OpenAPI project reference so regression tests can exercise AgentController.

tests/BotSharp.Core.UnitTests/BotSharp.Core.UnitTests.csproj

SharpCacheAgentMemoryTests.csTest agent cache reuse and entry lifetime +312/-0

Test agent cache reuse and entry lifetime

• Exercises the woven agent service with fresh repository results to verify repeated filters reuse one entry, scoped entries are collectible and clearable, and default settings stay unchanged. Also checks argument-key behavior and retained memory.

tests/BotSharp.Core.UnitTests/Infrastructures/SharpCacheAgentMemoryTests.cs

SharpCacheSharedResultTests.csTest isolation of shared agent results +131/-0

Test isolation of shared agent results

• Verifies that routing one conversation does not remove another's cached fields and that an agent response does not add inherited items or redirect names to the cached agent.

tests/BotSharp.Core.UnitTests/Infrastructures/SharpCacheSharedResultTests.cs

marsyusms and others added 3 commits October 9, 2026 14:01
Drop the hand-written AgentFilter.GetCacheKey. The generic SharpCache key already
hashes the filter's JSON, which picks up any field added later; a hand-written key
silently serves wrong results when a new field is left out, and its field-by-field
"_" join let text shift between adjacent free-text fields into the same key.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Build each conversation's routable agents with the already-filled fields left out,
instead of copying the cached agents and then removing fields from the copies.
Same result, one pass, no list mutation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The key prefix was the method name only, so same-named methods on different classes
with equal arguments shared one entry. OneBrain loads both CsvNERDataLoader and
OneDbEntityDataLoader, whose argument-less, cached LoadVocabularyAsync() therefore
served whichever vocabulary loaded first to both for 60 minutes.

The test project now references Rougamo.Fody so tests can weave [SharpCache] onto
their own classes; FodyWeavers.xml is generated on build, as for the src projects.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@JackJiang1234

Copy link
Copy Markdown
Contributor

reviewed

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.

3 participants