From a3b09218aa150795499b8b69224211ef8fb9eb89 Mon Sep 17 00:00:00 2001 From: anandh8x Date: Sat, 18 Jul 2026 00:21:22 +0530 Subject: [PATCH] Add deterministic completion policy Extract completion decisions from the agent loop into a feature-gated typed policy with complete, incomplete, and uncertain outcomes. Preserve bounded plan-stall nudges and allow at most one semantic acceptance check for self-correcting runs. Tested: make build Tested: make test Tested: go fmt ./... Tested: go vet ./... Tested: make lint Tested: govulncheck ./... Note: repository-wide pinned golangci-lint reports 36 pre-existing unrelated findings; internal/agent/... reports 0 issues. --- internal/agent/completion_policy.go | 86 ++++++++++++++++++++++++ internal/agent/completion_policy_test.go | 64 ++++++++++++++++++ internal/agent/loop.go | 79 ++++++---------------- internal/agent/types.go | 7 +- 4 files changed, 175 insertions(+), 61 deletions(-) create mode 100644 internal/agent/completion_policy.go create mode 100644 internal/agent/completion_policy_test.go diff --git a/internal/agent/completion_policy.go b/internal/agent/completion_policy.go new file mode 100644 index 000000000..71b0ddeec --- /dev/null +++ b/internal/agent/completion_policy.go @@ -0,0 +1,86 @@ +package agent + +// CompletionDecision is the deterministic outcome of evaluating a text-only +// assistant turn. Uncertain means the loop needs one bounded follow-up action +// before it can finalize the run. +type CompletionDecision string + +const ( + CompletionUncertain CompletionDecision = "uncertain" + CompletionComplete CompletionDecision = "complete" + CompletionIncomplete CompletionDecision = "incomplete" +) + +type completionAction string + +const ( + completionActionNone completionAction = "" + completionActionContinue completionAction = "continue" + completionActionSemanticCheck completionAction = "semantic_check" +) + +type completionEvaluation struct { + Decision CompletionDecision + Action completionAction + Reason string +} + +// completionPolicy owns the small amount of state needed to keep uncertain +// decisions bounded. requireSemanticCheck is set only for run profiles that +// already opted into self-correction; ordinary completion remains entirely +// local and adds no model call. +type completionPolicy struct { + continueNudges int + requireSemanticCheck bool + semanticCheckRequested bool +} + +func newCompletionPolicy(requireSemanticCheck bool) *completionPolicy { + return &completionPolicy{requireSemanticCheck: requireSemanticCheck} +} + +func (policy *completionPolicy) evaluate(text string, planPending bool) completionEvaluation { + // A direct admission is strong local evidence and takes precedence over all + // weaker signals, avoiding both continuation nudges and semantic checks. + if reason := selfReportedIncompletion(text); reason != "" { + return completionEvaluation{Decision: CompletionIncomplete, Reason: reason} + } + + // A continuation cue is strong unfinished evidence; a pending plan is only + // weak evidence because bookkeeping can be stale. Both get a bounded chance + // to continue, but only a persisted cue is ultimately incomplete. + cue := endsWithContinuationCue(text) + if cue || planPending { + if policy.continueNudges < maxContinueNudges { + policy.continueNudges++ + reason := "your message ended mid-step" + if !cue { + reason = "pending plan items remain — finish them, or mark them complete with update_plan if you are done" + } + return completionEvaluation{ + Decision: CompletionUncertain, + Action: completionActionContinue, + Reason: reason, + } + } + if cue { + return completionEvaluation{ + Decision: CompletionIncomplete, + Reason: "your message ended mid-step", + } + } + } + + // Profiles with self-correction enabled require one task-grounded semantic + // check. It is requested at most once per run; the next locally complete turn + // is accepted without another model call. + if policy.requireSemanticCheck && !policy.semanticCheckRequested { + policy.semanticCheckRequested = true + return completionEvaluation{ + Decision: CompletionUncertain, + Action: completionActionSemanticCheck, + } + } + + return completionEvaluation{Decision: CompletionComplete, Action: completionActionNone} +} diff --git a/internal/agent/completion_policy_test.go b/internal/agent/completion_policy_test.go new file mode 100644 index 000000000..342203ebb --- /dev/null +++ b/internal/agent/completion_policy_test.go @@ -0,0 +1,64 @@ +package agent + +import "testing" + +func TestCompletionPolicyLocalEvidenceDecidesWithoutSemanticCheck(t *testing.T) { + policy := newCompletionPolicy(false) + + complete := policy.evaluate("Done. All required checks pass.", false) + if complete.Decision != CompletionComplete { + t.Fatalf("confident completion decision = %q, want %q", complete.Decision, CompletionComplete) + } + + incomplete := policy.evaluate("I couldn't verify the result, so this is my best guess.", false) + if incomplete.Decision != CompletionIncomplete { + t.Fatalf("admitted failure decision = %q, want %q", incomplete.Decision, CompletionIncomplete) + } +} + +func TestCompletionPolicyPreservesBoundedPlanStallProtection(t *testing.T) { + policy := newCompletionPolicy(false) + for attempt := 0; attempt < maxContinueNudges; attempt++ { + got := policy.evaluate("Let me inspect the remaining configuration:", true) + if got.Decision != CompletionUncertain || got.Action != completionActionContinue { + t.Fatalf("attempt %d = (%q, %q), want uncertain continue", attempt+1, got.Decision, got.Action) + } + } + + got := policy.evaluate("Let me inspect the remaining configuration:", true) + if got.Decision != CompletionIncomplete { + t.Fatalf("decision after nudge budget = %q, want %q", got.Decision, CompletionIncomplete) + } +} + +func TestCompletionPolicyTreatsPendingPlanAsWeakEvidence(t *testing.T) { + policy := newCompletionPolicy(false) + for attempt := 0; attempt < maxContinueNudges; attempt++ { + got := policy.evaluate("All set.", true) + if got.Decision != CompletionUncertain || got.Action != completionActionContinue { + t.Fatalf("attempt %d = (%q, %q), want uncertain continue", attempt+1, got.Decision, got.Action) + } + } + + got := policy.evaluate("All set.", true) + if got.Decision != CompletionComplete { + t.Fatalf("stale plan decision after nudge budget = %q, want %q", got.Decision, CompletionComplete) + } +} + +func TestCompletionPolicyAllowsExactlyOneRequiredSemanticCheck(t *testing.T) { + policy := newCompletionPolicy(true) + + first := policy.evaluate("Implemented and tested.", false) + if first.Decision != CompletionUncertain || first.Action != completionActionSemanticCheck { + t.Fatalf("first decision = (%q, %q), want uncertain semantic check", first.Decision, first.Action) + } + + second := policy.evaluate("PASS. The result meets the task criterion.", false) + if second.Decision != CompletionComplete { + t.Fatalf("post-check decision = %q, want %q", second.Decision, CompletionComplete) + } + if second.Action == completionActionSemanticCheck { + t.Fatal("semantic check was requested more than once") + } +} diff --git a/internal/agent/loop.go b/internal/agent/loop.go index 61fed9711..867f9a8f9 100644 --- a/internal/agent/loop.go +++ b/internal/agent/loop.go @@ -170,13 +170,10 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options) // loaded tool's full schema; it lives only for the run (v1 within-run scope). loaded := map[string]bool{} - // continueNudges counts how many times the headless completion gate - // (Options.RequireCompletionSignal) has re-prompted a no-tool-call turn that - // stopped with work still unfinished. Bounded by maxContinueNudges. - continueNudges := 0 - // acceptanceRequested records that the one-time task-grounded acceptance check - // has already been demanded this run, so it fires at most once. - acceptanceRequested := false + // The feature-gated completion policy keeps local completion decisions and its + // bounded follow-up state out of the central loop. Self-correcting profiles + // opt into the one permitted task-grounded semantic check. + completionPolicy := newCompletionPolicy(options.SelfCorrect != nil) // toolDefCache memoizes each tool's rendered JSON-schema definition across // turns (a tool's advertised schema is stable for the run), so partitionTools @@ -521,68 +518,32 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options) // model's final answer ONLY when the work is actually done. Default off // (RequireCompletionSignal), so interactive runs stay byte-identical. if options.RequireCompletionSignal { - // (1) Self-report downgrade (strongest, unambiguous): the model's own - // final message admits it guessed / could not meet the objective. Checked - // FIRST so an admitted-impossible task is downgraded immediately (no wasted - // continue-nudges) and reports the accurate reason. - if reason := selfReportedIncompletion(collected.Text); reason != "" { + evaluation := completionPolicy.evaluate(collected.Text, guards.pendingPlanItems()) + switch evaluation.Decision { + case CompletionIncomplete: result.Incomplete = true - result.IncompleteReason = reason + result.IncompleteReason = evaluation.Reason result.FinalAnswer = collected.Text result.Messages = copyMessages(messages) return result, nil - } - - // (2) The model stopped without a tool call while work may be unfinished: - // - a continuation cue ("…Let me check the config:") is an unambiguous - // mid-step stop; - // - pending update_plan items are a WEAK, ambiguous signal (the model may - // have finished without re-marking the last step). - // Nudge to continue (bounded). After the budget: a persisted continuation - // cue finalizes INCOMPLETE; pending-plan WITHOUT a cue does NOT (that would - // false-fail a completed run with stale bookkeeping) — fall through to the - // acceptance check / success. - cue := endsWithContinuationCue(collected.Text) - planPending := guards.pendingPlanItems() - if cue || planPending { - if continueNudges < maxContinueNudges { - continueNudges++ + case CompletionUncertain: + switch evaluation.Action { + case completionActionContinue: options.Trace.Counter(trace.CounterCompletionNudges, 1) - reason := "your message ended mid-step" - if !cue { - reason = "pending plan items remain — finish them, or mark them complete with update_plan if you are done" - } messages = append(messages, zeroruntime.Message{ Role: zeroruntime.MessageRoleUser, - Content: continueNudge(reason), + Content: continueNudge(evaluation.Reason), + }) + case completionActionSemanticCheck: + options.Trace.Counter(trace.CounterAcceptanceChecks, 1) + messages = append(messages, zeroruntime.Message{ + Role: zeroruntime.MessageRoleUser, + Content: acceptanceVerificationNudge(), }) - continue - } - if cue { - result.Incomplete = true - result.IncompleteReason = "your message ended mid-step" - result.FinalAnswer = collected.Text - result.Messages = copyMessages(messages) - return result, nil } - // pending-plan only, budget spent: trust the model's completion claim - // over stale plan bookkeeping; fall through. - } - - // (3) Task-grounded acceptance: before accepting a "done" turn as success, - // require ONE acceptance check grounded in the task's stated criterion - // (only when self-correct is on). Rejects "well-formed == correct", - // "existing-tests-pass == objective met", and "result == baseline" false - // successes. Bounded to a single pass; a genuine post-check completion - // (no admission, no cue) then finalizes as success on the next turn. - if options.SelfCorrect != nil && !acceptanceRequested { - acceptanceRequested = true - options.Trace.Counter(trace.CounterAcceptanceChecks, 1) - messages = append(messages, zeroruntime.Message{ - Role: zeroruntime.MessageRoleUser, - Content: acceptanceVerificationNudge(), - }) continue + case CompletionComplete: + // Local evidence is sufficient; proceed to final diagnostics. } } // Finalization diagnostics gate: edits from this run may still have diff --git a/internal/agent/types.go b/internal/agent/types.go index 21e06e07a..30747387e 100644 --- a/internal/agent/types.go +++ b/internal/agent/types.go @@ -315,8 +315,11 @@ type Options struct { // continuation cue ("…Let me check the config:"). The loop then nudges the // model to continue instead, bounded by maxContinueNudges (and still by // MaxTurns and the run deadline); if the model keeps stalling, the run - // finalizes as INCOMPLETE (Result.Incomplete) rather than success. Default - // false leaves the loop byte-identical, so the interactive TUI is unaffected. + // finalizes as INCOMPLETE (Result.Incomplete) rather than success. When the + // run's profile also enables SelfCorrect, an otherwise-complete turn gets one + // task-grounded semantic check before success; profiles without SelfCorrect + // add no model call. Default false leaves the loop byte-identical, so the + // interactive TUI is unaffected. RequireCompletionSignal bool runPermissions *permissionRunState