Degrade incompatible session history before model calls - #1729
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds a strict session input compatibility gate in LlmSessionActor to stop incompatible persisted or new media before any provider request.
It supports issue #1727 by rejecting sessions that contain unsupported or unknown media modalities when the active model cannot accept them.
Changes:
- Add
ErrorCategory.InputCompatibilityand emit it for local modality incompatibility. - Add
ModelInputCompatibilityto evaluate required vs supported modalities from history and pending media. - Add regression tests for recovery, buffered input, and text-only model behavior, and update OpenSpec and operator guidance.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Netclaw.Actors/Sessions/SessionProtocol.Outputs.cs | Add InputCompatibility error category for local modality mismatch. |
| src/Netclaw.Actors/Sessions/ModelInputCompatibility.cs | Add pure compatibility evaluation and an error message builder. |
| src/Netclaw.Actors/Sessions/LlmSessionActor.cs | Enforce compatibility checks before turn admission and before each model call. |
| src/Netclaw.Actors.Tests/Sessions/SessionInputCompatibilityIntegrationTests.cs | Add integration coverage for recovered history and buffered follow-up behavior. |
| src/Netclaw.Actors.Tests/Sessions/ModelInputCompatibilityTests.cs | Add unit tests for supported, unsupported, combined, tool media, and unknown modalities. |
| src/Netclaw.Actors.Tests/Sessions/ModalityGateTests.cs | Update prior “strip media” behavior to “reject before provider call.” |
| openspec/changes/reject-incompatible-session-history/tasks.md | Track implementation tasks and verification evidence for the change. |
| openspec/changes/reject-incompatible-session-history/specs/netclaw-model-capabilities/spec.md | Add spec requirements and scenarios for full-history compatibility gating. |
| openspec/changes/reject-incompatible-session-history/proposal.md | Describe scope, impact, and non-goals for the compatibility contract. |
| openspec/changes/reject-incompatible-session-history/design.md | Document design decisions and trade-offs for actor-owned compatibility checks. |
| openspec/changes/reject-incompatible-session-history/.openspec.yaml | Define the OpenSpec change metadata. |
| feeds/skills/.system/files/netclaw-operations/SKILL.md | Bump netclaw-operations skill version. |
| feeds/skills/.system/files/netclaw-operations/references/providers.md | Add operator guidance for input compatibility errors vs provider outages. |
| docs/spec/SPEC-002-session-lifecycle-and-protocol.md | Update the turn lifecycle to include the new compatibility checks and error semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Netclaw.Actors.Tests/Sessions/SessionInputCompatibilityIntegrationTests.cs:49
- The recovery integration test seeds the journal only. Issue #1727 requires coverage for both journal recovery and snapshot recovery.
Add a second test that restores a session from a persisted snapshot that contains media, and confirm the actor rejects the turn before any provider call.
public async Task Recovered_image_history_is_rejected_before_model_call()
{
var sessionId = new SessionId("test-channel/recovered-image-compatibility");
var seeder = Sys.ActorOf(Props.Create(() => new SessionEventSeeder($"session-{sessionId.Value}")));
await seeder.Ask<Done>(new TurnRecorded
Aaronontheweb
added
bug
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Comment on lines
+178
to
+182
| foreach (var media in m.MediaReferences) | ||
| { | ||
| if (!AcceptsModality(supportedModalities, (MediaModality)media.Modality)) | ||
| count++; | ||
| } |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
src/Netclaw.Actors/Sessions/SessionMessageAssembler.cs:134
- The assembler inserts the media-filter notice at index 0. This puts a User message before the System prompt and can break strict servers that require System messages first.
// Inject volatile notice when media was stripped from history
var strippedCount = ChatMessageConverter.CountStrippedMedia(
input.State.History, input.SupportedInputModalities);
if (strippedCount > 0)
{
src/Netclaw.Actors/Sessions/LlmSessionActor.cs:2763
- TryRejectIncompatibleInput allows unknown modality values in recovered history to pass through on the history-only path. This conflicts with the fail-closed contract for unknown persisted modalities.
// History-only incompatibility: debug log and let the assembler strip
// incompatible media at wire time. Only new user-supplied media on this
// specific command triggers a hard rejection.
if (pendingMedia.Count == 0)
{
docs/spec/SPEC-002-session-lifecycle-and-protocol.md:55
- This section says the actor does not call any provider when the compatibility check fails. The current implementation strips incompatible historical media and still calls the model, so the spec text is inaccurate.
The actor checks all active media references against the main model input
modalities. The check includes recovered history, new input, buffered input,
and tool-result media. An unknown persisted modality fails closed.
The actor rejects incompatible new input before it changes the session state.
feeds/skills/.system/files/netclaw-operations/references/providers.md:125
- This guidance says the turn stops before any provider call when a recovered session needs an unsupported modality. The current implementation strips incompatible historical media and still calls the model, so this guidance does not match behavior.
A saved session can contain image, audio, or video input from an earlier model.
Netclaw checks the complete active history before each model call. If the new
main model lacks a required modality, the turn stops before any provider or
fallback call.
Comment on lines
+125
to
+135
| // Collect both TurnCompleted events. Neither turn should fail — | ||
| // the buffered image is stripped at assembly time. | ||
| for (var i = 0; i < 2; i++) | ||
| { | ||
| var completed = await subscriber.FishForMessageAsync<TurnCompleted>( | ||
| _ => true, TimeSpan.FromSeconds(10), cancellationToken: TestContext.Current.CancellationToken); | ||
| Assert.NotEqual(TurnOutcome.Failed, completed.Outcome); | ||
| } | ||
|
|
||
| Assert.Equal(2, _chatClient.CallCount); | ||
| } |
Aaronontheweb
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (4)
src/Netclaw.Actors/Sessions/LlmSessionActor.cs:2754
- The code treats an unknown persisted modality as a history-only incompatibility when the current command has no new media. This path only logs and continues. The code then strips the unknown modality on the wire. The operations guidance says Netclaw rejects an unknown saved modality instead of omitting it. Make the actor fail closed for unknown modality values even when pendingMedia is empty.
// History-only incompatibility: debug log and let the assembler strip
// incompatible media at wire time. Only new user-supplied media on this
// specific command triggers a hard rejection.
if (pendingMedia.Count == 0)
{
src/Netclaw.Actors/Sessions/SessionMessageAssembler.cs:133
- The code inserts the media-filtered notice at index 0 before the static system block insert logic runs. This breaks the scan that finds leading System messages. It can insert the static system block before the persisted system prompt in History, which can change model behavior and cache stability. Insert the notice after the leading System messages (and after the static block) instead of at index 0.
{
var notice = BuildMediaStrippedNotice(strippedCount, input.SupportedInputModalities);
messages.Insert(0, new AiChatMessage(
Microsoft.Extensions.AI.ChatRole.User,
notice));
feeds/skills/.system/files/netclaw-operations/references/providers.md:120
- This guidance says the turn stops before any provider or fallback call when recovered history contains an unsupported modality. In this PR, LlmSessionActor.FireLlmCall logs a warning and continues, and SessionMessageAssembler strips incompatible media at assembly time. This means a provider call can still occur with degraded history. Update this section to match the implemented behavior, or change the actor to reject recovered history before any provider call.
Netclaw checks the complete active history before each model call. If the new
main model lacks a required modality, the turn stops before any provider or
fallback call.
src/Netclaw.Actors/Sessions/LlmSessionActor.cs:2650
- FireLlmCall detects incompatible history, but it only logs and continues. This permits a provider call with degraded history on paths that append media to History before FireLlmCall runs (for example, buffered message drain and tool-loop media). This contradicts the operator guidance that says the turn stops before any provider or fallback call for an incompatible session input. Decide if this check must hard-stop the turn (no provider call) or if all docs must describe degrade-and-continue.
This issue also appears on line 2750 of the same file.
var compatibility = ModelInputCompatibility.Evaluate(_model.InputModalities, _state.History);
if (!compatibility.IsCompatible)
{
TurnLog().Warning(
"turn_media_history_incompatible required={Required} unsupported={Unsupported} unknownCount={UnknownCount} " +
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (4)
src/Netclaw.Actors/Sessions/LlmSessionActor.cs:2677
- FireLlmCall logs and continues when the active history has unsupported or unknown modalities. This still calls the provider and contradicts the spec text that says the turn stops before any provider or fallback call. Stop the turn and emit ErrorCategory.InputCompatibility before the call.
var resolved = _recallManager.ResolveForTurn(
recallQuery,
_state,
_sessionId,
_currentTurnSource,
src/Netclaw.Actors.Tests/Sessions/SessionInputCompatibilityIntegrationTests.cs:89
- This test asserts that recovered incompatible history is degraded and the model is called. The linked spec text says the turn must stop before any provider call for unsupported modalities in recovered history. Update the test to expect an InputCompatibility error and zero model calls.
// The session should proceed normally — the historical image is stripped
// at assembly time, not rejected. The model is called with text-only content.
var completed = await subscriber.FishForMessageAsync<TurnCompleted>(
_ => true,
TimeSpan.FromSeconds(10),
src/Netclaw.Actors/Sessions/SessionMessageAssembler.cs:134
- The code inserts the notice at index 0. This puts a User message before the persisted System prompt. Some providers reject this order with "System message must be at the beginning". Insert the notice after the last System message instead.
var notice = BuildMediaStrippedNotice(strippedCount, input.SupportedInputModalities);
messages.Insert(0, new AiChatMessage(
Microsoft.Extensions.AI.ChatRole.User,
notice));
}
src/Netclaw.Actors/Sessions/LlmSessionActor.cs:2790
- TryRejectIncompatibleInput treats history-only incompatibility as a pass when pendingMedia is empty. This allows a text-only command to proceed even when recovered history has unsupported or unknown media. Reject the command in this case so no provider call can occur.
correlationId);
EmitOutput(new ErrorOutput
{
SessionId = _sessionId,
Message = message,
Category = ErrorCategory.InputCompatibility,
CorrelationId = correlationId,
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (6)
src/Netclaw.Actors/Sessions/LlmSessionActor.cs:2763
- The actor logs and continues when the session history contains unknown modality values. ChatMessageConverter then strips that media silently. This contradicts the documented fail-closed behavior for unknown persisted modality values.
// History-only incompatibility: debug log and let the assembler strip
// incompatible media at wire time. Only new user-supplied media on this
// specific command triggers a hard rejection.
if (pendingMedia.Count == 0)
{
TurnLog().Info(
"session_history_media_stripped model={ModelId} required={Required} unsupported={Unsupported} unknown={Unknown} " +
"— historical media references are incompatible with the current model and will be stripped by the assembler",
_model.ModelId,
compatibility.RequiredModalities,
compatibility.UnsupportedModalities,
string.Join(",", compatibility.UnknownModalityValues));
return false;
}
src/Netclaw.Actors/Sessions/SessionMessageAssembler.cs:134
- The media-filter notice inserts at index 0. This can reorder persisted System messages (for example the persisted system prompt) behind the notice. This can change prompt semantics and break the intended System-message prefix ordering.
// Inject volatile notice when media was stripped from history
var strippedCount = ChatMessageConverter.CountStrippedMedia(
input.State.History, input.SupportedInputModalities);
if (strippedCount > 0)
{
var notice = BuildMediaStrippedNotice(strippedCount, input.SupportedInputModalities);
messages.Insert(0, new AiChatMessage(
Microsoft.Extensions.AI.ChatRole.User,
notice));
}
src/Netclaw.Actors/Sessions/LlmSessionActor.cs:2659
- FireLlmCall detects incompatible history but it always continues to the provider call. After the unknown-modality fix, reject unknown modalities here too so tool-loop calls fail closed before provider access.
This issue also appears on line 2750 of the same file.
var compatibility = ModelInputCompatibility.Evaluate(_model.InputModalities, _state.History);
if (!compatibility.IsCompatible)
{
TurnLog().Warning(
"turn_media_history_incompatible required={Required} unsupported={Unsupported} unknownCount={UnknownCount} " +
"model={ModelId} — incompatible media references will be stripped from wire messages by the assembler",
compatibility.RequiredModalities,
compatibility.UnsupportedModalities,
compatibility.UnknownModalityValues.Count,
_model.ModelId);
// Do not fail the turn — ChatMessageConverter.ToAiMessages strips
// incompatible DataContent at assembly time, and the assembler
// injects a volatile system notice. The session stays usable.
}
docs/spec/SPEC-002-session-lifecycle-and-protocol.md:55
- This spec states that the actor does not call any provider when the local compatibility check fails. The current implementation strips incompatible historical media and still calls the model. Update this text, or change the actor to reject incompatible recovered history before provider access.
The actor rejects incompatible new input before it changes the session state.
It checks again before each model call to protect paths that add media during a
turn. The actor emits `ErrorCategory.InputCompatibility` with the unsupported
modalities and recovery guidance. It does not call the primary client,
fallback client, or provider when this local check fails.
feeds/skills/.system/files/netclaw-operations/references/providers.md:125
- This guidance says the turn stops before any provider or fallback call when history contains unsupported media. The current behavior strips incompatible historical media and continues the provider call. Update this section to match the implemented degrade behavior, and keep the guidance about rejecting unknown modality values.
A saved session can contain image, audio, or video input from an earlier model.
Netclaw checks the complete active history before each model call. If the new
main model lacks a required modality, the turn stops before any provider or
fallback call.
The error names the unsupported modalities and the active model. Select a model
that accepts those modalities, or start a new conversation. Do not diagnose
this result as a provider outage. Netclaw also rejects an unknown saved modality
value instead of omitting that media.
openspec/changes/reject-incompatible-session-history/specs/netclaw-model-capabilities/spec.md:8
- The OpenSpec change requires the actor to reject unsupported recovered history before any provider call, but the current code and tests degrade (strip) recovered history instead. Resolve this mismatch by updating the OpenSpec change text, or by changing the runtime behavior to reject.
The session actor SHALL check all active persisted media and all new media against the active model input modalities before each model call.
The check SHALL include recovered history and media that a tool adds during the current turn.
The actor SHALL reject an unsupported or unknown modality before any primary, fallback, or provider client receives a request.
The actor SHALL preserve all original media references and SHALL identify the incompatible modalities in the session error.
Aaronontheweb
commented
Aug 4, 2026
| /// were stripped from the wire message list because the active model does | ||
| /// not support their modality. | ||
| /// </summary> | ||
| internal static string BuildMediaStrippedNotice(int strippedCount, ModelModality supported) |
| Unknown, | ||
|
|
||
| /// <summary>The active model cannot accept the complete session input.</summary> | ||
| InputCompatibility |
|
|
||
| foreach (var media in msg.MediaReferences) | ||
| { | ||
| if (!AcceptsModality(supportedModalities, (MediaModality)media.Modality)) |
Aaronontheweb
force-pushed
the
fix/1727-session-modality-compatibility
branch
from
August 4, 2026 03:50
9cab5b6 to
0277cfd
Compare
When a session contains historical media the current model cannot accept (e.g. images from a multimodal model in a text-only session), degrade gracefully: - ChatMessageConverter strips incompatible DataContent at assembly time without touching persisted MediaReferences. - SessionMessageAssembler injects a volatile [system: media-filtered] notice so the user knows content was omitted. - FireLlmCall logs a warning instead of failing the turn. - TryRejectIncompatibleInput only hard-rejects new user-supplied media (defense-in-depth; the adapter already handles this). The session stays usable. The assembler boundary is the single correct place to filter — after history reconstruction, before the provider. Tests: 11/11 pass (compatibility unit tests + modality gate tests + integration tests for recovered history and buffered drain paths).
Without the gate, the first FakeChatClient response completes instantly and the second turn's drain + model call races against the FishForMessageAsync timeout. Gating the first response ensures both TurnCompleted events arrive deterministically.
This guard was removed in the original PR but should have been kept. It catches media that slipped past the adapter's capability gate (a contract violation in netclaw-input-adapters), strips the offending references, logs an operator-visible error, and appends a system notice so the user knows something went wrong. This is the new-message counterpart to the assembly-time history filter added in the previous commit — both degrade instead of rejecting.
This reverts commit 8921afa.
Aaronontheweb
force-pushed
the
fix/1727-session-modality-compatibility
branch
from
August 4, 2026 04:49
0277cfd to
abbc622
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
[system: media-filtered]notice so the user knows content was omitted.Design
Two layers, zero new actor states:
ChatMessageConverter.ToAiMessages— accepts asupportedModalitiesmask. IncompatibleDataContentis silently dropped; persistedSerializableChatMessage.MediaReferencesare untouched.SessionMessageAssembler.Assemble— when media is stripped, injects a volatile User-role notice. Not persisted — vanishes on next turn if the model changes back.The
LlmSessionActor.FireLlmCallcompatibility check now logs a warning instead of failing the turn.TryRejectIncompatibleInputonly hard-rejects new user-supplied media (defense-in-depth; adapters already gatekeep this).Why degrade instead of reject
The original incident (#1727) was failover misclassification — DeepSeek 400 was treated as a provider outage, Qwen fallback silently masked it. The fix is preventing incompatible content from crossing the provider boundary. Degrading at assembly time achieves that without wedging the session or forcing "start a new conversation."
The image proxy (#1730) upgrades this path: instead of stripping, it calls a proxy model to produce a text description. Both PRs use the same detection code — #1729 is the no-proxy fallback.
Validation
dotnet test Netclaw.slnx --no-restore: pending.dotnet slopwatch analyze: pending.pwsh ./scripts/Add-FileHeaders.ps1 -Verify: pending.Closes #1727