fix(compact): preserve replay ownership (PR2 round 2)
This commit is contained in:
@@ -13,7 +13,7 @@ This backend owns the compaction policy:
|
||||
- **Convergence** — retry head-checkpoint compaction up to `compactionRetries`; reject a summary that does not shrink its source, and throw if retries cannot return below threshold.
|
||||
- **Summarization** — a direct `llm/stream` call uses the configured model and cap without running the loop-only `agent/request` seam. The input transcript preserves non-text blocks as tagged placeholders; only returned text enters the checkpoint, excluding reasoning and tool calls that would leak private reasoning or create an orphaned call.
|
||||
- **Framing** — the replacement user message marks established checkpoint context with `<compacted-summary>` tags. The raw summary remains on the provenance event, and later automatic cycles merge the prior checkpoint.
|
||||
- **Lifecycle** — `compactRegion()` records its start, summary, replacement, and end. The serial `agent/pre-step` listener checks pressure before every step, outside an open step, so a tool-heavy turn remains compactable and the loop derives history once after mutation.
|
||||
- **Lifecycle** — `compactRegion()` requires its agent to own the exact target session and rejects mismatch before resolution or mutation; a valid call records its start, summary, replacement, and end. The serial `agent/pre-step` listener checks pressure before every step, outside an open step, so a tool-heavy turn remains compactable and the loop derives history once after mutation.
|
||||
- **Failure handling** — an unmatched `compact/start` is an inert crash marker because no replacement landed. Recoverable failure records an error end and leaves the surface unchanged.
|
||||
|
||||
`summarize()` is the sole subclass hook. A template- or remote-summarizer subclass can override it while pressure, retention, provenance, shrink validation, and shadowed-token accounting stay on the conversation model's meter. The hook returns the summary blocks together with the call envelope it used (`{ summary, model, maxTokens? }`), which is logged on `compact/summary`.
|
||||
|
||||
@@ -159,11 +159,12 @@ export class BasicCompactService extends CompactService {
|
||||
|
||||
/**
|
||||
* Compact one inclusive positional surface range using the effective
|
||||
* conversation model for all retention and shrink pricing.
|
||||
* @param session - session whose surface is mutated.
|
||||
* conversation model for all retention and shrink pricing. Reject an agent
|
||||
* that does not own the exact target before any resolution or mutation.
|
||||
* @param session - session whose surface is mutated; must equal `agent.session`.
|
||||
* @param start - inclusive first surface-node seq.
|
||||
* @param end - inclusive last surface-node seq.
|
||||
* @param agent - agent used by the summarizer and model resolver.
|
||||
* @param agent - owner of the target session, used by the summarizer and model resolver.
|
||||
* @param signal - optional summarization cancellation signal.
|
||||
* @returns the successful durable compaction result.
|
||||
*/
|
||||
@@ -174,6 +175,9 @@ export class BasicCompactService extends CompactService {
|
||||
agent: Agent,
|
||||
signal?: AbortSignal,
|
||||
): Promise<CompactionResult> {
|
||||
if (session !== agent.session) {
|
||||
throw new Error('compactRegion: agent.session must be the exact target session')
|
||||
}
|
||||
const model = effectiveModel(agent)
|
||||
if (model === undefined || model.length === 0) {
|
||||
throw new Error('compactRegion: no routed or configured conversation model is available for token pricing')
|
||||
|
||||
@@ -361,6 +361,26 @@ describe('pressure measurement and retention', () => {
|
||||
})
|
||||
|
||||
describe('compaction region transaction', () => {
|
||||
it('rejects an agent that does not own the exact target session before mutation', async () => {
|
||||
const compact = service()
|
||||
const target = conversation(2)
|
||||
const owner = conversation(1)
|
||||
const targetEvents = [...target.events]
|
||||
const ownerEvents = [...owner.events]
|
||||
const nodes = target.surface.nodes
|
||||
|
||||
await expect(compact.compactRegion(
|
||||
target,
|
||||
nodes[0]!.seq,
|
||||
nodes[1]!.seq,
|
||||
agent(owner),
|
||||
)).rejects.toThrow('compactRegion: agent.session must be the exact target session')
|
||||
|
||||
expect(target.events).toEqual(targetEvents)
|
||||
expect(owner.events).toEqual(ownerEvents)
|
||||
expect(compact.calls).toEqual([])
|
||||
})
|
||||
|
||||
it('lands a framed, replayable checkpoint with exact pricing provenance', async () => {
|
||||
const compact = service()
|
||||
const session = conversation(3)
|
||||
|
||||
@@ -19,9 +19,9 @@ Both methods are **abstract** — the backend owns trigger policy, retention, ev
|
||||
| Member | Semantics |
|
||||
|---|---|
|
||||
| `compactIfNeeded(agent, fullSystemPrompt, sessionPrefix, signal)` | Estimate the surface-derived history size; if over the backend's threshold, compact an older range via `compactRegion`, keeping recent context intact. Returns the `CompactionResult`, or `null` if nothing needed compacting. All parameters required — the loop's `agent/pre-step` checkpoint supplies the agent, assembled `fullSystemPrompt`, composed `sessionPrefix` (request-only messages every request carries but the derived history omits — the pressure estimate must count them), and turn `signal`. A backend's summarization request is a direct `ctx.llm.stream()` call (not a loop step), so per-call interception happens at `llm/stream`. |
|
||||
| `compactRegion(session, start, end, agent, signal?)` | Forcibly summarize surface nodes `[start, end]` (inclusive seqs) into a single replacement node. **Throws** if a compaction is already in progress, if `start`/`end` aren't surface nodes, or if `start` is positioned after `end` on the surface. The range is a SURFACE-POSITION span, not a numeric seq interval — after a prior replace lands a fresh high-seq summary node at the shadowed range's position, surface order no longer tracks seq order. |
|
||||
| `compactRegion(session, start, end, agent, signal?)` | Forcibly summarize surface nodes `[start, end]` (inclusive seqs) into a single replacement node. The agent must own the exact target (`session === agent.session`); a backend rejects mismatch before model resolution, lock acquisition, summarization, or log mutation. **Throws** if a compaction is already in progress, if `start`/`end` aren't surface nodes, or if `start` is positioned after `end` on the surface. The range is a SURFACE-POSITION span, not a numeric seq interval — after a prior replace lands a fresh high-seq summary node at the shadowed range's position, surface order no longer tracks seq order. |
|
||||
|
||||
`compactIfNeeded` takes a required `signal`; `compactRegion`'s is optional. A backend that summarizes via `ctx.llm.stream()` **must** forward it into the call's `GenerateOptions.signal`, so an abort or fiber dispose tears down the in-flight summarization instead of leaving an orphaned model call running past the cancellation. The session being compacted comes from the agent context; the turn that the `compact/*` events belong to is recoverable from the log (the currently-open turn), so the backend stamps it from the log rather than trusting a caller-supplied value.
|
||||
`compactIfNeeded` takes a required `signal`; `compactRegion`'s is optional. A backend that summarizes via `ctx.llm.stream()` **must** forward it into the call's `GenerateOptions.signal`, so an abort or fiber dispose tears down the in-flight summarization instead of leaving an orphaned model call running past the cancellation. The turn that the `compact/*` events belong to is recoverable from the owned session's log (the currently-open turn), so the backend stamps it from the log rather than trusting a caller-supplied value.
|
||||
|
||||
## Tool-pairing boundaries
|
||||
|
||||
|
||||
@@ -67,17 +67,19 @@ export abstract class CompactService extends Service {
|
||||
* `start` and `end` name an inclusive span by surface position, not numeric seq
|
||||
* order; replacements can make visible seqs non-monotonic. Both edges must be
|
||||
* balanced so assistant tool calls remain paired with their results. A model-
|
||||
* backed implementation forwards cancellation and rejects active, missing,
|
||||
* reversed, or unbalanced ranges.
|
||||
* backed implementation forwards cancellation. The agent must own the exact
|
||||
* target session object; implementations reject an ownership mismatch before
|
||||
* model resolution, lock acquisition, summarization, or log mutation, and
|
||||
* reject active, missing, reversed, or unbalanced ranges.
|
||||
* Use {@link toolPairingBalancedBefore} and {@link toolPairingBalancedAfter}
|
||||
* for the edge checks.
|
||||
*
|
||||
* @param session - session to mutate.
|
||||
* @param session - session to mutate; must be identical to `agent.session`.
|
||||
* @param start - first surface seq, inclusive.
|
||||
* @param end - last surface seq, inclusive.
|
||||
* @param agent - summarizer context.
|
||||
* @param agent - owner of the target session and summarizer context.
|
||||
* @param signal - optional cancellation; model-backed implementations must forward it.
|
||||
* @throws when compaction is active or the range is missing, reversed, or unbalanced.
|
||||
* @throws when the agent does not own `session`, compaction is active, or the range is missing, reversed, or unbalanced.
|
||||
* @returns the replaced range and summary.
|
||||
*/
|
||||
abstract compactRegion(
|
||||
|
||||
Reference in New Issue
Block a user