From 8a4bf3d6617e694deee2739838a905efd71b69da Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso <4096860+jkyberneees@users.noreply.github.com> Date: Wed, 16 Sep 2026 16:26:16 +0200 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20side-call=20context=20visibility=20?= =?UTF-8?q?=E2=80=94=20plan=20titles=20in=20summarizer=20payloads=20+=20si?= =?UTF-8?q?de=5Fcall=5Fusage=20events=20(T3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit B3: sideCallPlanPrefix now includes step titles (s2=in_progress - Title). The old IDs-only rationale was wrong for compaction/budget payloads: after a trim those payloads may be the only surviving context, and bare ids tell the summarizer nothing. Titles are model-authored, stored normalized, and already on the main transcript — no new exposure. Notes remain excluded. Measurement: recordSideCallUsage(kind, res) emits a side_call_usage runtime event (typed constant, docs/EXTENSIONS.md updated) plus an internal signal, so --events-jsonl can quantify compaction vs progress_summary vs interrupted_partial token cost. Pinned RED-first; loop_trim_test.go legacy pins updated. internal/loop, internal/events, full cmd/odek green under -race. --- docs/EXTENSIONS.md | 4 +- internal/events/events.go | 1 + internal/loop/loop.go | 53 ++++++++--- internal/loop/loop_trim_test.go | 24 +++-- internal/loop/plan.go | 21 +++++ internal/loop/sidecall_visibility_test.go | 107 ++++++++++++++++++++++ 6 files changed, 186 insertions(+), 24 deletions(-) create mode 100644 internal/loop/sidecall_visibility_test.go diff --git a/docs/EXTENSIONS.md b/docs/EXTENSIONS.md index 82ea30f2..689c68a9 100644 --- a/docs/EXTENSIONS.md +++ b/docs/EXTENSIONS.md @@ -165,7 +165,8 @@ odek can emit a structured runtime event stream: **one JSON object per line `tool_call_started`, `tool_call_completed`, `tool_call_failed`, `session_saved`, `context_trimmed`, `budget_exceeded`, `run_completed`, `run_failed`, `plan_created`, `plan_updated`, `plan_blocked`, `subagent_denied`, - `subagent_spawned`, `subagent_completed`, `subagent_concurrency_wait`. + `subagent_spawned`, `subagent_completed`, `subagent_concurrency_wait`, + `side_call_usage`. - `run_id` is a random 128-bit hex identifier generated per agent run and stamped on every event of that run. `session_id` appears once the session is known; earlier events omit it. `iteration` is the 1-based loop @@ -190,6 +191,7 @@ Per-type `data` fields: | `tool_call_failed` | `call_id`, `duration_ms`, `error_class` | | `session_saved` | `message_count` | | `context_trimmed` | `mode` (`proactive`/`survival`), `dropped_groups`, `truncated_results` | +| `side_call_usage` | `kind` (`compaction`/`progress_summary`/`interrupted_partial`), `input_tokens`, `output_tokens`, `cache_read`, `cache_create` — compaction and budget-summary side-call cost, emitted separately from main-path token accounting | | `budget_exceeded` | `limit_name` (`runtime`/`tool_calls`/`input_tokens`/`output_tokens`/`cost_usd`), `observed`, `limit` | | `run_completed` | `duration_ms` (run wall clock, tools included), `input_tokens`, `output_tokens` (run totals), `llm_duration_ms` (sum of main think-step LLM calls), `tokens_per_second` (think-step output / `llm_duration_ms`; omitted when unknown) | | `run_failed` | `duration_ms`, `error_class`, plus the same `llm_duration_ms` / `tokens_per_second` as `run_completed` when at least one think step was measured before the failure | diff --git a/internal/events/events.go b/internal/events/events.go index ec033391..200a3e72 100644 --- a/internal/events/events.go +++ b/internal/events/events.go @@ -46,6 +46,7 @@ const ( TypePlanBlocked = "plan_blocked" TypeSubagentSpawned = "subagent_spawned" TypeSubagentCompleted = "subagent_completed" + TypeSideCallUsage = "side_call_usage" ) // Budget limit names carried in budget_exceeded events (data.limit_name). diff --git a/internal/loop/loop.go b/internal/loop/loop.go index 8897e167..d10c3958 100644 --- a/internal/loop/loop.go +++ b/internal/loop/loop.go @@ -1738,7 +1738,7 @@ func (e *Engine) applyPendingDigest(ctx context.Context, messages []session.Mess e.pendingDroppedCovered = 0 e.compactMu.Unlock() if usage != nil { - e.recordSideCallUsage(usage) + e.recordSideCallUsage("compaction", usage) } if summary == "" { return messages @@ -1893,7 +1893,7 @@ func (e *Engine) protectDerivedContext(ctx context.Context, source, content stri func (e *Engine) summarizeDropped(ctx context.Context, dropped []session.Message) string { summary, usage := e.summarizeDroppedWithUsage(ctx, dropped, e.compactDigest) if usage != nil { - e.recordSideCallUsage(usage) + e.recordSideCallUsage("compaction", usage) } return summary } @@ -1957,9 +1957,12 @@ func (e *Engine) summarizeDroppedWithUsage(ctx context.Context, dropped []sessio return strings.TrimSpace(res.Content), res } -// sideCallPlanPrefix prepends remaining plan step IDs and statuses to a -// compaction or progress-summary user payload. Titles and notes stay out — -// they already live in the wrapped plan message on the main transcript. +// sideCallPlanPrefix prepends remaining plan steps to a compaction or +// progress-summary user payload, WITH titles: these payloads may be the +// only surviving context after a trim (the wrapped plan message itself can +// be dropped), so bare ids tell the summarizer nothing. Titles are +// model-authored and stored normalized — no new exposure. (Stall hints +// keep the title-free format; that pin is unchanged.) func (e *Engine) sideCallPlanPrefix() string { if e == nil || e.planStore == nil { return "" @@ -1968,11 +1971,11 @@ func (e *Engine) sideCallPlanPrefix() string { if !ok { return "" } - ids := formatRemainingPlanSteps(state) + ids := formatRemainingPlanStepsDetailed(state) if ids == "" { return "" } - return "Remaining plan steps (ids and statuses only): " + ids + "\n\n" + return "Remaining plan steps: " + ids + "\n\n" } // ── Protected plan message (digest-pattern integration) ─────────────── @@ -2143,10 +2146,10 @@ func (e *Engine) summarizeProgress(ctx context.Context, messages []session.Messa // calls is not a summary (its content is pre-tool chatter), so treat it // as a failure and keep the original error path. if len(res.ToolCalls) > 0 || (res.Termination != "" && res.Termination != llmclient.TerminationComplete) { - e.recordSideCallUsage(res) + e.recordSideCallUsage("progress_summary", res) return "" } - e.recordSideCallUsage(res) + e.recordSideCallUsage("progress_summary", res) return strings.TrimSpace(res.Content) } @@ -2198,21 +2201,41 @@ func (e *Engine) budgetExceeded(ctx context.Context, messages []session.Message, // per-run totals. Totals feed budget enforcement (max_input_tokens / // max_output_tokens / cost caps) and usage reporting; a side call invisible // to them silently exceeds the caps and under-reports consumption. -func (e *Engine) recordSideCallUsage(res *llmclient.CallResult) { +// recordSideCallUsage merges a compaction/progress-summary side call's +// tokens into the run totals and emits a side_call_usage signal so run +// events (--events-jsonl) and /api/usage consumers can quantify the +// side-call cost separately from the main conversation. +func (e *Engine) recordSideCallUsage(kind string, res *llmclient.CallResult) { if res == nil { return } e.externalChargeMu.Lock() - defer e.externalChargeMu.Unlock() e.TotalInputTokens += res.InputTokens e.TotalOutputTokens += res.OutputTokens e.TotalCacheCreationTokens += res.CacheCreationTokens e.TotalCacheReadTokens += res.CacheReadTokens e.TotalCachedTokens += res.CachedTokens e.TotalCacheReported = e.TotalCacheReported || res.CacheReported - // Note: side calls (compaction/budget summaries) deliberately do NOT - // update lastPromptTokens — their prompts are tiny {system, snippet}, - // not the conversation window. + e.externalChargeMu.Unlock() + // Note: side calls deliberately do NOT update lastPromptTokens — their + // prompts are tiny {system, snippet}, not the conversation window. + e.emitSignal(SignalEvent{ + Type: "side_call_usage", + Tool: kind, + Detail: fmt.Sprintf("in=%d out=%d cache_read=%d cache_create=%d", + res.InputTokens, res.OutputTokens, res.CacheReadTokens, res.CacheCreationTokens), + }) + e.emitEvent(events.Event{ + Type: events.TypeSideCallUsage, + Tool: kind, + Data: map[string]any{ + "kind": kind, + "input_tokens": res.InputTokens, + "output_tokens": res.OutputTokens, + "cache_read": res.CacheReadTokens, + "cache_create": res.CacheCreationTokens, + }, + }) } // promptWindowTokens normalizes a call's provider-reported usage into the @@ -2773,7 +2796,7 @@ func (e *Engine) runLoop(ctx context.Context, in []session.Message) (answer stri result, err := e.callLLM(ctx, messages, tools) if err != nil { if result != nil { - e.recordSideCallUsage(result) + e.recordSideCallUsage("interrupted_partial", result) if result.Content != "" && !isContextLengthError(err) { partial := "[Partial response: interrupted]\n\n" + result.Content messages = append(messages, session.Message{Role: "assistant", Content: partial, ReasoningContent: result.ReasoningContent}) diff --git a/internal/loop/loop_trim_test.go b/internal/loop/loop_trim_test.go index 59778caa..8efa585f 100644 --- a/internal/loop/loop_trim_test.go +++ b/internal/loop/loop_trim_test.go @@ -832,7 +832,7 @@ func TestCompactionSystemPrompt_SkeletonAndIPI(t *testing.T) { } } -func TestSummarizeDropped_IncludesRemainingPlanIDsNotTitles(t *testing.T) { +func TestSummarizeDropped_IncludesRemainingPlanIDsAndTitles(t *testing.T) { var bodies []string server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { data, _ := io.ReadAll(r.Body) @@ -854,12 +854,17 @@ func TestSummarizeDropped_IncludesRemainingPlanIDsNotTitles(t *testing.T) { if !strings.Contains(body, "s2=in_progress") || !strings.Contains(body, "s3=pending") { t.Errorf("remaining plan ids missing from summarizer input: %.400s", body) } - if strings.Contains(body, secretPlanTitle) || strings.Contains(body, secretPlanNote) { - t.Errorf("plan titles/notes leaked into summarizer input: %.400s", body) + // Titles are model-authored context the summarizer needs (the plan + // message itself may be the dropped content). Notes stay out. + if !strings.Contains(body, secretPlanTitle) { + t.Errorf("remaining plan titles missing from summarizer input: %.400s", body) + } + if strings.Contains(body, secretPlanNote) { + t.Errorf("plan notes leaked into summarizer input: %.400s", body) } } -func TestExtractiveDigest_IncludesRemainingPlanIDsNotTitles(t *testing.T) { +func TestExtractiveDigest_IncludesRemainingPlanIDsAndTitles(t *testing.T) { store := NewPlanStore(12, 2000) seedPlanMessage(t, store) engine := &Engine{planStore: store} @@ -867,8 +872,11 @@ func TestExtractiveDigest_IncludesRemainingPlanIDsNotTitles(t *testing.T) { if !strings.Contains(got, "s2=in_progress") || !strings.Contains(got, "s3=pending") { t.Errorf("remaining plan ids missing from extractive digest: %.400s", got) } - if strings.Contains(got, secretPlanTitle) || strings.Contains(got, secretPlanNote) { - t.Errorf("plan titles/notes leaked into extractive digest: %.400s", got) + if !strings.Contains(got, secretPlanTitle) { + t.Errorf("remaining plan titles missing from extractive digest: %.400s", got) + } + if strings.Contains(got, secretPlanNote) { + t.Errorf("plan notes leaked into extractive digest: %.400s", got) } if !strings.Contains(got, "old work") { t.Errorf("dropped content missing from extractive digest: %.400s", got) @@ -900,8 +908,8 @@ func TestSummarizeProgress_IncludesRemainingPlanIDs(t *testing.T) { if !strings.Contains(body, "s2=in_progress") { t.Errorf("remaining plan ids missing from progress summarizer input: %.400s", body) } - if strings.Contains(body, secretPlanTitle) { - t.Errorf("plan title leaked into progress summarizer input: %.400s", body) + if !strings.Contains(body, secretPlanTitle) { + t.Errorf("remaining plan titles missing from progress summarizer input: %.400s", body) } } diff --git a/internal/loop/plan.go b/internal/loop/plan.go index 3f1962a1..752fdc68 100644 --- a/internal/loop/plan.go +++ b/internal/loop/plan.go @@ -507,6 +507,27 @@ func formatRemainingPlanSteps(state PlanState) string { return strings.Join(parts, " ") } +// formatRemainingPlanStepsDetailed is the side-call variant: it appends the +// normalized step title ("s1=pending - Ship the parser"). Titles are +// model-authored, already on the main transcript, and stored normalized +// (newlines/em dashes flattened at create), so the grammar stays one step +// per token. The summarizer needs them — its payload may be the only +// context where the plan survives after a trim. +func formatRemainingPlanStepsDetailed(state PlanState) string { + var parts []string + for _, st := range state.Steps { + if st.Status == StepDone { + continue + } + if st.Title != "" { + parts = append(parts, st.ID+"="+string(st.Status)+" - "+st.Title) + } else { + parts = append(parts, st.ID+"="+string(st.Status)) + } + } + return strings.Join(parts, "; ") +} + // normalizePlanText flattens text so the rendered line grammar stays // unambiguous: newlines become spaces (one step = one line) and em dashes // become hyphens (the renderer reserves " — " as the title/note separator). diff --git a/internal/loop/sidecall_visibility_test.go b/internal/loop/sidecall_visibility_test.go new file mode 100644 index 00000000..8d997ca3 --- /dev/null +++ b/internal/loop/sidecall_visibility_test.go @@ -0,0 +1,107 @@ +package loop + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/BackendStack21/odek/internal/session" + "github.com/BackendStack21/odek/internal/tool" +) + +// ── B3: side-call plan prefix carries step TITLES ──────────────────────── +// +// sideCallPlanPrefix feeds the compaction digest and budget-summary +// payloads — contexts where the wrapped plan message itself may be the +// dropped content. "s1=pending" without a title tells the summarizer +// nothing about what remains. Titles are model-authored and already on +// the main transcript, so including them adds no new exposure. (The +// anti-title pin applies to stall HINTS — a different path, unchanged.) + +func TestSideCallPlanPrefix_IncludesTitles(t *testing.T) { + engine := New(testChatClient(t, newIdleServer()), tool.NewRegistry(nil), 10, "sys", nil, 0) + store := NewPlanStore(0, 0) + if _, err := store.Execute(`{"verb":"create","steps":[` + + `{"id":"s1","title":"Ship the parser"},` + + `{"id":"s2","title":"Write the docs"}]}`); err != nil { + t.Fatalf("plan create: %v", err) + } + engine.planStore = store + + prefix := engine.sideCallPlanPrefix() + if prefix == "" { + t.Fatal("sideCallPlanPrefix = empty, want remaining steps") + } + if !strings.Contains(prefix, "s1=pending") { + t.Errorf("prefix lost the id=status contract: %q", prefix) + } + if !strings.Contains(prefix, "Ship the parser") || !strings.Contains(prefix, "Write the docs") { + t.Errorf("prefix must include step titles (the summarizer cannot know what 's1' means): %q", prefix) + } +} + +// ── Measurement: side-call usage is observable ─────────────────────────── +// +// recordSideCallUsage merges compaction/summary tokens into the run totals +// with no separate accounting, so the side-call cost is invisible to +// /api/usage and events. Every side call must emit a side_call_usage +// signal carrying its token split so run events (--events-jsonl) can +// quantify it. + +func TestSideCallUsageSignal_EmittedOnCompaction(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `{"choices":[{"message":{"content":"summary"}}],"usage":{"prompt_tokens":120,"completion_tokens":15}}`) + })) + defer server.Close() + + client := testChatClient(t, server.URL) + engine := New(client, tool.NewRegistry(nil), 10, "sys", nil, 0) + engine.SetCompaction(true) + + var usageSignals []SignalEvent + engine.SetSignalHandler(func(ev SignalEvent) { + if ev.Type == "side_call_usage" { + usageSignals = append(usageSignals, ev) + } + }) + + msgs := []session.Message{ + {Role: "system", Content: "sys"}, + {Role: "user", Content: "task"}, + } + dropped := []session.Message{{Role: "tool", Content: "dropped output"}} + out := engine.refreshDigest(context.Background(), msgs, dropped) + engine.waitDigestSideCall(context.Background()) + engine.applyPendingDigest(context.Background(), out) + _ = out + + if len(usageSignals) == 0 { + t.Fatal("no side_call_usage signal after compaction side call — side-call cost is unobservable") + } + ev := usageSignals[0] + if ev.Tool != "compaction" { + t.Errorf("side_call_usage Tool = %q, want kind %q", ev.Tool, "compaction") + } + if !strings.Contains(ev.Detail, "in=") || !strings.Contains(ev.Detail, "out=") { + t.Errorf("side_call_usage Detail must carry the in/out token split: %q", ev.Detail) + } +} + +func TestRecordSideCallUsage_NilResultNoSignal(t *testing.T) { + engine := New(testChatClient(t, newIdleServer()), tool.NewRegistry(nil), 10, "sys", nil, 0) + var n int + engine.SetSignalHandler(func(ev SignalEvent) { + if ev.Type == "side_call_usage" { + n++ + } + }) + engine.recordSideCallUsage("compaction", nil) + if n != 0 { + t.Errorf("nil result must not emit a signal, got %d", n) + } +} + +var _ = session.Message{} From 8997f1a640a2e2fe2555ea0a5fee29a738637b0b Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso <4096860+jkyberneees@users.noreply.github.com> Date: Wed, 16 Sep 2026 16:37:40 +0200 Subject: [PATCH 2/2] fix: main_partial kind + title separator hygiene (review round) --- docs/EXTENSIONS.md | 2 +- internal/loop/loop.go | 2 +- internal/loop/plan.go | 6 +++++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/EXTENSIONS.md b/docs/EXTENSIONS.md index 689c68a9..6aa679b1 100644 --- a/docs/EXTENSIONS.md +++ b/docs/EXTENSIONS.md @@ -191,7 +191,7 @@ Per-type `data` fields: | `tool_call_failed` | `call_id`, `duration_ms`, `error_class` | | `session_saved` | `message_count` | | `context_trimmed` | `mode` (`proactive`/`survival`), `dropped_groups`, `truncated_results` | -| `side_call_usage` | `kind` (`compaction`/`progress_summary`/`interrupted_partial`), `input_tokens`, `output_tokens`, `cache_read`, `cache_create` — compaction and budget-summary side-call cost, emitted separately from main-path token accounting | +| `side_call_usage` | `kind` (`compaction`/`progress_summary`/`main_partial`), `input_tokens`, `output_tokens`, `cache_read`, `cache_create` — `compaction`/`progress_summary` are side-call cost emitted separately from main-path token accounting; `main_partial` covers a failed main-path call's partial tokens (pre-existing charging, now observable) | | `budget_exceeded` | `limit_name` (`runtime`/`tool_calls`/`input_tokens`/`output_tokens`/`cost_usd`), `observed`, `limit` | | `run_completed` | `duration_ms` (run wall clock, tools included), `input_tokens`, `output_tokens` (run totals), `llm_duration_ms` (sum of main think-step LLM calls), `tokens_per_second` (think-step output / `llm_duration_ms`; omitted when unknown) | | `run_failed` | `duration_ms`, `error_class`, plus the same `llm_duration_ms` / `tokens_per_second` as `run_completed` when at least one think step was measured before the failure | diff --git a/internal/loop/loop.go b/internal/loop/loop.go index d10c3958..e920ce72 100644 --- a/internal/loop/loop.go +++ b/internal/loop/loop.go @@ -2796,7 +2796,7 @@ func (e *Engine) runLoop(ctx context.Context, in []session.Message) (answer stri result, err := e.callLLM(ctx, messages, tools) if err != nil { if result != nil { - e.recordSideCallUsage("interrupted_partial", result) + e.recordSideCallUsage("main_partial", result) if result.Content != "" && !isContextLengthError(err) { partial := "[Partial response: interrupted]\n\n" + result.Content messages = append(messages, session.Message{Role: "assistant", Content: partial, ReasoningContent: result.ReasoningContent}) diff --git a/internal/loop/plan.go b/internal/loop/plan.go index 752fdc68..32ffadca 100644 --- a/internal/loop/plan.go +++ b/internal/loop/plan.go @@ -520,7 +520,11 @@ func formatRemainingPlanStepsDetailed(state PlanState) string { continue } if st.Title != "" { - parts = append(parts, st.ID+"="+string(st.Status)+" - "+st.Title) + // ';' is the list separator on this surface — strip it from titles + // (nothing machine-parses the list, but unambiguous tokens keep + // the summarizer from seeing phantom steps). + title := strings.ReplaceAll(st.Title, ";", ",") + parts = append(parts, st.ID+"="+string(st.Status)+" - "+title) } else { parts = append(parts, st.ID+"="+string(st.Status)) }