Skip to content

Keep the original input under FreshContextPerIteration with ContinueWithMessages - #1063

Open
PratikDhanave (PratikDhanave) wants to merge 1 commit into
microsoft:mainfrom
PratikDhanaveFork:loop-freshcontext-keeps-initial
Open

PratikDhanave (PratikDhanave) wants to merge 1 commit into
microsoft:mainfrom
PratikDhanaveFork:loop-freshcontext-keeps-initial

Conversation

@PratikDhanave

Copy link
Copy Markdown
Contributor

Problem

nextMessages (agent/harness/loop/loop.go) returned the evaluator's explicit ContinueWithMessages messages before the FreshContextPerIteration branch:

if len(evaluation.Messages) > 0 {
    cloned := cloneMessages(evaluation.Messages)
    return cloned, cloned   // returns explicit-only, bypassing fresh re-seed
}
if cfg.FreshContextPerIteration { ... rebuild from InitialMessages ... }

So when an evaluator returns ContinueWithMessages(...) while FreshContextPerIteration is enabled, the reinvocation gets only the explicit messages. Fresh mode also resets the session to a pristine snapshot taken before iteration 1 (which does not yet contain the original request), so the original user input is lost from both the message list and the session.

This contradicts the FreshContextPerIteration doc: "restarts each reinvocation from the original input messages plus an aggregated feedback log …".

Fix

Check FreshContextPerIteration first: always re-seed InitialMessages, and when the evaluator also supplied explicit messages, compose them on top of the fresh initial context (surfacing the explicit messages). Non-fresh mode is unchanged — explicit ContinueWithMessages is still sent verbatim (locked by TestLoop_ContinueWithMessagesSendsMessagesVerbatim).

Test

TestLoop_FreshContextPerIteration_ContinueWithMessagesKeepsInitial runs a fresh-context loop whose evaluator returns ContinueWithMessages(["explicit"]) and asserts the reinvocation still contains the original "original" input plus the explicit message. Fails before the fix ([explicit] only), passes after; the existing fresh-context + feedback test remains green.

…ithMessages

nextMessages returned the evaluator's explicit ContinueWithMessages messages
before the FreshContextPerIteration branch, so in fresh mode the reinvocation
received only the explicit messages. FreshContextPerIteration also resets the
session to a pristine snapshot taken before iteration 1 (which does not contain
the original request), so the original user input was lost from both the message
list and the session - contradicting the documented contract that fresh mode
'restarts each reinvocation from the original input messages'.

In fresh mode, always re-seed InitialMessages and compose the explicit messages
on top; non-fresh behavior (explicit messages verbatim) is unchanged.
Copilot AI lite review requested due to automatic review settings September 13, 2026 13:11
@github-actions github-actions Bot added area:agent Changes files in the agent area size:medium At most 100 changed lines across at most 5 files labels Sep 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

Final comments identify only minor nits, with no approval-blocking issues.

Pull request overview

This pull request preserves the original input when fresh-context iterations also include explicit continuation messages.

Changes:

  • Re-seeds fresh iterations from initial messages.
  • Appends explicit continuation messages.
  • Adds regression coverage.
File summaries
File Summary
agent/harness/loop/loop.go Updates fresh-context message composition.
agent/harness/loop/loop_test.go Adds continuation-message regression coverage.
Review details

Suppressed comments (3)

agent/harness/loop/loop.go:293

  • This fresh-mode path introduces an undocumented exception to the public contracts: Evaluation.Messages is documented as being sent verbatim, while FreshContextPerIteration is documented as rebuilding the original input plus aggregated feedback. Here the request is InitialMessages plus explicit messages and the aggregated feedback message is skipped. Please update the API comments to document this precedence so callers can predict mixed evaluations.
		if len(evaluation.Messages) > 0 {
			explicit := cloneMessages(evaluation.Messages)
			nextMessages = append(nextMessages, explicit...)
			return nextMessages, explicit

agent/harness/loop/loop_test.go:581

  • This test only inspects the provider input; it would still pass if the new fresh-mode path returned no surfaced explicit message. Assert the collected response contains explicit, as the existing non-fresh ContinueWithMessages test does, so the second return value is covered too.
	if _, err := a.RunText(context.Background(), "original").Collect(); err != nil {
		t.Fatal(err)
	}

agent/harness/loop/loop_test.go:581

  • The reported regression also includes the reset session, but this test uses an implicit per-run session, for which the default history provider is disabled. A regression that still omits original from the fresh session's stored history would therefore pass; add an explicit session/history provider and inspect the second call's session state as well.
	if _, err := a.RunText(context.Background(), "original").Collect(); err != nil {
		t.Fatal(err)
	}
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

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

@github-actions github-actions Bot added kind:code Changes production behavior or code kind:tests Changes tests, fixtures, or test infrastructure pending-auto-risk labels Sep 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Scope: public API, user-visible behavior

Changed Go contract: nextMessages in agent/harness/loop/loop.go — when Config.FreshContextPerIteration is true and an evaluator returns ContinueWithMessages(...), the loop now composes the evaluator's explicit messages on top of loopCtx.InitialMessages (nextMessages = append(cloneMessages(loopCtx.InitialMessages), explicit...)) instead of sending them verbatim.

Upstream evidence reviewed:

  • .NET: dotnet/src/Microsoft.Agents.AI/Harness/Loop/LoopAgent.cs, EvaluateAndBuildNextAsync — the winner.Messages is not null branch returns LoopNextStep.Continue(winner.Messages, ...) before BuildNextMessages (which re-seeds context.InitialMessages) is ever called. Fresh-context mode only recreates/restores the session (CreateFreshIterationSessionAsync), it never composes InitialMessages with the evaluator's explicit messages.
  • .NET doc comment (LoopAgent.cs remarks, lines ~40-47): "An evaluator may instead supply the exact next messages via LoopEvaluation.ContinueWithMessages, bypassing this construction" — i.e., bypassing the fresh-context re-seed of InitialMessages, not composing with it.
  • .NET tests: RunAsync_Fresh_WithContinueWithMessages_RecreatesSessionAsync and RunAsync_Fresh_WithCallerSession_AndContinueWithMessages_ClonesFromSnapshotAsync (dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/Loop/LoopAgentTests.cs) assert capture.MessagesPerCall[1] / [2] equal exactly ["explicit"] — the original input text ("go") is not present — while confirming the session is still reset each iteration.
  • Python: python/packages/core/agent_framework/_harness/_loop.py, _resolve_next_message — the next_message callable's return value is sent as the entire next input; fresh_context only affects whether the default nudge falls back when next_message returns None, and controls session/progress-log composition, not composition with an explicit override.

Result: findings reported — a semantic divergence from the upstream (.NET/Python) FreshContextPerIteration/fresh_context contract. Both upstream implementations treat an evaluator's explicit "continue with these messages" as a full override of the next input (verbatim), independent of fresh-context mode, which only resets/rebuilds session and default-input state. This Go PR instead prepends InitialMessages ahead of the evaluator's explicit messages, changing what the wrapped agent receives on that iteration and diverging from the documented/tested cross-repo behavior. See the inline comment for the specific lines and suggested resolution.

No exported Go API surface (types/signatures) changed — nextMessages is unexported — but the observable runtime behavior for public loop.Config.FreshContextPerIteration + loop.ContinueWithMessages changed, which is in scope per the review charter.

Generated by Go API Consistency Review Agent · copilot · auto · 102.1 AIC · ⌖ 5.39 AIC · ⊞ 9.6K ·

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generated by Go API Consistency Review Agent · copilot · auto · 102.1 AIC · ⌖ 5.39 AIC · ⊞ 9.6K

Comment on lines 289 to +293
nextMessages := cloneMessages(loopCtx.InitialMessages)
if len(evaluation.Messages) > 0 {
explicit := cloneMessages(evaluation.Messages)
nextMessages = append(nextMessages, explicit...)
return nextMessages, explicit

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Parity issue with upstream FreshContextPerIteration + explicit "continue with messages" behavior

When FreshContextPerIteration is true and the evaluator supplies explicit messages, this composes them on top of loopCtx.InitialMessages (nextMessages = append(cloneMessages(loopCtx.InitialMessages), explicit...)). Both upstream implementations treat an evaluator-supplied explicit next-input as a full, verbatim override of the message list — not something that gets composed with the initial/original input — while FreshContextPerIteration/fresh_context only governs session and default-feedback-input state, not this override.

  • .NET: dotnet/src/Microsoft.Agents.AI/Harness/Loop/LoopAgent.cs, EvaluateAndBuildNextAsync returns LoopNextStep.Continue(winner.Messages, ...) verbatim when winner.Messages is not null, before BuildNextMessages (the method that re-seeds InitialMessages) is ever invoked. The doc comment states ContinueWithMessages "bypass[es] this construction" (the fresh-context re-seed).
  • .NET test RunAsync_Fresh_WithContinueWithMessages_RecreatesSessionAsync asserts capture.MessagesPerCall[1] == ["explicit"] exactly (no trace of the original "go" input), while confirming the session is still reset each iteration — i.e. fresh-context only resets session state, not the message override.
  • Python: python/packages/core/agent_framework/_harness/_loop.py, _resolve_next_message sends next_msgs (the caller's explicit override) as the entire next input; fresh_context there only changes the default-nudge fallback and session/progress handling, not an explicit override.

Suggested resolution: when evaluation.Messages is non-empty under FreshContextPerIteration, send those messages verbatim (as the pre-PR non-fresh branch already does), and let fresh-context mode continue to reset only the session state — matching ContinueWithMessages's documented "bypass" semantics in both upstream SDKs. If the original bug report's concern (losing the original input entirely) still needs addressing, that should be solved via a separate opt-in mechanism analogous to upstream rather than by unconditionally prepending InitialMessages to every explicit-messages continuation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:agent Changes files in the agent area kind:code Changes production behavior or code kind:tests Changes tests, fixtures, or test infrastructure size:medium At most 100 changed lines across at most 5 files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants