diff --git a/openspec/changes/compaction-rework/.openspec.yaml b/openspec/changes/compaction-rework/.openspec.yaml new file mode 100644 index 000000000..11393eacc --- /dev/null +++ b/openspec/changes/compaction-rework/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-04-11 diff --git a/openspec/changes/compaction-rework/design.md b/openspec/changes/compaction-rework/design.md new file mode 100644 index 000000000..5a1cc6ac9 --- /dev/null +++ b/openspec/changes/compaction-rework/design.md @@ -0,0 +1,328 @@ +# Design: compaction-rework + +## Context + +Netclaw's session compaction pipeline lives in +`LlmSessionActor` + `SessionCompactionPipeline` + `ObservationPromptBuilder` ++ `ExtractiveSessionReducer`. It runs as a three-phase tiered process: +clear old tool results (phase 1), extractive reduction to keep last N +non-system messages (phase 2), and an observer LLM call that summarizes +the discarded portion into a bullet list wrapped with an +`[observations from earlier in this session]` header and inserted as a +`User`-role message at index 0 of the compacted history (phase 3). + +Three failure modes were observed or confirmed: + +1. **Grounding stripping**: the observer system prompt explicitly says + "preserve tool names and outcomes but not full tool arguments/results", + and the user-prompt renderer collapses every tool call to `[Called: + {name}]`. Observer loses WHAT was grepped, WHAT file was read. +2. **Session-id conflation**: the observer is invoked with no knowledge of + the self session ID, so if the discarded window references a *different* + session ID (e.g. the agent was investigating another session via a + tool), the observation can conflate them and produce "user asked about + session X" without marking whether X is self or foreign. +3. **Second-compaction decay**: on the N+1'th compaction, the observation + message from compaction N sits as a `User` message in the discarded + window, and the observer re-summarizes its own summary plus new turns. + Grounding loss compounds geometrically. + +There is also a latent correctness bug: `ExtractiveSessionReducer` slices +history by count, which can leave a `Tool`-role message at the kept +window's first position — its matching assistant `FunctionCallContent` is +in the discarded portion, and providers (OpenAI, Anthropic) reject +messages that reference `tool_use` IDs not present in the request. The +existing `netclaw-session` spec requires pair integrity but the code does +not enforce it. + +This design is informed by source-level reads of four real LLM harnesses. +Detailed research citations live in the plan file +(`/home/petabridge/.claude/plans/proud-honking-goose.md`). The three +adopted ideas are (a) Cline's 9-section structured summary template, (b) +Cline's monotonic compaction boundary that prevents re-summarization, and +(c) OpenCode's truncate-only-at-user-message-boundaries rule for pair +integrity. All three are battle-tested in production LLM harnesses. + +## Goals / Non-Goals + +**Goals:** + +- Produce a compaction summary whose format survives multiple successive + compactions with arithmetic (not exponential) grounding decay +- Disambiguate the self session from any foreign session IDs referenced + in the discarded window +- Guarantee tool call/result pair integrity at the compaction boundary + (already required by the existing spec, newly enforced in code) +- Keep the compaction trigger paths unchanged (threshold + overflow + recovery both continue to work) +- Zero breaking changes to journal format for existing compacted sessions + — old `SessionCompacted` events continue to deserialize and replay + +**Non-Goals:** + +- Durable task-state grounding (`WorkingContext` with `RecentFiles` / + `OpenGoals` / `ProgressMarkers`) — that is the `working-context-grounding` + change that stacks on this one +- Session `CurrentWorkingDirectory` + project-scoped identity file + re-reading — tracked as GitHub issues #595 and #596 +- Authoritative CWD for path-taking tool calls — GitHub issue #596 +- Files-as-source-of-truth refactor (stop persisting file contents in + history, re-read from disk on demand) — called out in the plan as the + deepest architectural direction but explicitly out of scope +- Checkpoint/rollback UI — possible as a follow-up by adding an explicit + deleted-range index (similar to Cline's approach) but no UI ships in + this change +- Eval suite compaction regression cases — tracked separately + +## Decisions + +### Decision 1: Structured 9-section summary borrowed from Cline + +**Chosen**: Rewrite the observer system prompt to produce output with +nine fixed sections, adapted from Cline's +`src/core/prompts/contextManagement.ts:10-110`. Sections: + +1. Primary Request and Intent +2. Key Technical Concepts +3. Files and Code Sections +4. Problem Solving +5. Pending Tasks +6. Task Evolution (**with direct quotes from user messages that changed + the task**, borrowed verbatim from Cline — this is the structural + anti-drift rule) +7. Current Work +8. Next Step +9. Required Files (bullet list of paths the agent should re-read on resume) + +**Alternatives considered**: + +- *Keep free-form bullet list, just improve the prompt*: what the aborted + PR1 tried. Middle-ground, doesn't fix the decay problem on successive + compactions, and three of four researched systems use structured + sections for a reason. +- *OpenCode's 5-section template* (Goal / Instructions / Discoveries / + Accomplished / Relevant files): simpler but missing Cline's "Task + Evolution with direct user quotes" anti-drift rule, which is the single + most important structural defense against drift. The 9-section variant + has been through more iterations of real-world validation. +- *Aider's free-form summarization*: correlates with the simplest design + in the four researched systems. Works because Aider re-reads files from + disk every turn, so conversation grounding matters less. Netclaw does + not (yet) re-read files that way, so conversation grounding matters + more, so we need the structure. + +**Why Cline's 9-section over OpenCode's 5-section**: the Task Evolution +section with direct user quotes. The Slack failure that triggered this +rework was a drift scenario — foreign session ID mentioned earlier, +conflated after compaction. Direct user quotes are the strongest +available structural defense short of actual durable state. + +### Decision 2: Distinctive summary header instead of a persisted boundary index + +**Chosen**: Wrap the structured summary with a distinctive +`[session-summary session:{id}]` header. The header is the only +recognition marker — there is no separately-persisted index pointing +at the summary position. Consumers that need to find the summary walk +history looking for a User-role message whose content starts with the +header prefix. The reducer's user-message-boundary walk-back (Decision 4) +naturally preserves these messages because they are User-role and are +never cut mid-pair. + +**Alternatives considered and rejected**: + +- *Store the summary as a System-role message*: mid-history System + messages confuse some providers and force the actor to track an + explicit "this is the summary" index separately anyway. +- *Persist a `CompactionBoundaryIndex` on `SessionState`*: tried this + initially but found zero consumers actually read it. Pure debt — an + invariant to maintain with no present-day payoff. Header-based + recognition gives equivalent behavior without persisting derivable + state. A future consumer can walk history and find the last header + on demand; at ~50-200 messages per session, this is free. +- *Cline's `conversationHistoryDeletedRange: [start, end]`*: more + general than either of the above (supports arbitrary gaps) but + Netclaw doesn't need gap semantics. Adopt if and when a concrete + rollback feature needs it. + +**Persistence implications**: none. No new fields on `SessionState`, +`SessionSnapshot`, or `SessionCompacted` for this decision — only the +header change in the summary message content. + +### Decision 3: Observer receives self `SessionId` in its system prompt + +**Chosen**: Thread `SessionId` through `CompactionParameters` → +`SessionCompactionPipeline.ExecuteAsync` → +`SessionCompactionPipeline.GenerateObservationsAsync` → +`ObservationPromptBuilder.BuildObservationSystemPrompt(SessionId)`. The +system prompt embeds `"You are summarizing a session with id {id}. This +is the self session. If observations reference OTHER session IDs from +tool calls or user content, mark them explicitly as `session:{id}` and +never conflate them with the summarizing session."` + +**Alternatives considered**: + +- *Let the model infer self session from tool-call context*: fragile, + relies on the model noticing the distinction. Not reliable on smaller + models. +- *Scrub foreign session IDs before passing to observer*: brittle + (regex matching on ID formats), loses information the observer might + legitimately want to reference. + +### Decision 4: Truncate only at user-message boundaries — OpenCode's rule + +**Chosen**: Update `ExtractiveSessionReducer` to walk backward from the +naive cutoff (`list.Count - keepCount`) until it hits a `User`-role +message that is not a system nudge (not prefixed with +`SessionState.SystemNudgePrefix`). The kept window always starts on a +user message. + +**Why this works**: in a well-formed MEAI conversation, tool call/result +pairs are bookended by `User → Assistant → Tool → Assistant → User`. A +truncation that starts on a `User` message cannot split a pair — the +pair's `Assistant` (with `FunctionCallContent`) and `Tool` messages are +contiguous and either both in the kept window or both discarded. System +nudges are `User`-role messages containing recall content or empty- +response nudges; they are legitimate conversation input but not user +turns, so we skip them. + +**Alternatives considered**: + +- *Walk forward past orphan Tool messages* (what the aborted PR1 did): + wrong direction. Skipping past an orphan leaves the kept window + starting on the message *after* the orphan, which may or may not be a + user message. Brittle. +- *Cline's explicit `ensureToolResultsFollowToolUse` pair walker*: more + general than our need — walks the entire kept window and synthesizes + `"result missing"` placeholders. We can get 90% of the benefit with a + single-point backward walk because Netclaw's conversations are always + well-formed at write time (no partial tool calls get persisted). +- *Preserve pairs by walking forward to the next user message*: shrinks + the kept window too aggressively (drops valid recent context). + Backward walk preserves more of what the user cares about. + +**Failure mode**: if the user-message-boundary search reaches +`systemOffset` without finding a user message (i.e. the entire post- +system history is tool/assistant chatter with no user turns), the +reducer keeps everything post-system. Practically impossible given the +turn-append protocol, but guarded anyway. + +### Decision 5: "Preserve prior summary block" rule in the observer prompt + +**Chosen**: The observer system prompt includes a rule: "If the input +contains a prior `[session-summary]` block, preserve its sections +verbatim and update in place — do not rewrite or re-summarize." + +The reducer's user-message-boundary walk-back is the structural defense: +when compaction runs on a session that already has a +`[session-summary session:{id}]` User-role message in its history, the +walk-back preserves that message in the kept window (it's a +non-system-nudge User message, which is a valid truncation point). The +observer then sees its own prior output in the discarded portion *only* +if the naive cut lands before the summary — in which case the +prompt-level rule kicks in: the model is instructed to preserve the +prior summary verbatim and update it in place. + +Belt-and-suspenders. Cline does something similar for the focus-chain +checklist (`contextManagement.ts:46-50`): *"If no task_progress list +was included in the previous context, you should NOT create a new +task_progress list"* — structural and prompt-level defenses layered. + +## Risks / Trade-offs + +- **Risk**: The 9-section prompt is longer than the current free-form + bullet prompt. Observer LLM call cost increases marginally. **Mitigation**: + the observer uses `_compactionClient` which is typically a cheaper/ + weaker model (sidecar tier). The added cost is per-compaction, not + per-turn. Measurable via existing usage telemetry on compaction boundary + events. + +- **Risk**: The structured output format is advisory, not enforced. If + the model ignores the template and produces free-form text, the new + "preserve prior summary" rule has no anchor. **Mitigation**: Cline has + shipped the 9-section template for months on production Claude traffic + without compliance issues. The section headers are distinctive enough + that minor format drift is tolerable. If model compliance degrades on a + new model family, we can tighten via forced tool-call response (also a + Cline pattern — `SummarizeTaskHandler` uses the `summarize_task` tool). + +- **Risk**: Header-based summary recognition relies on the observer LLM + actually emitting the `[session-summary session:{id}]` header (the + `WrapObservations` wrapper takes care of the canonical form even if + the model omits it). **Mitigation**: `WrapObservations` is deterministic + and normalizes any header-like first line to the canonical form. Tested. + +- **Risk**: Reducer walking backward to a user-message boundary can + *extend* the effective kept window past the requested `keepCount`. + In a long tool-loop (many messages without an intervening user + turn), the walk-back can carry the window all the way back to the + user message that started the loop, effectively ignoring `keepCount`. + **Mitigation**: today, we accept the larger window — extending is + safer than orphaning tool pairs. The adaptive loop in + `SessionCompactionPipeline.ExecuteAsync` halves `keepCount` on + each iteration when estimated tokens exceed half the context window, + but halving can't shrink the window below the walk-back floor. In + pathological cases this produces no reduction and the next turn may + re-trigger compaction immediately. + **Future fix**: Cline's `ensureToolResultsFollowToolUse` pattern + (`src/core/context/context-management/ContextManager.ts:375-477`) + synthesizes placeholder Tool messages for orphan Assistant + tool_calls and strips orphan tool_results, allowing truncation at + ANY boundary without pair integrity violations. Worth adopting if + production telemetry shows the walk-back failing to reduce context + in real sessions. Out of scope for this change. + +- **Defense in depth**: The reducer explicitly advances forward past + leading Tool-role messages in the degenerate case where the walk-back + falls through to `systemOffset` and that message is a Tool orphan + (e.g. recovered from a prior bug that left orphan tools at the head + of history). A kept window that starts with a Tool orphan would be + rejected by downstream providers and trigger an infinite + compact-retry loop — the skip-forward guards against that. See + `ExtractiveSessionReducer.cs` and the + `Degenerate_orphan_tool_at_head_is_advanced_past_not_kept` test. + +- **Trade-off**: Journal grows. Pre-boundary messages stay in history + for debugging/replay, so compacted sessions accumulate a growing + tail of journaled-but-not-sent-to-LLM content. The memory cost is + per-session; the storage cost is per-snapshot. Acceptable — it's + what enables future checkpoint rollback and eval replay without + re-running the session. + +## Migration Plan + +### Deployment + +1. Ship as part of the `0.12` release cycle. No config toggle — the new + behavior is always on. +2. No database migration. No new fields on `SessionCompacted`. Sessions + compacted before this change continue to work: their existing + `[observations from earlier in this session]` user message remains in + history and the next compaction will fold it into the new + `[session-summary session:{id}]` form via `WrapObservations`. + +### Rollback + +Revert the PR. No data migration needed — no persisted fields were +added or removed. Sessions compacted under the new format will have +User-role messages in history whose content starts with +`[session-summary session:{id}]`; the old code treats these as regular +user messages (harmless) and the next compaction on the reverted code +will fold them through the legacy observer prompt. + +## Open Questions + +- Should we eventually persist an explicit deleted-range index (like + Cline's `conversationHistoryDeletedRange`) to enable checkpoint + rollback and multi-tier summaries? Deferred — current rework uses + header-based recognition which is sufficient for the primary goal + (second-compaction defense). If rollback or multi-tier becomes a + concrete feature, the index can be added additively without disturbing + the header contract. +- Should the reducer's user-message-boundary walk have a maximum + backtrack distance? If so, what happens when the walk exceeds it? + Current decision: no limit. If the walk reaches `systemOffset`, keep + everything. Revisit if production telemetry shows aberrant cases. +- Should the observer's "preserve prior summary" rule be enforced via + a forced tool-call response (as Cline does), or rely on prompt + compliance? Deferred — prompt compliance first, escalate to forced + tool-call if production drift observed. diff --git a/openspec/changes/compaction-rework/proposal.md b/openspec/changes/compaction-rework/proposal.md new file mode 100644 index 000000000..3a1321352 --- /dev/null +++ b/openspec/changes/compaction-rework/proposal.md @@ -0,0 +1,149 @@ +# Change Proposal: compaction-rework + +## Why + +Netclaw's compaction pipeline exhibited post-compaction grounding failures in a +live Slack session on 2026-04-11 — ArdyBot lost specificity around a `Rect` +struct it had been inspecting, and failed to recall its own session ID because +an earlier turn had referenced a foreign session ID. Root-cause analysis showed +the observer phase strips the grounding signal it most needs (tool-call +arguments, self-session-id disambiguation), produces a free-form bullet list +that decays exponentially across successive compactions, and relies on an +extractive reducer that can create orphan tool-result messages the provider +will reject. + +Rather than another incremental prompt patch, this change is a structural +rework informed by reading the actual source of four real LLM harnesses: +**OpenCode** (SST, `2719063`), **Aider** (`f09d706`), **Cline** (`a0faf7c`), +and **Claude Code** (Anthropic — docs at `code.claude.com/docs/en/*`). The +research surfaced three ideas worth adopting: Cline's 9-section structured +summary with explicit anti-drift rules, Cline's monotonic compaction boundary +that prevents summary-over-summary decay, and OpenCode's +truncate-only-at-user-message-boundaries rule that cleanly enforces tool +pair integrity. The current netclaw-session spec already has a "Tool call/result +pair integrity" scenario that the code does not honor — this change fixes that +gap as well. + +Research write-up with file/line citations is in the plan file at +`/home/petabridge/.claude/plans/proud-honking-goose.md`. + +## What Changes + +- **BREAKING**: `CompactionParameters` record gains a `SessionId` field. All + callers of `SessionCompactionPipeline.ExecuteAsync` must pass it. +- **BREAKING**: `ObservationPromptBuilder.BuildObservationSystemPrompt()`, + `BuildObservationUserPrompt()`, and `WrapObservations()` gain `SessionId` + parameters and a new output contract (structured sections, not free-form + bullets). +- **BREAKING**: The compacted session history format changes. Prior + observation messages stored as `User`-role messages with `[observations + from earlier in this session]` prefix now use a distinctive + `[session-summary session:{id}]` header. The header is the recognition + marker used by the observer on successive compactions and by the reducer + when walking backward to a user-message boundary. Journals written + before this change still replay via the existing `Apply(SessionCompacted)` + path — the old format stays readable — but new compactions produce the + new format. +- **New**: Observer LLM system prompt rewritten to the 9-section structured + format, borrowed from Cline's `contextManagement.ts:10-110`: + Primary Request / Technical Concepts / Files+Code / Problem Solving / + Pending Tasks / Task Evolution (with direct user quotes to prevent drift) / + Current Work / Next Step / Required Files. Adapted to Netclaw vocabulary. +- **New**: Observer receives the self `SessionId` in its system prompt so it + can disambiguate the running session from any foreign session IDs + referenced in the discarded window. +- **New**: Observer is instructed: "If the input already contains a + `[session-summary]` block from a prior compaction, preserve its sections + verbatim and append/update — do not rewrite." This is the structural + second-compaction defense. +- **Modified**: `ExtractiveSessionReducer` truncates only at user-message + boundaries. When the naive cutoff would land on a `Tool`-role message or + an `Assistant` message with `FunctionCallContent`, walk backward to the + nearest user-message boundary. Subsumes the latent pair-integrity bug in + the current reducer which slices by count and produces orphan tool + results. +- **Modified**: `netclaw-session` "Conversation compaction" requirement — + updated to reflect the new structured summary format, the distinctive + `[session-summary session:{id}]` header, and user-boundary truncation. + The "Tool call/result pair integrity" scenario becomes enforced (was + aspirational). + +## Capabilities + +### New Capabilities + +_(none — this change modifies an existing capability)_ + +### Modified Capabilities + +- `netclaw-session`: the "Conversation compaction" requirement is rewritten + to reflect structured 9-section summary output, the + `[session-summary session:{id}]` header that makes summaries + recognizable across successive compactions, and tool call/result pair + integrity via user-boundary truncation. The existing "Tool call/result + pair integrity during compaction" scenario is strengthened from + aspirational to strictly enforced. + +## Impact + +### Affected code + +- `src/Netclaw.Actors/Sessions/ObservationPromptBuilder.cs` — prompt rewrite, + new signatures, structured section output contract +- `src/Netclaw.Actors/Sessions/Pipelines/SessionCompactionPipeline.cs` — + `CompactionParameters.SessionId`, emit summary as System-role with + boundary marker +- `src/Netclaw.Actors/Sessions/SessionState.cs` — update + `Apply(SessionCompacted)` to preserve `WorkingContext` and rebuild + history with the structured summary at the head +- `src/Netclaw.Actors/Sessions/ExtractiveSessionReducer.cs` — walk backward + to user-message boundary instead of slicing by count +- `src/Netclaw.Actors/Sessions/LlmSessionActor.cs` — `CompactionParameters` + construction includes `_sessionId` + +### Affected tests + +- `src/Netclaw.Actors.Tests/Sessions/ObservationPromptBuilderTests.cs` — + assertions for structured sections, self-session-id embedding, + preserve-prior-summary rule +- `src/Netclaw.Actors.Tests/Sessions/ExtractiveSessionReducerTests.cs` — + user-boundary truncation, orphan prevention for tool results and + assistant tool_calls +- `src/Netclaw.Actors.Tests/Sessions/CompactionIntegrationTests.cs` — + monotonic boundary, second-compaction no-decay test, session-id + disambiguation test + +### Affected APIs / journals + +- **Journal compatibility**: existing `SessionCompacted` events continue to + deserialize and replay. New compactions write the extended event with the + boundary index. No migration required. +- **IPC**: no public API change visible outside the actor package. +- **Memory queue**: unchanged — compaction still emits its high-priority + memory checkpoint via `EnqueueCheckpointFireAndForget` per the existing + "Compaction boundary emits memory checkpoint" scenario. + +### Security & operational impact + +- **Security**: none. No new trust surfaces, no new grant categories, no + new tool capabilities. The observer LLM call continues to use the + existing `_compactionClient` with the same audience context. +- **Operational**: slight increase in observer prompt size from the + structured template. Offset by the existing `KeepRecentToolResults` + + Phase 1 tool-result clearing which bounds the discarded window size. No + change to the compaction trigger threshold. +- **Observability**: `SessionCompacted` event format extends — existing + eval replay tooling continues to work on old journals; new journals + carry additional metadata useful for regression analysis. + +### Dependencies / out of scope + +- Durable `WorkingContext` task state (`RecentFiles`, `OpenGoals`, + `ProgressMarkers`) is a separate OpenSpec change + (`working-context-grounding`) that stacks on this one. +- Session CWD tracking and project identity file re-reading are tracked as + GitHub issues **#595** and **#596** against milestone 0.12, not this + change. +- Aider's "files-as-source-of-truth" philosophy (stop persisting file + contents in history, re-read on demand) is called out as the deepest + architectural direction but is not in scope. diff --git a/openspec/changes/compaction-rework/specs/netclaw-session/spec.md b/openspec/changes/compaction-rework/specs/netclaw-session/spec.md new file mode 100644 index 000000000..e276066a2 --- /dev/null +++ b/openspec/changes/compaction-rework/specs/netclaw-session/spec.md @@ -0,0 +1,111 @@ +# netclaw-session Delta Spec — compaction-rework + +## MODIFIED Requirements + +### Requirement: Conversation compaction + +The system SHALL compact long session history using a tiered approach that +produces a structured summary surviving successive compactions without +grounding decay, enforces tool call/result pair integrity at the compaction +boundary, and disambiguates the self session from any foreign session +identifiers referenced in the discarded window. Before and after compaction +boundaries, the session SHALL emit high-priority memory checkpoints into the +durable memory queue instead of performing a synchronous one-off memory flush +that depends on the turn path completing all curation work inline. + +The compaction observer LLM SHALL produce output in a fixed structured +format with nine sections: Primary Request and Intent, Key Technical +Concepts, Files and Code Sections, Problem Solving, Pending Tasks, Task +Evolution, Current Work, Next Step, and Required Files. The Task Evolution +section SHALL contain direct quotes from user messages that changed the +task, to prevent drift across successive compactions. + +The compaction summary message SHALL be wrapped with a distinctive header +of the form `[session-summary session:{id}]` so that consumers (the +observer on successive compactions, the reducer, and the UI) can +recognize it as a prior-compaction artifact and preserve it across +successive compactions without relying on a separately-persisted index. + +The compaction observer SHALL receive the self `SessionId` in its system +prompt and SHALL explicitly mark any foreign session identifiers in +observations as `session:{id}` rather than conflating them with the self +session. + +The compaction observer system prompt SHALL include a rule instructing the +model to preserve any prior structured summary block verbatim and update +in place, rather than re-summarizing or rewriting it. + +#### Scenario: Compaction threshold reached + +- **GIVEN** `UsageDetails.InputTokenCount` exceeds `SessionConfig.CompactionTokenLimit` +- **WHEN** compaction runs +- **THEN** the actor enters `Compacting` behavior state +- **AND** incoming messages are buffered during compaction + +#### Scenario: Compaction boundary emits memory checkpoint + +- **GIVEN** compaction is about to run or has just completed a summary reduction +- **WHEN** the compaction boundary is reached +- **THEN** the session enqueues a high-priority memory checkpoint for durable + curation +- **AND** the user-facing session does not wait for background curation to + finish + +#### Scenario: Tiered compaction — tool result clearing first + +- **GIVEN** compaction is triggered +- **WHEN** phase 1 runs +- **THEN** old tool results are replaced with placeholders +- **AND** the N most recent tool interactions are preserved in full +- **AND** if threshold is now satisfied, no summarization LLM call is made + +#### Scenario: Tiered compaction — structured summarization + +- **GIVEN** phase 1 (tool clearing) did not bring context under threshold +- **WHEN** the observer LLM call runs +- **THEN** the observer produces a summary containing the nine fixed sections + (Primary Request and Intent, Key Technical Concepts, Files and Code Sections, + Problem Solving, Pending Tasks, Task Evolution, Current Work, Next Step, + Required Files) +- **AND** the Task Evolution section contains direct quotes from user + messages that changed the task +- **AND** the summary is wrapped with a `[session-summary session:{id}]` + header and stored in the compacted history +- **AND** a `SessionCompacted` event is persisted carrying the compacted + messages +- **AND** a persistence snapshot is taken +- **AND** compacted state remains usable for future turns + +#### Scenario: Successive compactions do not re-summarize prior summary + +- **GIVEN** a session that has been compacted, with a prior + `[session-summary session:{id}]` message in history +- **WHEN** a subsequent compaction is triggered +- **THEN** the observer system prompt instructs the model to preserve the + prior summary block verbatim and update its sections in place +- **AND** the reducer's user-message-boundary walk-back preserves the + prior summary message in the kept window (the summary is a User-role + message with a distinctive header) + +#### Scenario: Self session disambiguation in observer + +- **GIVEN** the discarded window contains a reference to a session identifier + that is not the running session (e.g. the agent was investigating another + session via a tool call) +- **WHEN** the observer LLM call runs +- **THEN** the observer system prompt includes the self session id +- **AND** the produced summary marks the foreign session as `session:{id}` +- **AND** the produced summary does not conflate the foreign session with the + self session + +#### Scenario: Tool call/result pair integrity during compaction + +- **GIVEN** conversation history contains tool call/result pairs +- **WHEN** the extractive reducer selects the kept window +- **THEN** the kept window starts on a `User`-role message (not a + `Tool`-role message and not an `Assistant` message that contains + `FunctionCallContent` without a matching preceding user turn) +- **AND** tool call/result pairs are never split across the compaction + boundary +- **AND** older tool interactions remain representable in the journal for + checkpoint extraction and summarization diff --git a/openspec/changes/compaction-rework/tasks.md b/openspec/changes/compaction-rework/tasks.md new file mode 100644 index 000000000..517df7f43 --- /dev/null +++ b/openspec/changes/compaction-rework/tasks.md @@ -0,0 +1,86 @@ +# Tasks: compaction-rework + +## 1. ExtractiveSessionReducer — user-message boundary truncation + +- [x] 1.1 Replace the slice-by-count logic in + `ExtractiveSessionReducer.ReduceAsync` with a backward walk: starting + from `list.Count - keepCount`, walk backward until we hit a `User`-role + message that is not prefixed with `SessionState.SystemNudgePrefix` +- [x] 1.2 Keep-zero edge case: skip the walk entirely and return just + the system prompt (or empty when no system prompt is present) +- [x] 1.3 Unit tests in `ExtractiveSessionReducerTests.cs`: + - [x] 1.3.1 `Window_walks_backward_to_user_boundary_when_naive_cut_would_orphan_tool_result` + - [x] 1.3.2 `Window_walks_backward_past_assistant_tool_call_to_user_boundary` + - [x] 1.3.3 `Window_skips_system_nudges_when_finding_user_boundary` + - [x] 1.3.4 `Window_start_already_on_user_boundary_is_preserved` + - [x] 1.3.5 `Window_falls_back_to_keep_all_post_system_when_no_user_message_found` + - [x] 1.3.6 `Keep_zero_preserves_only_system_prompt` (and no-system variant) + +## 2. ObservationPromptBuilder — structured 9-section prompt + +- [x] 2.1 Rewrite `BuildObservationSystemPrompt` to accept `SessionId` and + emit the nine-section template (Primary Request and Intent, Key Technical + Concepts, Files and Code Sections, Problem Solving, Pending Tasks, Task + Evolution, Current Work, Next Step, Required Files) +- [x] 2.2 Add explicit Task Evolution rule: include direct quotes from + user messages that changed the task (anti-drift rule) +- [x] 2.3 Add explicit self-session-id disambiguation rule: "You are + summarizing session {id}. Mark foreign session IDs as `session:{id}`, + never conflate them with the self session." +- [x] 2.4 Add explicit "preserve prior summary" rule: "If the input + already contains a `[session-summary ...]` block, preserve its sections + verbatim and update in place — do not rewrite." +- [x] 2.5 `BuildObservationUserPrompt` — preserve tool-call arguments as + compact `{name}({short-args})` evidence for the observer. Raise tool + result truncation to 1500 chars. +- [x] 2.6 `WrapObservations` takes `SessionId` and produces a canonical + `[session-summary session:{id}]` header block. Normalizes any + pre-existing header-like first line to the canonical form. +- [x] 2.7 Update `ObservationPromptBuilderTests.cs`: + - [x] 2.7.1 `System_prompt_embeds_self_session_id_for_disambiguation` + - [x] 2.7.2 `System_prompt_lists_all_nine_structured_sections` + - [x] 2.7.3 `System_prompt_requires_direct_quotes_in_task_evolution` + - [x] 2.7.4 `System_prompt_instructs_preserve_prior_summary_verbatim` + - [x] 2.7.5 `User_prompt_preserves_tool_call_arguments_as_short_projection` + - [x] 2.7.6 `WrapObservations_uses_session_summary_marker_with_session_id` + +## 3. SessionCompactionPipeline — thread SessionId + +- [x] 3.1 Add `SessionId SessionId` field to `CompactionParameters` record +- [x] 3.2 Thread `SessionId` through `ExecuteAsync` to + `GenerateObservationsAsync` and `WrapObservations` / `BuildObservationSystemPrompt` +- [x] 3.3 Store the summary as a User-role `SerializableChatMessage` at + index 0 of the compacted messages list (the content begins with the + `[session-summary session:{id}]` header per task 2.6, which is how + consumers recognize it — no separate boundary index is persisted) +- [x] 3.4 Update `LlmSessionActor` to construct `CompactionParameters` + with `_sessionId` + +## 4. CompactionIntegrationTests + +- [x] 4.1 `Compaction_observer_system_prompt_receives_self_session_id` — + inspect `_fakeChatClient.ReceivedMessages` for the observer sidecar + call and assert the self session id appears in the system text +- [x] 4.2 `Compaction_observation_wrapper_embeds_session_id_in_header` — + after compaction, the next main-model call includes a User message + whose content starts with `[session-summary session:{id}]` +- [x] 4.3 Existing scenarios continue to pass: buffer drain, session + recovery after compaction+kill, emergency compaction with buffered + message, summary format with context-summary tags + +## 5. Quality gates + +- [x] 5.1 `dotnet build src/Netclaw.Actors.Tests/Netclaw.Actors.Tests.csproj` + passes with zero warnings +- [x] 5.2 `dotnet test src/Netclaw.Actors.Tests/Netclaw.Actors.Tests.csproj` + — all 909 tests pass +- [x] 5.3 `dotnet slopwatch analyze` reports no new violations against + baseline +- [x] 5.4 `openspec validate compaction-rework` passes + +## 6. PR + commit + +- [x] 6.1 Commit with a message referencing the Slack failure and the + research sources (Aider, OpenCode, Cline, Claude Code) +- [x] 6.2 Push branch `compaction-rework`, open PR against `dev` +- [x] 6.3 PR body references GH issues #595 and #596 as follow-ups diff --git a/src/Netclaw.Actors.Tests/Sessions/CompactionIntegrationTests.cs b/src/Netclaw.Actors.Tests/Sessions/CompactionIntegrationTests.cs index 13c158bfd..5efbfeee0 100644 --- a/src/Netclaw.Actors.Tests/Sessions/CompactionIntegrationTests.cs +++ b/src/Netclaw.Actors.Tests/Sessions/CompactionIntegrationTests.cs @@ -450,6 +450,211 @@ await sessionManager.Ask(new SendUserMessage Assert.Contains("fake", text.Text, StringComparison.OrdinalIgnoreCase); } + [Fact] + public async Task Compaction_observation_wrapper_embeds_session_id_in_header() + { + // Observer wrapper must carry the session id in its header so that + // subsequent compactions (or a weaker model reading the observation + // text) can disambiguate the self session from any foreign session + // ids referenced in the observations. + _fakeChatClient.UsageOverride = new UsageDetails + { + InputTokenCount = 800, + OutputTokenCount = 50, + TotalTokenCount = 850 + }; + + var sessionId = new SessionId("test-channel/observation-session-id"); + var sessionManager = ActorRegistry.Get(); + var subscriber = CreateTestProbe("obs-session-id-sub"); + + await sessionManager.Ask(new JoinSession + { + SessionId = sessionId, + Subscriber = subscriber, + Filter = OutputFilter.Full + }, TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + + await sessionManager.Ask(new SendUserMessage + { + SessionId = sessionId, + Content = "Trigger compaction" + }, TestContext.Current.CancellationToken); + + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + + _fakeChatClient.UsageOverride = new UsageDetails + { + InputTokenCount = 100, + OutputTokenCount = 20, + TotalTokenCount = 120 + }; + + // Next turn — the observation message should now be in history. + await sessionManager.Ask(new SendUserMessage + { + SessionId = sessionId, + Content = "Post-compaction probe" + }, TestContext.Current.CancellationToken); + + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + + // Inspect the main-model call that followed compaction. Filter out + // the observer sidecar calls by matching on the new session-summarizer + // system prompt marker. + var mainModelCalls = _fakeChatClient.ReceivedMessages + .Where(msgs => !(msgs.FirstOrDefault(m => m.Role == Microsoft.Extensions.AI.ChatRole.System)?.Text + ?? string.Empty).Contains("session summarizer", StringComparison.OrdinalIgnoreCase)) + .ToList(); + + Assert.NotEmpty(mainModelCalls); + var lastMainCall = mainModelCalls[^1]; + var observationHeader = $"[session-summary session:{sessionId.Value}]"; + var hasObservationMessage = lastMainCall.Any(m => + m.Role == Microsoft.Extensions.AI.ChatRole.User + && (m.Text ?? string.Empty).StartsWith(observationHeader, StringComparison.Ordinal)); + + Assert.True(hasObservationMessage, + $"Expected post-compaction main-model call to include a User message starting with '{observationHeader}'"); + } + + [Fact] + public async Task Compaction_observer_system_prompt_receives_self_session_id() + { + // The observer LLM call must see the self session id in its system + // prompt so it can disambiguate foreign session ids in the discarded + // window. + _fakeChatClient.UsageOverride = new UsageDetails + { + InputTokenCount = 800, + OutputTokenCount = 50, + TotalTokenCount = 850 + }; + + var sessionId = new SessionId("test-channel/observer-grounding"); + var sessionManager = ActorRegistry.Get(); + var subscriber = CreateTestProbe("observer-grounding-sub"); + + await sessionManager.Ask(new JoinSession + { + SessionId = sessionId, + Subscriber = subscriber, + Filter = OutputFilter.Full + }, TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + + await sessionManager.Ask(new SendUserMessage + { + SessionId = sessionId, + Content = "Trigger compaction" + }, TestContext.Current.CancellationToken); + + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + + // Find the observer sidecar call + var observerCall = _fakeChatClient.ReceivedMessages.FirstOrDefault(msgs => + (msgs.FirstOrDefault(m => m.Role == Microsoft.Extensions.AI.ChatRole.System)?.Text ?? string.Empty) + .Contains("session summarizer", StringComparison.OrdinalIgnoreCase)); + + Assert.NotNull(observerCall); + var systemText = observerCall!.First(m => m.Role == Microsoft.Extensions.AI.ChatRole.System).Text ?? string.Empty; + Assert.Contains(sessionId.Value, systemText); + Assert.Contains("SELF session", systemText, StringComparison.Ordinal); + } + + [Fact] + public async Task Successive_compactions_preserve_prior_summary_in_observer_system_prompt() + { + // Anti-drift defense: on the second compaction, the observer's + // system prompt must include the prior summary as an explicit + // "preserve verbatim" block. This is the structural defense the + // spec requires (Conversation compaction → "preserve any prior + // structured summary block verbatim and update in place"). + _fakeChatClient.UsageOverride = new UsageDetails + { + InputTokenCount = 800, + OutputTokenCount = 50, + TotalTokenCount = 850 + }; + + // The observer returns a structured-looking summary so the wrapper + // produces a recognizable [session-summary session:{id}] block in + // history. The next compaction should lift this out of the + // discarded window and include it in the observer's system prompt. + _fakeChatClient.ObservationResponseOverride = + "## 1. Primary Request and Intent\nUser wants to debug the Rect struct\n## 6. Task Evolution\n- Original: \"help me with Rect\""; + + var sessionId = new SessionId("test-channel/successive-compactions"); + var sessionManager = ActorRegistry.Get(); + var subscriber = CreateTestProbe("successive-sub"); + + await sessionManager.Ask(new JoinSession + { + SessionId = sessionId, + Subscriber = subscriber, + Filter = OutputFilter.Full + }, TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + + // First compaction + await sessionManager.Ask(new SendUserMessage + { + SessionId = sessionId, + Content = "First user turn that triggers compaction" + }, TestContext.Current.CancellationToken); + + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + + // Snapshot how many calls happened before the second compaction. + var callsBeforeSecondCompaction = _fakeChatClient.ReceivedMessages.Count; + + // Second compaction: keep usage high so another compaction fires + // after the next turn. + await sessionManager.Ask(new SendUserMessage + { + SessionId = sessionId, + Content = "Second user turn that also triggers compaction" + }, TestContext.Current.CancellationToken); + + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + + // Find the SECOND observer sidecar call (the one that ran during + // the second compaction). It's the last observer-role call made. + var observerCalls = _fakeChatClient.ReceivedMessages + .Select((msgs, idx) => (msgs, idx)) + .Where(x => (x.msgs.FirstOrDefault(m => m.Role == Microsoft.Extensions.AI.ChatRole.System)?.Text ?? string.Empty) + .Contains("session summarizer", StringComparison.OrdinalIgnoreCase)) + .ToList(); + + Assert.True(observerCalls.Count >= 2, + $"Expected at least 2 observer sidecar calls across two compactions; got {observerCalls.Count}"); + + // The second observer call's system prompt must contain the prior + // summary lifted from the discarded window. + var secondObserverCall = observerCalls.Last(x => x.idx >= callsBeforeSecondCompaction); + var systemText = secondObserverCall.msgs + .First(m => m.Role == Microsoft.Extensions.AI.ChatRole.System).Text ?? string.Empty; + + Assert.Contains("PRIOR SUMMARY", systemText, StringComparison.Ordinal); + Assert.Contains("preserve the bullets", systemText, StringComparison.OrdinalIgnoreCase); + Assert.Contains($"[session-summary session:{sessionId.Value}]", systemText, StringComparison.Ordinal); + } + [Fact] public async Task Emergency_compaction_auto_resends_pending_message() { diff --git a/src/Netclaw.Actors.Tests/Sessions/ExtractiveSessionReducerTests.cs b/src/Netclaw.Actors.Tests/Sessions/ExtractiveSessionReducerTests.cs index 460edaa1b..8dee58884 100644 --- a/src/Netclaw.Actors.Tests/Sessions/ExtractiveSessionReducerTests.cs +++ b/src/Netclaw.Actors.Tests/Sessions/ExtractiveSessionReducerTests.cs @@ -175,4 +175,155 @@ public async Task No_system_prompt_keeps_last_N() Assert.Equal("Second", result[0].Text); Assert.Equal("Reply 2", result[1].Text); } + + [Fact] + public async Task Window_walks_backward_to_user_boundary_when_naive_cut_would_orphan_tool_result() + { + // keepCount=3 would place the naive cut on the Tool result, whose + // matching FunctionCallContent is in the discarded portion. The + // reducer must walk backward to the preceding User-role message. + var reducer = new ExtractiveSessionReducer(keepRecentMessages: 3); + var messages = new List + { + new(ChatRole.System, "System prompt"), + new(ChatRole.User, "Old user turn"), // index 1, discarded + new(ChatRole.Assistant, "Old assistant reply"), // index 2, discarded + new(ChatRole.User, "Search for X"), // index 3, user boundary + new(ChatRole.Assistant, [new FunctionCallContent("call-1", "search", new Dictionary { ["q"] = "X" })]), // 4 + new(ChatRole.Tool, [new FunctionResultContent("call-1", "Found X")]), // 5, naive cut lands here + new(ChatRole.Assistant, "Let me check further.") // 6 + }; + + var result = (await reducer.ReduceAsync(messages, CancellationToken.None)).ToList(); + + // Expect: system + user "Search for X" + assistant tool_call + tool result + assistant + Assert.Equal(ChatRole.System, result[0].Role); + Assert.Equal("Search for X", result[1].Text); + Assert.Contains(result[2].Contents, c => c is FunctionCallContent); + Assert.Equal(ChatRole.Tool, result[3].Role); + Assert.Equal("Let me check further.", result[4].Text); + } + + [Fact] + public async Task Window_walks_backward_past_assistant_tool_call_to_user_boundary() + { + // keepCount=2 naive cut lands on the Assistant tool_call. The reducer + // must walk back to the preceding User-role message so the tool_call + // is contextualized by the user turn that requested it. + var reducer = new ExtractiveSessionReducer(keepRecentMessages: 2); + var messages = new List + { + new(ChatRole.System, "System prompt"), + new(ChatRole.User, "Old user turn"), + new(ChatRole.Assistant, "Old assistant reply"), + new(ChatRole.User, "Please grep for foo"), + new(ChatRole.Assistant, [new FunctionCallContent("call-a", "grep", new Dictionary { ["pattern"] = "foo" })]), + new(ChatRole.Tool, [new FunctionResultContent("call-a", "3 matches")]) + }; + + var result = (await reducer.ReduceAsync(messages, CancellationToken.None)).ToList(); + + Assert.Equal(ChatRole.System, result[0].Role); + Assert.Equal("Please grep for foo", result[1].Text); + Assert.Contains(result[2].Contents, c => c is FunctionCallContent); + Assert.Equal(ChatRole.Tool, result[3].Role); + } + + [Fact] + public async Task Window_skips_system_nudges_when_finding_user_boundary() + { + // A system nudge uses User role but has the SystemNudgePrefix — it's + // actor-injected (recall content, empty-response nudges), not a real + // user turn. The backward walk must skip it. + var reducer = new ExtractiveSessionReducer(keepRecentMessages: 2); + var nudgeContent = $"{SessionState.SystemNudgePrefix} recalled-memory blah]"; + var messages = new List + { + new(ChatRole.System, "System prompt"), + new(ChatRole.User, "Real user turn"), + new(ChatRole.Assistant, "Reply"), + new(ChatRole.User, nudgeContent), // nudge — not a real user turn + new(ChatRole.Assistant, [new FunctionCallContent("call-x", "tool", null)]), + new(ChatRole.Tool, [new FunctionResultContent("call-x", "done")]) + }; + + var result = (await reducer.ReduceAsync(messages, CancellationToken.None)).ToList(); + + // Walk should skip the nudge and land on "Real user turn". + Assert.Equal(ChatRole.System, result[0].Role); + Assert.Equal("Real user turn", result[1].Text); + } + + [Fact] + public async Task Window_start_already_on_user_boundary_is_preserved() + { + // Naive cut lands on a User message — no walk needed. + var reducer = new ExtractiveSessionReducer(keepRecentMessages: 3); + var messages = new List + { + new(ChatRole.System, "System prompt"), + new(ChatRole.User, "Discarded"), + new(ChatRole.Assistant, "Discarded reply"), + new(ChatRole.User, "Kept user turn"), + new(ChatRole.Assistant, [new FunctionCallContent("call-1", "search", null)]), + new(ChatRole.Tool, [new FunctionResultContent("call-1", "result")]) + }; + + var result = (await reducer.ReduceAsync(messages, CancellationToken.None)).ToList(); + + Assert.Equal(4, result.Count); + Assert.Equal(ChatRole.System, result[0].Role); + Assert.Equal("Kept user turn", result[1].Text); + } + + [Fact] + public async Task Window_falls_back_to_keep_all_post_system_when_no_user_message_found() + { + // Degenerate case: no user message post-system, first post-system + // message is Assistant (not Tool). Keep everything post-system — + // the defense-in-depth advance-forward only triggers on leading Tool + // orphans, which this history doesn't have. + var reducer = new ExtractiveSessionReducer(keepRecentMessages: 2); + var messages = new List + { + new(ChatRole.System, "System prompt"), + new(ChatRole.Assistant, "Assistant only"), + new(ChatRole.Assistant, [new FunctionCallContent("c", "t", null)]), + new(ChatRole.Tool, [new FunctionResultContent("c", "r")]), + new(ChatRole.Assistant, "More assistant") + }; + + var result = (await reducer.ReduceAsync(messages, CancellationToken.None)).ToList(); + + Assert.Equal(5, result.Count); + Assert.Equal(ChatRole.System, result[0].Role); + } + + [Fact] + public async Task Degenerate_orphan_tool_at_head_is_advanced_past_not_kept() + { + // Defense in depth: history starts (post-system) with Tool-role + // orphans whose matching Assistant tool_calls do not exist. This + // shouldn't happen in a well-formed session, but if it does via + // recovery from broken state, the reducer must not emit a kept + // window that starts with an orphan Tool — downstream providers + // would reject the request. + var reducer = new ExtractiveSessionReducer(keepRecentMessages: 2); + var messages = new List + { + new(ChatRole.System, "System prompt"), + new(ChatRole.Tool, [new FunctionResultContent("orphan-1", "r1")]), + new(ChatRole.Tool, [new FunctionResultContent("orphan-2", "r2")]), + new(ChatRole.Assistant, "Recovery assistant"), + new(ChatRole.Assistant, "More assistant") + }; + + var result = (await reducer.ReduceAsync(messages, CancellationToken.None)).ToList(); + + // The kept window must not start with a Tool orphan. Leading Tool + // messages are skipped, even if that shrinks the window below the + // requested keepCount. + Assert.Equal(ChatRole.System, result[0].Role); + Assert.DoesNotContain(result.Skip(1), m => m.Role == ChatRole.Tool); + } } diff --git a/src/Netclaw.Actors.Tests/Sessions/LlmSessionIntegrationTests.cs b/src/Netclaw.Actors.Tests/Sessions/LlmSessionIntegrationTests.cs index fd1dd8f64..22d3b5bbb 100644 --- a/src/Netclaw.Actors.Tests/Sessions/LlmSessionIntegrationTests.cs +++ b/src/Netclaw.Actors.Tests/Sessions/LlmSessionIntegrationTests.cs @@ -1630,6 +1630,14 @@ internal sealed class FakeChatClient : IChatClient /// public int HangingObservationCallsRemaining { get; set; } + /// + /// When set, the observer sidecar call returns this text instead of the + /// default "- compacted observation". Used by tests that need the + /// observer output to look like a real structured summary so successive + /// compactions can be exercised. + /// + public string? ObservationResponseOverride { get; set; } + /// /// Number of compaction observation sidecar calls that should ignore cancellation /// entirely and never complete. Used to simulate a wedged compaction provider. @@ -1716,7 +1724,7 @@ public async Task GetResponseAsync( JsonSerializer.Serialize(plan))); } - if (systemText.Contains("You are an observation compressor", StringComparison.Ordinal)) + if (systemText.Contains("You are a session summarizer", StringComparison.Ordinal)) { if (StuckObservationCallsRemaining > 0) { @@ -1735,7 +1743,7 @@ public async Task GetResponseAsync( return new ChatResponse(new ChatMessage( Microsoft.Extensions.AI.ChatRole.Assistant, - "- compacted observation")); + ObservationResponseOverride ?? "- compacted observation")); } var plannedToolCallDecision = PlannedToolCallDecisions.Count > 0 diff --git a/src/Netclaw.Actors.Tests/Sessions/ObservationPromptBuilderTests.cs b/src/Netclaw.Actors.Tests/Sessions/ObservationPromptBuilderTests.cs index 19d54823c..19acaedad 100644 --- a/src/Netclaw.Actors.Tests/Sessions/ObservationPromptBuilderTests.cs +++ b/src/Netclaw.Actors.Tests/Sessions/ObservationPromptBuilderTests.cs @@ -6,15 +6,68 @@ namespace Netclaw.Actors.Tests.Sessions; public class ObservationPromptBuilderTests { + private static readonly SessionId TestSession = new("test-channel/test-thread"); + [Fact] - public void System_prompt_instructs_compression() + public void System_prompt_describes_summarization_task() { - var prompt = ObservationPromptBuilder.BuildObservationSystemPrompt(); + var prompt = ObservationPromptBuilder.BuildObservationSystemPrompt(TestSession); - Assert.Contains("observation", prompt, StringComparison.OrdinalIgnoreCase); + Assert.Contains("session summarizer", prompt, StringComparison.OrdinalIgnoreCase); Assert.Contains("compress", prompt, StringComparison.OrdinalIgnoreCase); } + [Fact] + public void System_prompt_embeds_self_session_id_for_disambiguation() + { + var prompt = ObservationPromptBuilder.BuildObservationSystemPrompt(TestSession); + + Assert.Contains(TestSession.Value, prompt); + Assert.Contains("SELF session", prompt, StringComparison.Ordinal); + } + + [Fact] + public void System_prompt_lists_all_nine_structured_sections() + { + var prompt = ObservationPromptBuilder.BuildObservationSystemPrompt(TestSession); + + // Each of the nine section headers must appear in the prompt. + Assert.Contains("## 1. Primary Request and Intent", prompt, StringComparison.Ordinal); + Assert.Contains("## 2. Key Technical Concepts", prompt, StringComparison.Ordinal); + Assert.Contains("## 3. Files and Code Sections", prompt, StringComparison.Ordinal); + Assert.Contains("## 4. Problem Solving", prompt, StringComparison.Ordinal); + Assert.Contains("## 5. Pending Tasks", prompt, StringComparison.Ordinal); + Assert.Contains("## 6. Task Evolution", prompt, StringComparison.Ordinal); + Assert.Contains("## 7. Current Work", prompt, StringComparison.Ordinal); + Assert.Contains("## 8. Next Step", prompt, StringComparison.Ordinal); + Assert.Contains("## 9. Required Files", prompt, StringComparison.Ordinal); + } + + [Fact] + public void System_prompt_requires_direct_quotes_in_task_evolution() + { + var prompt = ObservationPromptBuilder.BuildObservationSystemPrompt(TestSession); + + // The Task Evolution section is the anti-drift defense — it MUST + // require direct user quotes rather than paraphrase. + Assert.Contains("Direct quotes", prompt, StringComparison.OrdinalIgnoreCase); + Assert.Contains("paraphrase", prompt, StringComparison.OrdinalIgnoreCase); + // Section 6 is Task Evolution + Assert.Contains("## 6. Task Evolution", prompt, StringComparison.Ordinal); + } + + [Fact] + public void System_prompt_instructs_preserve_prior_summary_verbatim() + { + var prompt = ObservationPromptBuilder.BuildObservationSystemPrompt(TestSession); + + // The second-compaction defense: when a prior summary exists, the + // observer must preserve its sections verbatim. + Assert.Contains("session-summary", prompt, StringComparison.Ordinal); + Assert.Contains("preserve its sections verbatim", prompt, StringComparison.Ordinal); + Assert.Contains("CRITICAL RULE", prompt, StringComparison.Ordinal); + } + [Fact] public void User_prompt_includes_message_content() { @@ -46,9 +99,9 @@ public void User_prompt_skips_system_messages() } [Fact] - public void User_prompt_truncates_long_tool_results() + public void User_prompt_truncates_long_tool_results_to_grounding_budget() { - var longContent = new string('x', 1000); + var longContent = new string('x', 4000); var messages = new List { new() { Role = ChatRole.Tool, Content = longContent, Name = "shell_execute", ToolCallId = "1" }, @@ -56,9 +109,10 @@ public void User_prompt_truncates_long_tool_results() var prompt = ObservationPromptBuilder.BuildObservationUserPrompt(messages); - // Should be truncated to 500 chars Assert.True(prompt.Length < longContent.Length); Assert.Contains("...", prompt); + // Budget is 1500 chars — previous implementation truncated at 500 + Assert.True(prompt.Length > 500, "Tool result budget should be larger than the old 500-char limit"); } [Fact] @@ -80,22 +134,177 @@ public void User_prompt_includes_tool_call_names() } [Fact] - public void WrapObservations_adds_delimiter_when_missing() + public void User_prompt_preserves_tool_call_arguments_as_short_projection() + { + var messages = new List + { + new() + { + Role = ChatRole.Assistant, + Content = string.Empty, + ToolCalls = + [ + new SerializableToolCall + { + CallId = "1", + Name = "grep_files", + ArgumentsJson = """{"pattern":"Rect","path":"src/Termina.Layout"}""" + } + ] + }, + }; + + var prompt = ObservationPromptBuilder.BuildObservationUserPrompt(messages); + + // The observer must be able to see WHAT was grepped, not just that grep_files was called + Assert.Contains("grep_files", prompt); + Assert.Contains("Rect", prompt); + Assert.Contains("Termina.Layout", prompt); + } + + [Fact] + public void User_prompt_truncates_extremely_long_tool_call_arguments() + { + var bigBlob = new string('y', 500); + var messages = new List + { + new() + { + Role = ChatRole.Assistant, + Content = string.Empty, + ToolCalls = + [ + new SerializableToolCall + { + CallId = "1", + Name = "huge_tool", + ArgumentsJson = $"{{\"blob\":\"{bigBlob}\"}}" + } + ] + }, + }; + + var prompt = ObservationPromptBuilder.BuildObservationUserPrompt(messages); + + Assert.Contains("huge_tool", prompt); + Assert.Contains("...", prompt); + // Should be nowhere near 500 y's — projection clamps to ~120 chars + Assert.DoesNotContain(new string('y', 200), prompt); + } + + [Fact] + public void WrapObservations_uses_session_summary_marker_with_session_id() { - var text = "- Discussed deployment\n- User prefers Docker"; + var text = "## 1. Primary Request and Intent\nDiscussed deployment"; - var wrapped = ObservationPromptBuilder.WrapObservations(text); + var wrapped = ObservationPromptBuilder.WrapObservations(text, TestSession); - Assert.StartsWith("[observations from earlier in this session]", wrapped); + Assert.StartsWith($"[session-summary session:{TestSession.Value}]", wrapped); + Assert.Contains("Primary Request and Intent", wrapped); + Assert.Contains("Discussed deployment", wrapped); } [Fact] - public void WrapObservations_preserves_existing_delimiter() + public void WrapObservations_rewrites_legacy_observations_header_to_session_summary_marker() { + // Old format: "[observations from earlier in this session]" (pre-rework) + // New format: "[session-summary session:{id}]" — the wrapper normalizes + // any header-like first line to the canonical form. var text = "[observations from earlier in this session]\n- Already formatted"; - var wrapped = ObservationPromptBuilder.WrapObservations(text); + var wrapped = ObservationPromptBuilder.WrapObservations(text, TestSession); + + Assert.StartsWith($"[session-summary session:{TestSession.Value}]", wrapped); + Assert.Contains("Already formatted", wrapped); + Assert.DoesNotContain("observations from earlier", wrapped, StringComparison.Ordinal); + } + + [Fact] + public void ExtractPriorSummary_returns_null_when_no_prior_summary_present() + { + var messages = new List + { + new() { Role = ChatRole.User, Content = "Regular user message" }, + new() { Role = ChatRole.Assistant, Content = "Regular assistant reply" }, + }; + + var (prior, remaining) = ObservationPromptBuilder.ExtractPriorSummary(messages); + + Assert.Null(prior); + Assert.Same(messages, remaining); + } + + [Fact] + public void ExtractPriorSummary_extracts_prior_summary_and_removes_it_from_remaining() + { + var priorSummaryContent = "[session-summary session:test-channel/test-thread]\n## 1. Primary Request and Intent\nPrior goal text"; + var messages = new List + { + new() { Role = ChatRole.User, Content = "Older user turn" }, + new() { Role = ChatRole.User, Content = priorSummaryContent }, + new() { Role = ChatRole.Assistant, Content = "Assistant reply after the summary" }, + new() { Role = ChatRole.User, Content = "Newer user turn" }, + }; + + var (prior, remaining) = ObservationPromptBuilder.ExtractPriorSummary(messages); + + Assert.NotNull(prior); + Assert.Equal(priorSummaryContent, prior); + Assert.Equal(3, remaining.Count); + Assert.DoesNotContain(remaining, m => (m.Content ?? "").StartsWith("[session-summary", StringComparison.Ordinal)); + } + + [Fact] + public void ExtractPriorSummary_takes_the_most_recent_summary_when_multiple_are_present() + { + var first = "[session-summary session:test-channel/test-thread]\nFirst summary"; + var second = "[session-summary session:test-channel/test-thread]\nSecond summary (newer)"; + var messages = new List + { + new() { Role = ChatRole.User, Content = first }, + new() { Role = ChatRole.User, Content = "In between" }, + new() { Role = ChatRole.User, Content = second }, + }; + + var (prior, _) = ObservationPromptBuilder.ExtractPriorSummary(messages); + + Assert.Equal(second, prior); + } + + [Fact] + public void System_prompt_includes_prior_summary_block_when_provided() + { + var prior = "[session-summary session:test-channel/test-thread]\n## 1. Primary Request and Intent\nOld goal: refactor the compactor"; + + var prompt = ObservationPromptBuilder.BuildObservationSystemPrompt(TestSession, prior); + + Assert.Contains("PRIOR SUMMARY", prompt, StringComparison.Ordinal); + Assert.Contains("preserve the bullets", prompt, StringComparison.OrdinalIgnoreCase); + Assert.Contains("Old goal: refactor the compactor", prompt, StringComparison.Ordinal); + } + + [Fact] + public void System_prompt_without_prior_summary_matches_base_shape() + { + var prompt = ObservationPromptBuilder.BuildObservationSystemPrompt(TestSession, priorSummary: null); + + Assert.DoesNotContain("PRIOR SUMMARY", prompt, StringComparison.Ordinal); + // Base prompt is still present — nine-section instructions are there. + Assert.Contains("## 1. Primary Request and Intent", prompt, StringComparison.Ordinal); + Assert.Contains("## 9. Required Files", prompt, StringComparison.Ordinal); + } + + [Fact] + public void WrapObservations_preserves_existing_session_summary_marker_line() + { + // If the model emitted its own session-summary header (because it was + // instructed to preserve a prior one), the wrapper replaces the header + // line with the canonical header carrying the current session id. + var text = "[session-summary session:some-other-session]\n- Body content"; + + var wrapped = ObservationPromptBuilder.WrapObservations(text, TestSession); - Assert.Equal(text, wrapped); + Assert.StartsWith($"[session-summary session:{TestSession.Value}]", wrapped); + Assert.Contains("Body content", wrapped); } } diff --git a/src/Netclaw.Actors/Sessions/ExtractiveSessionReducer.cs b/src/Netclaw.Actors/Sessions/ExtractiveSessionReducer.cs index f17428d74..b9ca23f45 100644 --- a/src/Netclaw.Actors/Sessions/ExtractiveSessionReducer.cs +++ b/src/Netclaw.Actors/Sessions/ExtractiveSessionReducer.cs @@ -8,12 +8,26 @@ namespace Netclaw.Actors.Sessions; /// result messages within the window are preserved (unlike MEAI's /// MessageCountingChatReducer which drops them silently). /// +/// Enforces the netclaw-session "Tool call/result pair integrity" requirement +/// by truncating only at user-message boundaries (OpenCode's approach). In a +/// well-formed MEAI conversation, tool call/result pairs are bracketed by +/// user messages: User → Assistant(with tool_calls) → Tool(result) → Assistant +/// → User. A window that starts on a user message cannot split a pair, since +/// the pair's components are contiguous and either all kept or all discarded. +/// +/// System nudges (user-role messages prefixed with [system:) are not +/// user turns and are skipped during the backward walk — they are recall +/// content / empty-response nudges injected by the actor, not actual user +/// input. +/// /// This reducer is synchronous () and /// cannot fail. The actor calls it via await inside a /// CommandAsync handler, so async reducers are also supported. /// public sealed class ExtractiveSessionReducer : IChatReducer { + private const string SystemNudgePrefix = SessionState.SystemNudgePrefix; + private readonly int _keepRecentMessages; public ExtractiveSessionReducer(int keepRecentMessages) @@ -38,13 +52,44 @@ public Task> ReduceAsync( return Task.FromResult>(list); } - var result = new List(keepCount + systemOffset); + // Keep-zero: drop everything post-system. The caller is explicitly + // saying "only the system prompt survives". + if (keepCount == 0) + { + if (!hasSystemPrompt) + return Task.FromResult>(new List()); + + return Task.FromResult>(new List { list[0] }); + } + + var startIndex = list.Count - keepCount; + + // Pair integrity: walk backward from the naive cut to the nearest + // user-message boundary. A user-role message that is not a system + // nudge is a safe truncation point — tool call/result pairs in the + // conversation are contiguous within a user-to-user envelope, so a + // cut on a user boundary cannot split a pair. + while (startIndex > systemOffset && !IsUserTurn(list[startIndex])) + { + startIndex--; + } + + // Defense in depth: if the walk-back fell through to systemOffset + // and that message is a Tool-role orphan (e.g. recovered from broken + // state or a buggy prior compaction), advance forward past leading + // Tool messages. A kept window that starts with a Tool orphan is + // rejected by downstream providers — we'd rather shrink the window + // than emit an unsendable request. + while (startIndex < list.Count && list[startIndex].Role == ChatRole.Tool) + { + startIndex++; + } + + var result = new List(list.Count - startIndex + systemOffset); if (hasSystemPrompt) result.Add(list[0]); - // Keep last N non-system messages (tool calls and results included) - var startIndex = list.Count - keepCount; for (var i = startIndex; i < list.Count; i++) { result.Add(list[i]); @@ -52,4 +97,19 @@ public Task> ReduceAsync( return Task.FromResult>(result); } + + private static bool IsUserTurn(ChatMessage message) + { + if (message.Role != ChatRole.User) + return false; + + // Skip system nudges — they use User role but are injected by the + // actor (recall content, empty-response nudges), not actual user + // input. + var text = message.Text; + if (!string.IsNullOrEmpty(text) && text.StartsWith(SystemNudgePrefix, StringComparison.Ordinal)) + return false; + + return true; + } } diff --git a/src/Netclaw.Actors/Sessions/LlmSessionActor.cs b/src/Netclaw.Actors/Sessions/LlmSessionActor.cs index 1e7dfe1bc..a5445ffbb 100644 --- a/src/Netclaw.Actors/Sessions/LlmSessionActor.cs +++ b/src/Netclaw.Actors/Sessions/LlmSessionActor.cs @@ -1002,6 +1002,7 @@ private void Compacting() var compactionClient = _compactionClient; var compactionParams = new CompactionParameters( + _sessionId, msg.InputTokenCount, _config.Tuning.KeepRecentToolResults, _config.Tuning.KeepRecentMessages, diff --git a/src/Netclaw.Actors/Sessions/ObservationPromptBuilder.cs b/src/Netclaw.Actors/Sessions/ObservationPromptBuilder.cs index 3ec039cf1..4262e23ea 100644 --- a/src/Netclaw.Actors/Sessions/ObservationPromptBuilder.cs +++ b/src/Netclaw.Actors/Sessions/ObservationPromptBuilder.cs @@ -3,50 +3,194 @@ namespace Netclaw.Actors.Sessions; /// -/// Builds prompts for the Observer phase of compaction. -/// The Observer compresses messages being discarded into concise observation notes -/// that remain in context for future LLM calls. +/// Builds prompts for the Observer phase of compaction. The Observer produces +/// a structured 9-section summary of the discarded conversation window that +/// replaces the verbatim messages going forward. The structure (borrowed from +/// Cline's production summarization prompt) is designed to survive successive +/// compactions without decay — the observer prompt explicitly tells the model +/// to preserve any prior [session-summary session:{id}] block verbatim. /// public static class ObservationPromptBuilder { + internal const int ToolArgsMaxLength = 120; + internal const int ToolResultMaxLength = 1500; + + /// + /// Header marker used to wrap the structured summary when it is stored as + /// a System-role message in post-compaction history. Used by consumers to + /// recognize a prior summary block. + /// + public const string SessionSummaryHeaderPrefix = "[session-summary"; + + /// + /// Scans for the most recent prior + /// [session-summary ...] block and returns (prior summary text, + /// remaining messages). When no prior summary is present, the prior + /// text is null and the message list is returned unchanged. Used by the + /// compaction pipeline to lift a prior summary out of the discarded + /// window and inject it into the observer's system prompt as an + /// explicit "preserve verbatim" block — a structural defense against + /// summary-over-summary decay. + /// + public static (string? PriorSummary, IReadOnlyList Remaining) ExtractPriorSummary( + IReadOnlyList messages) + { + for (var i = messages.Count - 1; i >= 0; i--) + { + var msg = messages[i]; + if (msg.Role != ChatRole.User) + continue; + + var content = msg.Content; + if (string.IsNullOrEmpty(content)) + continue; + + if (!content.StartsWith(SessionSummaryHeaderPrefix, StringComparison.OrdinalIgnoreCase)) + continue; + + var remaining = new List(messages.Count - 1); + for (var j = 0; j < messages.Count; j++) + { + if (j != i) + remaining.Add(messages[j]); + } + + return (content, remaining); + } + + return (null, messages); + } + /// - /// System prompt instructing the model to compress messages into observations. + /// System prompt instructing the observer LLM to produce a structured + /// 9-section summary. Takes the self so the + /// observer can explicitly disambiguate the running session from any + /// foreign session identifiers referenced in the discarded window. + /// When is non-null, appends a block + /// containing the verbatim prior summary and instructs the model to + /// preserve it — the structural anti-decay defense for successive + /// compactions. /// - public static string BuildObservationSystemPrompt() + public static string BuildObservationSystemPrompt(SessionId sessionId, string? priorSummary = null) + { + var basePrompt = BuildBaseSystemPrompt(sessionId); + + if (string.IsNullOrWhiteSpace(priorSummary)) + return basePrompt; + + return $$""" + {{basePrompt}} + + --- + + PRIOR SUMMARY (from a previous compaction of this session). You + MUST preserve the bullets under each of its nine sections verbatim + in your output. Update sections in place with new observations + from the discarded messages below — do not rewrite or rephrase + existing bullets. The prior summary is your anchor; the discarded + messages are new material to fold in. + + {{priorSummary.Trim()}} + """; + } + + private static string BuildBaseSystemPrompt(SessionId sessionId) { - return """ - You are an observation compressor. Your job is to compress conversation messages - into concise, dated observation notes that preserve the most important information. - - Rules: - - Preserve key facts, decisions, user preferences, and action outcomes - - Use bullet points for each observation - - Mark critical observations with [!] prefix - - Include "Current task:" line if there is an active task in progress - - Be extremely concise — aim for 3-10x compression + return $$""" + You are a session summarizer. Your job is to compress conversation messages + into a structured summary that preserves the grounding needed to continue + the work after compaction. + + You are summarizing a session with id `{{sessionId.Value}}`. This is the + SELF session. If observations reference OTHER session ids (from tool calls + or user content), mark them explicitly as `session:{id}` — never conflate + them with the self session. + + CRITICAL RULE: If the input already contains a `[session-summary ...]` + block from a prior compaction, you MUST preserve its sections verbatim + and update them in place. Do not rewrite, re-summarize, or "improve" + prior sections — they have already survived compression and will decay + if touched. Append new observations to the appropriate sections; leave + the rest unchanged. + + Produce your output using EXACTLY these nine section headers in this + order. Leave a section's body empty if nothing applies, but do NOT omit + the header. + + ## 1. Primary Request and Intent + What the user is fundamentally trying to accomplish in this session. + One to three sentences. Copy direct phrasing from the user where + possible — do not paraphrase their stated intent. + + ## 2. Key Technical Concepts + Named technical entities being actively worked on: file paths, type + names, method names, struct/class members, identifiers, external APIs, + frameworks. These are the anchors the agent uses to continue work + after compaction. + + ## 3. Files and Code Sections + Paths the agent has read, edited, or referenced, plus the specific + facts it established about each. Format: + - `src/Rect.cs`: readonly record struct with `Inset(Thickness)`, + `Offset(Point)`, `Contains(Point)`, `Intersect(Rect)` methods + - `src/Thickness.cs`: record struct; `Thickness(left, top, right, bottom)` + + ## 4. Problem Solving + Problems the agent has diagnosed, and how (tool calls used, evidence + gathered, hypotheses rejected). Keep tool-call intent compact: + - `grep_files("Rect", "src/Termina.Layout")` → 7 matches, all in Rect.cs + + ## 5. Pending Tasks + Work items still open, as a bulleted list. Use `[ ]` / `[x]` markers + if progress is clear: + - [ ] Add `Rect.Inflate(int)` method + - [x] Confirm `Inset` returns new Rect + + ## 6. Task Evolution + Direct quotes from user messages that changed or clarified the task. + This section is the single most important anti-drift defense — do NOT + paraphrase. Include at least one direct quote per major task shift. + Format: + - Original: "help me understand Rect" + - Then: "what about Inset specifically?" + - Now: "write a unit test for boundary cases" + + ## 7. Current Work + What the agent is actively doing right now, in one or two sentences. + This is the resume point if the session is interrupted. + + ## 8. Next Step + The immediate next action the agent should take when the session + resumes. One sentence. + + ## 9. Required Files + Bullet list of file paths the agent should re-read to restore context + on resume. Relative paths, most relevant first. + + Additional guidance: - Skip pleasantries, acknowledgments, and filler - - Preserve tool names and outcomes but not full tool arguments/results - - If the user corrected the assistant, note the correction - - Output format: - [observations from earlier in this session] - - [!] User prefers concise output, no caveats - - Discussed deployment strategy for homelab services - - Used shell_execute to check Docker containers — 3 running - - Decision: use Docker Compose for service orchestration - Current task: setting up monitoring with Prometheus + - Preserve user-requested memories verbatim, marked with [!] prefix + - For tool calls, preserve arguments that carry intent (search + patterns, file paths, commands) — not the full argument JSON + - For tool results, extract the key finding (counts, paths, errors, + confirmations) — not the full output + - If the user corrected the assistant, note the correction in section 4 """; } /// - /// Builds the user prompt containing the messages to be compressed. - /// Only includes messages that will be discarded by extractive reduction. + /// Builds the user prompt containing the messages to be compressed. Only + /// includes messages that will be discarded by extractive reduction. Tool + /// calls are rendered with a truncated projection of their arguments so + /// the observer can see the intent (search pattern, file path, command) + /// without drowning in raw argument JSON. /// public static string BuildObservationUserPrompt( IReadOnlyList messagesToCompress) { var sb = new System.Text.StringBuilder(); - sb.AppendLine("Compress the following conversation messages into observation notes:"); + sb.AppendLine("Summarize the following conversation messages using the 9-section template in your system prompt."); + sb.AppendLine("If any message below is itself a `[session-summary ...]` block from a prior compaction, preserve its sections verbatim and only update them with new facts from subsequent messages."); sb.AppendLine(); foreach (var msg in messagesToCompress) @@ -66,7 +210,8 @@ public static string BuildObservationUserPrompt( sb.AppendLine($"**{roleLabel}:**"); foreach (var tc in msg.ToolCalls) { - sb.AppendLine($"[Called: {tc.Name}]"); + var shortArgs = TruncateArgs(tc.ArgumentsJson); + sb.AppendLine($"[Called: {tc.Name}({shortArgs})]"); } if (!string.IsNullOrEmpty(msg.Content)) sb.AppendLine(msg.Content); @@ -74,10 +219,9 @@ public static string BuildObservationUserPrompt( } else if (!string.IsNullOrEmpty(msg.Content)) { - // Truncate very long tool results to keep the prompt manageable var content = msg.Content; - if (msg.Role == ChatRole.Tool && content.Length > 500) - content = content[..497] + "..."; + if (msg.Role == ChatRole.Tool && content.Length > ToolResultMaxLength) + content = content[..(ToolResultMaxLength - 3)] + "..."; sb.AppendLine($"**{roleLabel}:** {content}"); } @@ -87,14 +231,42 @@ public static string BuildObservationUserPrompt( } /// - /// Wraps observation text in the standard delimiter format for inclusion in chat history. + /// Wraps observation text in the standard [session-summary ...] + /// header format for inclusion in history. The session id is embedded in + /// the header so consumers can distinguish a self-session summary from + /// any foreign session id referenced inside the summary body. /// - public static string WrapObservations(string observationText) + public static string WrapObservations(string observationText, SessionId sessionId) { + var header = $"{SessionSummaryHeaderPrefix} session:{sessionId.Value}]"; var trimmed = observationText.Trim(); - if (trimmed.StartsWith("[observations", StringComparison.OrdinalIgnoreCase)) - return trimmed; - return $"[observations from earlier in this session]\n{trimmed}"; + // If the model emitted its own header-like line (possibly from a prior + // summary it was instructed to preserve), strip that line and use our + // canonical header so downstream parsers can reliably locate the + // summary message. + if (trimmed.StartsWith(SessionSummaryHeaderPrefix, StringComparison.OrdinalIgnoreCase) + || trimmed.StartsWith("[observations", StringComparison.OrdinalIgnoreCase)) + { + var newlineIdx = trimmed.IndexOf('\n', StringComparison.Ordinal); + return newlineIdx >= 0 + ? header + trimmed[newlineIdx..] + : header; + } + + return $"{header}\n{trimmed}"; + } + + private static string TruncateArgs(string? argumentsJson) + { + if (string.IsNullOrEmpty(argumentsJson) || argumentsJson == "{}") + return string.Empty; + + var collapsed = System.Text.RegularExpressions.Regex.Replace( + argumentsJson, @"\s+", " ").Trim(); + + return collapsed.Length <= ToolArgsMaxLength + ? collapsed + : collapsed[..(ToolArgsMaxLength - 3)] + "..."; } } diff --git a/src/Netclaw.Actors/Sessions/Pipelines/SessionCompactionPipeline.cs b/src/Netclaw.Actors/Sessions/Pipelines/SessionCompactionPipeline.cs index 12623d1c5..87af6a15c 100644 --- a/src/Netclaw.Actors/Sessions/Pipelines/SessionCompactionPipeline.cs +++ b/src/Netclaw.Actors/Sessions/Pipelines/SessionCompactionPipeline.cs @@ -10,6 +10,7 @@ namespace Netclaw.Actors.Sessions.Pipelines; /// Bundles the many parameters needed by the compaction pipeline into a single record. /// internal sealed record CompactionParameters( + SessionId SessionId, long PreCompactionInputTokens, int KeepRecentToolResults, int KeepRecentMessages, @@ -85,6 +86,7 @@ public static async Task ExecuteAsync( var observationText = await GenerateObservationsAsync( client, + parameters.SessionId, history, systemOffset, discardStartIndex, @@ -97,9 +99,10 @@ public static async Task ExecuteAsync( var observationMsg = new SerializableChatMessage { Role = Protocol.ChatRole.User, - Content = ObservationPromptBuilder.WrapObservations(observationText) + Content = ObservationPromptBuilder.WrapObservations(observationText, parameters.SessionId) }; compactedMessages.Insert(0, observationMsg); + log.Info("Observer: generated {ObsLength} chars of observations from {DiscardedCount} discarded messages", observationText.Length, Math.Max(0, discardStartIndex - systemOffset)); } @@ -131,6 +134,7 @@ public static async Task ExecuteAsync( /// public static async Task GenerateObservationsAsync( IChatClient client, + SessionId sessionId, IReadOnlyList history, int systemOffset, int keepStartIndex, @@ -147,6 +151,18 @@ public static async Task ExecuteAsync( if (discardedMessages.Count == 0) return null; + // Lift any prior [session-summary ...] block out of the discarded + // window and include it in the observer's system prompt with an + // explicit "preserve verbatim" instruction. Structural defense + // against summary-over-summary decay: the prior summary becomes + // an anchor the model sees before the new material, reducing the + // chance of rewrite or decay. + var (priorSummary, remainingDiscarded) = + ObservationPromptBuilder.ExtractPriorSummary(discardedMessages); + + if (priorSummary is not null) + log.Info("Observer: lifting prior session summary ({Length} chars) into system prompt for verbatim preservation", priorSummary.Length); + try { using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); @@ -154,9 +170,9 @@ public static async Task ExecuteAsync( var observerMessages = new List { new(Microsoft.Extensions.AI.ChatRole.System, - ObservationPromptBuilder.BuildObservationSystemPrompt()), + ObservationPromptBuilder.BuildObservationSystemPrompt(sessionId, priorSummary)), new(Microsoft.Extensions.AI.ChatRole.User, - ObservationPromptBuilder.BuildObservationUserPrompt(discardedMessages)) + ObservationPromptBuilder.BuildObservationUserPrompt(remainingDiscarded)) }; var response = await client.GetResponseAsync( diff --git a/src/Netclaw.Actors/Sessions/SessionState.cs b/src/Netclaw.Actors/Sessions/SessionState.cs index 64ef5bbb9..b57dd1093 100644 --- a/src/Netclaw.Actors/Sessions/SessionState.cs +++ b/src/Netclaw.Actors/Sessions/SessionState.cs @@ -42,7 +42,11 @@ public SessionState Apply(SessionTitleSet evt) public SessionState Apply(SessionCompacted evt) { - // Preserve system prompt if present + // Preserve system prompt if present, then layer the compacted messages. + // Summaries are recognizable by their [session-summary session:{id}] + // header — no separate index is persisted. The reducer's + // user-message-boundary walk-back naturally preserves prior summary + // messages because they use User-role and are distinctive. var builder = ImmutableList.CreateBuilder(); if (History.Count > 0 && History[0].Role == ChatRole.System) {