Skip to content

Add the provider-agnostic session engine with tiered context compaction - #6

Open
Malcolmnixon wants to merge 4 commits into
mainfrom
feat/sessions-engine
Open

Malcolmnixon wants to merge 4 commits into
mainfrom
feat/sessions-engine

Conversation

@Malcolmnixon

Copy link
Copy Markdown
Member

First increment of the session layer: the provider-agnostic engine. No Copilot or ChatClient wiring yet — that follows.

Why

AgentKit ships guarded tools that behave identically on two backends, but has no session lifecycle, so a long-running agent is impossible. This adds the missing half.

Design, validated by measurement

Rotation, not in-place reduction. One backend resends history each turn, the other keeps it server-side; re-seeding a fresh session is the only reduction both support.

Tiered retention with fixed per-tier budgets, aged only in batches at rotation. Between rotations the context is append-only, which preserves prompt caching; at rotation the cache is invalidated anyway, so consolidation is free there.

Measured over 50 rotations (compressed window, one run per arrangement). Recall by rotations-ago:

Rotations ago Flat single summary Tiered
0-4 100% 100%
5-9 50% 100%
10-14 67% 50%
15-19 0% 100%
20-29 0% 67%
30-39 0% 100%
40-50 0% 50%

Overall 5/23 flat versus 13/17 tiered, and tiered cost 17% less in summarizer tokens (381,440 vs 461,173). A flat rolling summary collapses because repeated re-summarization only ratchets downward — its final context had shrunk to 363 tokens against tiered's 3,158.

Honest cost: tiering yielded about 27% fewer turns of work per rotation, because tier content occupies window space.

Two constraints that came out of measurement

The summarizer runs out of session. Asking the live session to summarize itself consumes context and triggers the provider's own compactor — self-defeating.

Never ask a model to hit a token budget. Requested 9,870-19,741 tokens; got 1,665 and 4,259. Prompt for specificity and content; measure size ourselves.

Scope

Provider-agnostic engine only, deterministic and unit-testable with a fake summarizer. Ships an in-memory provider session so the engine is exercised end to end.

Sessions takes no reference on Core — only \Microsoft.Extensions.AI.Abstractions\ for \AIFunction\ — so an adapter can depend on both with no cycle.

Gates

  • \�uild.ps1: 1978 to 2350 passing
  • \lint.ps1: exit 0
  • \sysml2tools lint: 72 files clean

  • eviewmark --plan --enforce: full coverage
  • ReqStream orphan check: exit 0 (caught 22 orphans on first run, all linked)

Full companion artifact set created: SysML2 model, design, verification, requirements, review sets.

AgentKit shipped guarded tools but no session lifecycle, so a long-running
agent was impossible. This adds AgentKitSessions: an AgentKit-owned session
that keeps its own transcript out of session and, when the context window
fills, consolidates older history into fixed-budget tiers, disposes the
provider session and creates a fresh one seeded with the preserved content.

Rotation rather than in-place reduction is the only reduction mechanism both
provider shapes support - one sends the whole history again each turn, the
other holds it server-side - so the behavior is identical on either. Aging
happens only at rotation and only in the tiers that overflowed, so the
context is strictly append-only between rotations and a provider prompt
cache survives. The context is bounded by construction: system prompt plus
tool declarations plus the sum of the tier budgets, asserted when the
options are constructed.

The summarizer is injected and runs out of session, so the rotation engine
is a pure deterministic function and is unit-tested against a fake with no
model at all. A saturation signal is raised, not acted on, when a
consolidation cannot reduce what it was given.

This increment is the engine only; no Copilot or ChatClient wiring.

Co-authored-by: Copilot <bot@github.com>
Copilot AI lite review requested due to automatic review settings September 15, 2026 08:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds a provider-agnostic session engine with tiered context compaction, provider rotation, deterministic token accounting, and an in-memory provider implementation.

Changes:

  • Adds the Sessions production library and provider/session abstractions.
  • Adds comprehensive unit, integration, and XML documentation tests.
  • Adds design, verification, requirements, SysML2, solution, and README integration.
File summaries
File Description
test/DemaConsulting.AgentKit.Sessions.Tests/XmlDocExampleTests.cs Updated as part of this pull request.
test/DemaConsulting.AgentKit.Sessions.Tests/TokenEstimatorTests.cs Updated as part of this pull request.
test/DemaConsulting.AgentKit.Sessions.Tests/SummarizerTests.cs Updated as part of this pull request.
test/DemaConsulting.AgentKit.Sessions.Tests/SessionTestData.cs Updated as part of this pull request.
test/DemaConsulting.AgentKit.Sessions.Tests/ProviderSessionTests.cs Updated as part of this pull request.
test/DemaConsulting.AgentKit.Sessions.Tests/InMemoryProviderSessionTests.cs Updated as part of this pull request.
test/DemaConsulting.AgentKit.Sessions.Tests/FakeSummarizer.cs Updated as part of this pull request.
test/DemaConsulting.AgentKit.Sessions.Tests/DemaConsulting.AgentKit.Sessions.Tests.csproj Updated as part of this pull request.
test/DemaConsulting.AgentKit.Sessions.Tests/ContextUsageTests.cs Updated as part of this pull request.
test/DemaConsulting.AgentKit.Sessions.Tests/ContextLayoutTests.cs Updated as part of this pull request.
test/DemaConsulting.AgentKit.Sessions.Tests/CompactionPolicyTests.cs Updated as part of this pull request.
test/DemaConsulting.AgentKit.Sessions.Tests/AgentSessionTests.cs Updated as part of this pull request.
test/DemaConsulting.AgentKit.Sessions.Tests/AgentSessionOptionsTests.cs Updated as part of this pull request.
src/DemaConsulting.AgentKit.Sessions/DemaConsulting.AgentKit.Sessions.csproj Updated as part of this pull request.
src/DemaConsulting.AgentKit.Sessions/ContextUsage.cs Updated as part of this pull request.
src/DemaConsulting.AgentKit.Sessions/AgentSession.cs Updated as part of this pull request.
requirements.yaml Updated as part of this pull request.
README.md Updated as part of this pull request.
docs/verification/introduction.md Updated as part of this pull request.
docs/verification/definition.yaml Updated as part of this pull request.
docs/verification/agent-kit-sessions/token-estimator.md Updated as part of this pull request.
docs/verification/agent-kit-sessions/summarizer.md Updated as part of this pull request.
docs/verification/agent-kit-sessions/session-transcript.md Updated as part of this pull request.
docs/verification/agent-kit-sessions/provider-session.md Updated as part of this pull request.
docs/verification/agent-kit-sessions/in-memory-provider-session.md Updated as part of this pull request.
docs/verification/agent-kit-sessions/context-usage.md Updated as part of this pull request.
docs/verification/agent-kit-sessions/context-layout.md Updated as part of this pull request.
docs/verification/agent-kit-sessions/compaction-policy.md Updated as part of this pull request.
docs/verification/agent-kit-sessions/compacting-agent-session.md Updated as part of this pull request.
docs/verification/agent-kit-sessions/agent-session.md Updated as part of this pull request.
docs/verification/agent-kit-sessions/agent-session-options.md Updated as part of this pull request.
docs/verification/agent-kit-sessions.md Updated as part of this pull request.
docs/sysml2/views/design-views.sysml Updated as part of this pull request.
docs/sysml2/model/agent-kit-sessions/token-estimator.sysml Updated as part of this pull request.
docs/sysml2/model/agent-kit-sessions/summarizer.sysml Updated as part of this pull request.
docs/sysml2/model/agent-kit-sessions/session-transcript.sysml Updated as part of this pull request.
docs/sysml2/model/agent-kit-sessions/rotation-engine.sysml Updated as part of this pull request.
docs/sysml2/model/agent-kit-sessions/provider-session.sysml Updated as part of this pull request.
docs/sysml2/model/agent-kit-sessions/in-memory-provider-session.sysml Updated as part of this pull request.
docs/sysml2/model/agent-kit-sessions/context-usage.sysml Updated as part of this pull request.
docs/sysml2/model/agent-kit-sessions/context-layout.sysml Updated as part of this pull request.
docs/sysml2/model/agent-kit-sessions/compaction-policy.sysml Updated as part of this pull request.
docs/sysml2/model/agent-kit-sessions/compacting-agent-session.sysml Updated as part of this pull request.
docs/sysml2/model/agent-kit-sessions/agent-session.sysml Updated as part of this pull request.
docs/sysml2/model/agent-kit-sessions/agent-session-options.sysml Updated as part of this pull request.
docs/sysml2/model/agent-kit-sessions.sysml Updated as part of this pull request.
docs/reqstream/agent-kit-sessions/token-estimator.yaml Updated as part of this pull request.
docs/reqstream/agent-kit-sessions/summarizer.yaml Updated as part of this pull request.
docs/reqstream/agent-kit-sessions/session-transcript.yaml Updated as part of this pull request.
docs/reqstream/agent-kit-sessions/rotation-engine.yaml Updated as part of this pull request.
docs/reqstream/agent-kit-sessions/provider-session.yaml Updated as part of this pull request.
docs/reqstream/agent-kit-sessions/platform-requirements.yaml Updated as part of this pull request.
docs/reqstream/agent-kit-sessions/in-memory-provider-session.yaml Updated as part of this pull request.
docs/reqstream/agent-kit-sessions/context-usage.yaml Updated as part of this pull request.
docs/reqstream/agent-kit-sessions/context-layout.yaml Updated as part of this pull request.
docs/reqstream/agent-kit-sessions/compaction-policy.yaml Updated as part of this pull request.
docs/reqstream/agent-kit-sessions/compacting-agent-session.yaml Updated as part of this pull request.
docs/reqstream/agent-kit-sessions/agent-session.yaml Updated as part of this pull request.
docs/reqstream/agent-kit-sessions/agent-session-options.yaml Updated as part of this pull request.
docs/design/introduction.md Updated as part of this pull request.
docs/design/definition.yaml Updated as part of this pull request.
docs/design/agent-kit-sessions/token-estimator.md Updated as part of this pull request.
docs/design/agent-kit-sessions/summarizer.md Updated as part of this pull request.
docs/design/agent-kit-sessions/session-transcript.md Updated as part of this pull request.
docs/design/agent-kit-sessions/rotation-engine.md Updated as part of this pull request.
docs/design/agent-kit-sessions/provider-session.md Updated as part of this pull request.
docs/design/agent-kit-sessions/in-memory-provider-session.md Updated as part of this pull request.
docs/design/agent-kit-sessions/context-usage.md Updated as part of this pull request.
docs/design/agent-kit-sessions/context-layout.md Updated as part of this pull request.
docs/design/agent-kit-sessions/compaction-policy.md Updated as part of this pull request.
docs/design/agent-kit-sessions/compacting-agent-session.md Updated as part of this pull request.
docs/design/agent-kit-sessions/agent-session.md Updated as part of this pull request.
docs/design/agent-kit-sessions/agent-session-options.md Updated as part of this pull request.
AGENTS.md Updated as part of this pull request.
AgentKit.slnx Updated as part of this pull request.
Review details

Suppressed comments (4)

src/DemaConsulting.AgentKit.Sessions/AgentSession.cs:138

  • The summary says compaction happens before the answer, but the remarks and implementation deliberately compact after the provider returns. This contradicts the public API contract and can mislead callers about whether a send waits for summarization; describe the post-answer ordering in the summary.
    src/DemaConsulting.AgentKit.Sessions/CompactingAgentSession.cs:252
  • The replacement is installed before the old provider is disposed, but Layout and Usage are published only after that await. If an adapter's DisposeAsync throws, the next call uses the replacement seeded from outcome.Layout while the engine still exposes the pre-rotation layout, so provider history and engine accounting diverge. Publish the new layout/state before disposal, or otherwise handle disposal failures while preserving a consistent session state.
    src/DemaConsulting.AgentKit.Sessions/CompactionPolicy.cs:179
  • This public default list is an exposed mutable array. A caller can change it, after which new CompactionPolicy() reads the corrupted defaults even though the shared CompactionPolicy.Default was initialized earlier. Publish a read-only wrapper rather than the array itself.
    src/DemaConsulting.AgentKit.Sessions/ProviderSession.cs:139
  • The ProviderTurn documentation promises immutability, but a non-empty caller-supplied list is stored directly. The producer can mutate that list after returning the turn, changing what the engine records and making the returned turn's history unstable. Copy the entries into a read-only collection.
  • Files reviewed: 93/93 changed files
  • Comments generated: 12
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/DemaConsulting.AgentKit.Sessions/InMemoryProviderSession.cs Outdated
Comment thread src/DemaConsulting.AgentKit.Sessions/AgentSession.cs Outdated
Comment thread src/DemaConsulting.AgentKit.Sessions/AgentSessionOptions.cs Outdated
Comment thread src/DemaConsulting.AgentKit.Sessions/AgentSessionOptions.cs Outdated
Comment thread src/DemaConsulting.AgentKit.Sessions/CompactingAgentSession.cs Outdated
Comment thread src/DemaConsulting.AgentKit.Sessions/ContextLayout.cs Outdated
Comment thread src/DemaConsulting.AgentKit.Sessions/ContextLayout.cs Outdated
Comment thread src/DemaConsulting.AgentKit.Sessions/ProviderSession.cs Outdated
Comment thread src/DemaConsulting.AgentKit.Sessions/RotationEngine.cs Outdated
Comment thread src/DemaConsulting.AgentKit.Sessions/SessionTranscript.cs Outdated
Review of the session engine found five defects and a family of unsafe
collection publications.

The construction bound counted raw tier content, but a seed wraps every
non-empty tier in a labeled record charged per-entry framing, so the seed a
provider receives could exceed the bound with every tier exactly within its
budget - worst for a provider reporting no usage, where that estimate is
what drives rotation. The bound and the conversation estimate now count the
framing, the label is built by one helper shared with the seed so the two
cannot drift, and the options refuse a window that cannot hold it.

A turn was recorded before the provider was asked, so a provider that
honored cancellation left a ghost turn that a later rotation consolidated
and seeded into a session that never saw it. Nothing is recorded now until
the provider accepts the turn.

A positive NaN passed the policy range checks, because every ordered
comparison against NaN is false, and silently disabled rotation or
saturation reporting; it is rejected explicitly, as MemoryOptions already
does. A rotation threshold too small to survive truncation produced zero,
which a conversation of no tokens satisfies, so the derived threshold is
clamped to one token. The in-memory factory recorded sessions through an
unsynchronized list although the factory contract requires concurrent
safety; creation is serialized and the record is handed out as a snapshot.

Every type that publishes an IReadOnlyList now copies the caller's
collection at construction and exposes a read-only view of that copy: an
IReadOnlyList over a bare array can be cast back and written through, which
let a cached token total, a saturation verdict or a validated seed disagree
with its own contents.

Co-authored-by: Copilot <bot@github.com>
Copilot AI review requested due to automatic review settings September 15, 2026 13:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical and moderate issues remain in lifecycle safety, compatibility, validation, cancellation, and transcript handling.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (8)

docs/design/agent-kit-sessions/agent-session.md:48

  • This says the session is "compacting first," but the postconditions below specify—and the implementation does—the opposite: the answer is produced first and the replacement session is prepared afterwards. Update this companion design text to avoid documenting the wrong latency and ordering contract.
Sends one message and returns the answer, compacting first if the window requires it.

docs/reqstream/agent-kit-sessions/session-transcript.yaml:14

  • This requirement says every append produces a new transcript, but the linked SessionTranscript_AppendMany_Empty_ReturnsSameInstance scenario—and the design—deliberately return the existing instance for an empty append. Qualify this as every non-empty append so the requirement matches the specified behavior and its evidence.
    docs/verification/agent-kit-sessions/compacting-agent-session.md:22
  • These arithmetic figures do not describe the central test in CompactingAgentSessionTests: the test uses a 400-token window and 70-token messages/responses, giving a 280-token threshold and about 148 tokens per turn, not a 300-token window, 210-token threshold, and 108-token turns. Update the verification design so its stated boundary can be checked against the test that supplies the evidence.
    src/DemaConsulting.AgentKit.Sessions/AgentSession.cs:141
  • This summary says compaction happens before the answer is returned, but the remarks immediately below and CompactingAgentSession.SendAsync perform it after the provider answer. The public API documentation should describe the actual post-answer ordering so consumers do not assume every threshold-crossing call waits for consolidation first.
    src/DemaConsulting.AgentKit.Sessions/CompactingAgentSession.cs:209
  • The provider has produced turn.ResponseText, but SendAsync still awaits the full RotateAsync (including all summarizer calls and replacement-session creation) before returning it. This contradicts the documented contract that compaction happens after the answer and the caller never waits on the summarizer; a threshold-crossing turn will incur compaction latency or fail instead of returning its answer. Either decouple rotation from the response path and define how its saturation is reported, or revise the contract and documentation to state that SendAsync waits for rotation.
    src/DemaConsulting.AgentKit.Sessions/CompactingAgentSession.cs:226
  • If the live provider's DisposeAsync throws, _disposed is set before the await and the next disposal returns immediately, so cleanup cannot be retried and the provider resource may remain live. Set the terminal state only after successful cleanup or retain a retryable disposal state.
    src/DemaConsulting.AgentKit.Sessions/RotationEngine.cs:363
  • The re-recording after a cascade is a separate consolidation, but only its over-budget case is checked here. If this call returns output at or above the saturation ratio of material, no NoRedundancy signal is emitted, making a failed reduction invisible. Measure alone against its input and report that signal as is done for merged.
    src/DemaConsulting.AgentKit.Sessions/SessionTranscript.cs:305
  • Every single-entry append allocates a new array and copies the entire existing transcript. CompactingAgentSession.SendAsync invokes this once for the user message and again for the provider entries on every turn, so a window containing many short turns incurs quadratic copying and allocation before rotation. Keep the public snapshot semantics, but use chunked/persistent storage or a private append buffer so appending remains sublinear.
  • Files reviewed: 93/93 changed files
  • Comments generated: 5
  • Review effort level: Lite

Comment thread src/DemaConsulting.AgentKit.Sessions/CompactingAgentSession.cs Outdated
Comment thread src/DemaConsulting.AgentKit.Sessions/InMemoryProviderSession.cs
Comment thread src/DemaConsulting.AgentKit.Sessions/ContextLayout.cs
Comment thread src/DemaConsulting.AgentKit.Sessions/RotationEngine.cs
Comment thread docs/verification/agent-kit-sessions/provider-session.md Outdated
Three defects from the second review round, each with a test that fails
without its fix.

CompactingAgentSession completed its rotation state transition after
awaiting the disposal of the session it replaced, so an adapter whose
DisposeAsync threw left the object pointing at the replacement while its
layout and counters still described the old session, and the next turn
would append to - and might rotate - the wrong transcript. The whole
transition now happens before the disposal, in a block containing no
await. A failed disposal is deliberately caught and not reported: by then
the rotation has succeeded and the session is coherent against its
replacement, so reporting a failure would tell the caller the opposite
while repairing nothing. Explicit disposal of the session still
propagates, because a caller that asked for a release is entitled to
learn it did not happen.

ContextLayout.WithTiers validated only the tier count and null entries,
so a public caller could supply a tier whose index or budget disagreed
with the policy - a tier-one slot carrying tier three's budget - leaving
the policy, the tiers, the labels and the bound describing different
hierarchies. Each tier is now checked against
Policy.TierBudgetTokens[index + 1] before it is copied.

RotationEngine.RotateAsync never checked its token before the early
no-work return, so an already-canceled rotation returned a successful
result whenever the transcript already fit. The token is checked after
argument validation, which holds the contract on both paths.

The verification design for ProviderSession claimed
ProviderTurn_Construct_WithEntries_PreservesThemExactly asserts
reference identity; the test asserts equality and the implementation
deliberately copies the entries into owned storage. Reworded to claim
only what the test proves.

Co-authored-by: Copilot <copilot@github.com>
Copilot AI review requested due to automatic review settings September 15, 2026 15:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (11)

Previously missed (2) — in code that hasn't changed since the last review.

src/DemaConsulting.AgentKit.Sessions/RotationEngine.cs:383

  • The second consolidation performed after a cascade is also subject to the saturation policy, but this branch only reports TierOverBudget. If alone is nearly as large as material yet still fits the tier, the rotation returns without a NoRedundancy signal even though that summarizer call produced the exact saturation condition the policy promises to report. Measure the second call's input/output ratio and apply the same saturation check before the budget check.
    src/DemaConsulting.AgentKit.Sessions/AgentSession.cs:141
  • This public summary says compaction happens before the answer, but the implementation and the following remarks deliberately rotate only after the provider has answered. Because this timing determines whether callers wait on summarization before receiving a response, the summary should describe the actual post-answer behavior.

docs/design/agent-kit-sessions/agent-session.md:48

  • This design text repeats the opposite ordering from the implementation: it says the session compacts first, while CompactingAgentSession sends the turn and prepares the replacement only after the answer. That contradiction can lead consumers to expect summarizer latency before a response; describe the post-answer compaction order here as well.
Sends one message and returns the answer, compacting first if the window requires it.

docs/design/introduction.md:246

  • The overview now says there are five systems and the view exposes AgentKitSessions, but the following sentence still says the structure view renders only four shipped systems. That leaves the architecture overview internally contradictory; update the stale count.
The `SoftwareStructureView.svg` above renders all five systems.

docs/reqstream/agent-kit-sessions.yaml:89

  • The system requirement omits the seeded-record framing from the construction bound, but the implementation and child requirements deliberately include it (ContextLayout.MaximumBoundTokens and AgentSessionOptions reject budgets plus SeedFramingTokens). As written, the requirement promises a smaller bound than the code can guarantee, so update the requirement title to include the framing or align the implementation.
          The library shall bound the context it sends to the system prompt, plus the tool
          declarations, plus the sum of the tier budgets, and shall refuse a configuration whose
          effective window cannot accommodate that bound.

src/DemaConsulting.AgentKit.Sessions/AgentSession.cs:76

  • A threshold-triggered call can be a pure re-seed: RotationEngine returns the same layout with zero consolidations when the transcript still fits tier zero, while RotationOccurred is still true because the provider session is replaced. This remark claims every rotation consolidated older history, so describe it as a replacement that may consolidate history.
    src/DemaConsulting.AgentKit.Sessions/CompactingAgentSession.cs:208
  • The rotation threshold is precomputed from options.ProviderWindowTokens, but when a provider reports ContextUsage this branch still ignores Usage.WindowTokens. If the provider's actual limit differs from the configured value (for example, a 400-token provider with 100,000 configured), the session can continue far past the provider window or rotate unnecessarily early, violating the provider-usage contract. Derive the runtime threshold from the reported window and reconcile the public RotationThresholdTokens contract, then add a mismatched-limit test.
    src/DemaConsulting.AgentKit.Sessions/CompactionPolicy.cs:193
  • The documented per-tier constraints allow [int.MaxValue, int.MaxValue], but summing those budgets into the int TotalTierBudgetTokens overflows and throws OverflowException from Enumerable.Sum rather than rejecting the configuration with the documented argument-validation failure. Validate that the total is representable (or change the public total type) before accepting the policy, otherwise the bound cannot be trusted for extreme valid inputs.
    src/DemaConsulting.AgentKit.Sessions/DemaConsulting.AgentKit.Sessions.csproj:64
  • This project now enables ApiMark enforcement for a fifth shipped package, but the ApiMark OTS requirement and verification still enumerate only Core, Tools, Copilot, and ChatClient. As a result, the Sessions package's generated API index is not asserted and the documented four-project coverage remains stale; add the new package to that companion test set and update its counts.
    src/DemaConsulting.AgentKit.Sessions/RotationEngine.cs:115
  • This public constructor snapshots the list but accepts a null element, so [null!] creates an outcome whose IsSaturated is true and whose saturation entry fails later when a consumer inspects it. The other public snapshot types reject null entries; validate saturations before copying it.
    src/DemaConsulting.AgentKit.Sessions/RotationEngine.cs:415
  • After awaiting the summarizer, the engine immediately uses the result without checking the cancellation token. A summarizer that completes despite cancellation (cooperative cancellation is not guaranteed) therefore lets the rotation return successfully and can continue cascading, contrary to the documented requirement that cancellation is honored between consolidations. Check cancellationToken after the await before accepting the result.
  • Files reviewed: 93/93 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread src/DemaConsulting.AgentKit.Sessions/CompactingAgentSession.cs
Comment thread src/DemaConsulting.AgentKit.Sessions/CompactingAgentSession.cs
Two defects from the third review round, each with a test that fails
without its fix.

ProviderTurn left ResponseText out of Entries whenever an adapter
supplied explicit tool-call and result entries, and every consumer
records Entries: the compaction engine's transcript and the in-memory
session's own history. A tool-using turn therefore recorded what was
looked at but not what was concluded, and since rotation seeds the
replacement session from that transcript, an agent that uses tools on
nearly every turn would be reseeded from a history holding almost none
of its own output - the material this package exists to preserve. The
turn now appends the answer itself, as the final entry, on both paths:
the only entry when none were supplied, and after the supplied ones when
they were. Appending it in ProviderTurn rather than in the session is
what makes every consumer agree; an answer added by one consumer would
be missing from the others. An adapter that already ended its entries
with the answer is not made to strip it - a trailing assistant entry
carrying exactly the answer is taken to be the answer - so either
adapter style records it exactly once.

CompactingAgentSession set its disposal flag before awaiting the
provider's release, so a release that threw left the provider holding
server-side state while every later DisposeAsync returned at the flag
and could not retry: a transient provider failure became a permanent
leak. The disposal flag and a new release flag are now separate. The
session is marked disposed from the first call and refuses further turns
either way, but the provider counts as released only once its own
disposal has completed, so a later call tries again. The asymmetry
decided in the previous round is unchanged: a disposal failure during
rotation is still swallowed, while explicit DisposeAsync still
propagates - which is precisely what makes the retry reachable.

Co-authored-by: Copilot <copilot@github.com>
Copilot AI review requested due to automatic review settings September 15, 2026 17:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

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.

2 participants