feat(agent): context trace, partial-success terminal, and scored self-critique

README.md

@@ -51,7 +51,11 @@ go get github.com/hung12ct/gopheragent
   load only when a skill is used.
 - **Deterministic ReAct loop** — dependency-aware parallel tool scheduling with
   `<output_of:ID>` refs, anti-loop detection, token-budget-aware pruning.
-- **Streaming & HITL** — SSE streaming, human approvals, plan mode, self-critique.
+- **Streaming & HITL** — SSE streaming, human approvals, plan mode, and
+  self-critique that keeps the best-scoring pass instead of the last one.
+- **Honest terminals** — a `context_trace` event says exactly which messages
+  pruning rewrote and why; a `degraded` terminal reports "the artifact landed,
+  the bookkeeping did not" instead of forcing a turn into success or failure.
 - **Custom tools** — one interface, schema derived from a Go struct; a middleware
   chain for logging, timing, rate limiting, and tracing.
 - **Multi-provider** — OpenAI, Anthropic, Gemini, Vertex, and OpenAI-compatible

pkg/agent/constants.go

@@ -18,4 +18,12 @@ const (
 	// budgetWarnRatio is the fraction of MaxTokenBudget at which
 	// aggressive context pruning kicks in (truncate tool arguments).
 	budgetWarnRatio = 0.85
+
+	// defaultProtectedEnds is how many trailing messages the routine
+	// depth prune leaves untouched on every LLM call.
+	defaultProtectedEnds = 3
+
+	// emergencyProtectedEnds is the shallower protection used once the
+	// estimate has blown past MaxTokenBudget outright.
+	emergencyProtectedEnds = 1
 )

pkg/agent/context_trace.go

@@ -0,0 +1,109 @@
+package agent
+
+import (
+	"context"
+
+	"github.com/hung12ct/gopheragent/pkg/history"
+)
+
+// ContextChangeReason classifies why the loop rewrote a message before
+// handing the conversation to the provider. It is a closed enum — the
+// pruner has exactly three ways to shrink a message.
+type ContextChangeReason string
+
+const (
+	// ContextChangeSoftTrim: a long tool/assistant message had its middle
+	// cut out by the depth-based prune; head and tail survive.
+	ContextChangeSoftTrim ContextChangeReason = "soft-trim"
+	// ContextChangeOutlierDiscarded: the message blew past the outlier
+	// ceiling and its entire payload was replaced by a system notice. This
+	// is the only reason that loses content outright.
+	ContextChangeOutlierDiscarded ContextChangeReason = "outlier-discarded"
+	// ContextChangeArgsTruncated: a tool result was clipped to its first
+	// few hundred runes because the run crossed the token-budget warn
+	// threshold.
+	ContextChangeArgsTruncated ContextChangeReason = "args-truncated"
+)
+
+// ContextPolicy names which enforceTokenBudget path produced a trace, so a
+// host can tell routine depth pruning apart from budget pressure.
+type ContextPolicy string
+
+const (
+	// ContextPolicyDefault is the unbudgeted path — MaxTokenBudget is
+	// unset, so only the standard depth prune ran.
+	ContextPolicyDefault ContextPolicy = "default"
+	// ContextPolicyBudgetWarn: the estimate crossed budgetWarnRatio of
+	// MaxTokenBudget and tool arguments were truncated.
+	ContextPolicyBudgetWarn ContextPolicy = "budget-warn"
+	// ContextPolicyBudgetEmergency: the estimate exceeded MaxTokenBudget
+	// outright and the aggressive shallow prune ran.
+	ContextPolicyBudgetEmergency ContextPolicy = "budget-emergency"
+)
+
+// ContextRef identifies one message the pruner rewrote for a single LLM
+// call. Index is the position in the pre-prune slice; because no pruning
+// path reorders or removes messages, it also indexes the post-prune
+// slice — and, since the loop prunes a transient copy and persists the
+// conversation at full fidelity, the same position in the slice returned
+// by Sessions.History. That is what makes a trace joinable back to the
+// stored transcript.
+//
+// ToolCallID and CorrelationID are the stable handles for naming a tool
+// result across events — CorrelationID is the agent-generated ID also
+// carried by ToolCallEvent, so a host can join a trimmed message back to
+// the call that produced it. Both are empty for assistant messages.
+//
+// EstTokensBefore/After use the same 4-chars/token heuristic as
+// MaxTokenBudget enforcement, so they are comparable with the event's
+// totals but are not exact provider counts.
+type ContextRef struct {
+	Index           int                 `json:"index"`
+	Role            string              `json:"role"`
+	ToolCallID      string              `json:"tool_call_id,omitempty"`
+	CorrelationID   string              `json:"correlation_id,omitempty"`
+	Reason          ContextChangeReason `json:"reason"`
+	EstTokensBefore int                 `json:"est_tokens_before"`
+	EstTokensAfter  int                 `json:"est_tokens_after"`
+}
+
+// contextRefFor builds a trace entry for msg at index i. estAfter is the
+// post-rewrite estimate; the caller supplies it because only the rewriting
+// branch knows the replacement content.
+func contextRefFor(i int, msg history.Message, reason ContextChangeReason, estBefore, estAfter int) ContextRef {
+	return ContextRef{
+		Index:           i,
+		Role:            msg.Role,
+		ToolCallID:      msg.ToolCallID,
+		CorrelationID:   msg.CorrelationID,
+		Reason:          reason,
+		EstTokensBefore: estBefore,
+		EstTokensAfter:  estAfter,
+	}
+}
+
+// emitContextTrace fires a ContextTraceEvent describing what the pruner
+// changed on the way into one LLM call. No-op when nothing was rewritten,
+// which is the common case — a turn whose messages all fit emits nothing,
+// and the two estimateTokens sweeps only run when there is something to
+// report.
+func (al *AgentLoop) emitContextTrace(
+	ctx context.Context,
+	sessionKey string,
+	streamChan chan<- StreamEvent,
+	iteration int,
+	policy ContextPolicy,
+	before, after []history.Message,
+	changes []ContextRef,
+) {
+	if len(changes) == 0 {
+		return
+	}
+	al.emit(ctx, sessionKey, streamChan, Event(ContextTraceEvent{
+		Policy:          policy,
+		Iteration:       iteration,
+		Changes:         changes,
+		EstTokensBefore: estimateTokens(before),
+		EstTokensAfter:  estimateTokens(after),
+	}))
+}

pkg/agent/context_trace_test.go

@@ -0,0 +1,158 @@
+package agent
+
+import (
+	"context"
+	"strings"
+	"testing"
+
+	"github.com/hung12ct/gopheragent/pkg/history"
+)
+
+// collectTrace drains ch and returns every ContextTraceEvent payload seen.
+func collectTrace(ch chan StreamEvent) []ContextTraceEvent {
+	close(ch)
+	var out []ContextTraceEvent
+	for ev := range ch {
+		if p, ok := ev.Payload.(ContextTraceEvent); ok {
+			out = append(out, p)
+		}
+	}
+	return out
+}
+
+func TestEnforceTokenBudget_UnbudgetedPathEmitsTrace(t *testing.T) {
+	al := &AgentLoop{}
+	ch := make(chan StreamEvent, 16)
+	msgs := []history.Message{
+		{Role: "tool", Content: strings.Repeat("x", softTrimThreshold+100), ToolCallID: "call-1", CorrelationID: "corr-1"},
+		{Role: "user", Content: "hi"},
+		{Role: "assistant", Content: "ok"},
+		{Role: "user", Content: "again"},
+	}
+
+	al.enforceTokenBudget(context.Background(), "s", ch, 2, msgs)
+
+	traces := collectTrace(ch)
+	if len(traces) != 1 {
+		t.Fatalf("want 1 trace on the unbudgeted path, got %d", len(traces))
+	}
+	tr := traces[0]
+	if tr.Policy != ContextPolicyDefault {
+		t.Fatalf("policy = %q, want %q", tr.Policy, ContextPolicyDefault)
+	}
+	if tr.Iteration != 2 {
+		t.Fatalf("iteration = %d, want 2", tr.Iteration)
+	}
+	if len(tr.Changes) != 1 {
+		t.Fatalf("want 1 change, got %d", len(tr.Changes))
+	}
+	c := tr.Changes[0]
+	if c.Index != 0 || c.Role != "tool" || c.Reason != ContextChangeSoftTrim {
+		t.Fatalf("change = %+v, want index 0 / tool / soft-trim", c)
+	}
+	if c.ToolCallID != "call-1" || c.CorrelationID != "corr-1" {
+		t.Fatalf("change lost its correlation handles: %+v", c)
+	}
+	if c.EstTokensAfter >= c.EstTokensBefore {
+		t.Fatalf("est tokens should shrink, got before=%d after=%d", c.EstTokensBefore, c.EstTokensAfter)
+	}
+	if tr.EstTokensAfter >= tr.EstTokensBefore {
+		t.Fatalf("totals should shrink, got before=%d after=%d", tr.EstTokensBefore, tr.EstTokensAfter)
+	}
+}
+
+func TestEnforceTokenBudget_NoChangeEmitsNothing(t *testing.T) {
+	al := &AgentLoop{}
+	ch := make(chan StreamEvent, 16)
+	msgs := []history.Message{
+		{Role: "user", Content: "hi"},
+		{Role: "assistant", Content: "short"},
+		{Role: "user", Content: "again"},
+	}
+
+	al.enforceTokenBudget(context.Background(), "s", ch, 0, msgs)
+
+	if traces := collectTrace(ch); len(traces) != 0 {
+		t.Fatalf("want no trace when nothing was pruned, got %d", len(traces))
+	}
+}
+
+func TestEnforceTokenBudget_EmergencyPolicyTagged(t *testing.T) {
+	// MaxTokenBudget=1 puts the estimate past the ceiling outright, which
+	// skips the warn window (it only covers thresh < est <= budget) and
+	// goes straight to the shallow emergency prune.
+	al := &AgentLoop{MaxTokenBudget: 1}
+	ch := make(chan StreamEvent, 16)
+	msgs := []history.Message{
+		{Role: "tool", Content: strings.Repeat("y", softTrimThreshold+100), ToolCallID: "call-1"},
+		{Role: "user", Content: "hi"},
+	}
+
+	al.enforceTokenBudget(context.Background(), "s", ch, 0, msgs)
+
+	traces := collectTrace(ch)
+	if len(traces) != 1 {
+		t.Fatalf("want 1 trace, got %d", len(traces))
+	}
+	if traces[0].Policy != ContextPolicyBudgetEmergency {
+		t.Fatalf("policy = %q, want %q", traces[0].Policy, ContextPolicyBudgetEmergency)
+	}
+	if len(traces[0].Changes) != 1 || traces[0].Changes[0].Reason != ContextChangeSoftTrim {
+		t.Fatalf("changes = %+v, want a single soft-trim", traces[0].Changes)
+	}
+}
+
+func TestEnforceTokenBudget_WarnPolicyTagsArgsTruncation(t *testing.T) {
+	// Size the budget so the estimate lands inside the warn window:
+	// thresh (0.85 * 1200 = 1020) < est (~1100) <= 1200.
+	al := &AgentLoop{MaxTokenBudget: 1200}
+	ch := make(chan StreamEvent, 16)
+	msgs := []history.Message{
+		{Role: "tool", Content: strings.Repeat("y", 4400), ToolCallID: "call-1", CorrelationID: "corr-1"},
+		{Role: "user", Content: "hi"},
+	}
+
+	al.enforceTokenBudget(context.Background(), "s", ch, 0, msgs)
+
+	traces := collectTrace(ch)
+	if len(traces) != 1 {
+		t.Fatalf("want 1 trace, got %d", len(traces))
+	}
+	if traces[0].Policy != ContextPolicyBudgetWarn {
+		t.Fatalf("policy = %q, want %q", traces[0].Policy, ContextPolicyBudgetWarn)
+	}
+	if len(traces[0].Changes) != 1 {
+		t.Fatalf("changes = %+v, want exactly one", traces[0].Changes)
+	}
+	c := traces[0].Changes[0]
+	if c.Reason != ContextChangeArgsTruncated || c.CorrelationID != "corr-1" {
+		t.Fatalf("change = %+v, want args-truncated on corr-1", c)
+	}
+}
+
+func TestPruneContextMessages_OutlierReasonRecorded(t *testing.T) {
+	msgs := []history.Message{
+		{Role: "tool", Content: strings.Repeat("z", outlierTrimThreshold+10)},
+		{Role: "user", Content: "hi"},
+	}
+
+	_, changes := pruneContextMessages(msgs, 1)
+
+	if len(changes) != 1 {
+		t.Fatalf("want 1 change, got %d", len(changes))
+	}
+	if changes[0].Reason != ContextChangeOutlierDiscarded {
+		t.Fatalf("reason = %q, want %q", changes[0].Reason, ContextChangeOutlierDiscarded)
+	}
+}
+
+func TestPruneContextMessages_NoChangeAllocatesNoTrace(t *testing.T) {
+	msgs := []history.Message{
+		{Role: "tool", Content: "small"},
+		{Role: "user", Content: "hi"},
+	}
+
+	if _, changes := pruneContextMessages(msgs, 1); changes != nil {
+		t.Fatalf("want nil trace when nothing changed, got %+v", changes)
+	}
+}