fix(engine): pin evaluator EffortOff, inherit summarizer session effort

[andybons]

Summary

• The goal-loop evaluator (runEvaluator, engine/goal.go) and the compaction summarizer (runCompactionSummary, engine/compact.go) never set provider.Request.Effort, so both ran at EffortUnset regardless of the session's effort level.
• On the openaicompat route, unset omits reasoning_effort, and several gateway models (kimi-k3, deepseek-v4) reason by default — the evaluator's MaxTokens 256 budget could be mostly spent on reasoning before it ever emits MET/NOT MET.
• Decision (tech lead): the evaluator is a classifier, so it pins message.EffortOff unconditionally. The summarizer is a writing task, so it inherits the session's current effort via Session.Effort().

Test plan

• TestPursueGoalEvaluatorRequestPinsEffortOff (engine/goal_test.go) drives PursueGoal end to end with a fake provider, asserts the evaluator request's Effort == EffortOff even when the session's own effort is set to EffortHigh.
• TestCompactSummaryRequestInheritsSessionEffort (engine/compact_test.go) sets SetEffort(EffortMedium), drives Session.Compact end to end, asserts the summarizer request's Effort == EffortMedium.
• Both tests red-verified independently: reverting each fix's one-line Effort: field (in isolation) fails only its own test, for the expected reason.
• go build ./..., go vet ./..., go test -race ./... all green.
• AGENTS.md updated in the same commit ("Goal loop" and "Reasoning effort" sections).

Fixes #124

[claude]

📦 Previous Review (superseded by newer review)

Claude finished @andybons's task in 4m 6s —— View job

---

Review complete

• Read AGENTS.md priorities and changed files
• Analyze evaluator/summarizer effort changes for correctness
• Check tests against AGENTS.md testing rules
• Post review findings

The evaluator change (runEvaluator pins EffortOff, goal.go) is correct and well-motivated — a classifier must not spend its 256-token budget on reasoning. The locking, both request-build sites, and the two red-verified tests all check out.

One substantive issue on the summarizer side, filed inline on engine/compact.go:333:

Having runCompactionSummary inherit s.Effort() enables extended thinking on the anthropic adapter for any non-off effort. That interacts badly with the deliberate compactionMaxTokens = 1024 cap: transcode.go:196 bumps max_tokens to budget + thinkingCompletionMargin, so at EffortHigh the summary's completion budget goes 1024 → 4096 and each compaction also pays up to ~16k thinking tokens. The 1024 cap only survives at EffortOff/EffortUnset. A secondary, lower-confidence risk (the documented — but so far only theoretical — ENABLE-direction tool_use reject) rides the same code path; details inline.

Suggested direction: clamp the summarizer effort so the thinking budget can't exceed the summary cap, rather than forwarding s.Effort() unmodified. If forwarding is intentional despite the 4x cap override and thinking-token cost, it's worth a sentence in the AGENTS.md "Reasoning effort" note documenting that tradeoff.

Nothing else flagged — the diff is small, tests follow the TDD/red-verify rules, and AGENTS.md was updated in the same commit.
· andybons/internal-effort

[claude]

📦 Previous Review (superseded by newer review)

Claude finished @andybons's task in 3m 37s —— View job

---

Review: PR #125 — pin evaluator EffortOff, inherit summarizer session effort

• Read AGENTS.md priorities and PR diff
• Review engine/goal.go evaluator effort pin
• Review engine/compact.go summarizer effort inheritance
• Review tests (TDD, red-verify, no sleeps)
• Assess prior review finding (thinking-budget vs summary cap)
• Post review

---

Verdict: LGTM. The two-way split is correct, well-tested, and the one real tradeoff is explicitly documented per AGENTS.md scope discipline. No blocking issues.

What the change does right

• Evaluator pins EffortOff unconditionally (goal.go:2267). Correct: it's a one-line classifier on a 256-token budget, and letting a reason-by-default gateway model burn that budget on reasoning was the actual bug (engine: goal evaluator and compaction summarizer requests never set Effort #124). The EffortOff (not EffortUnset) choice matters — since a7c5cce, only the literal "off" suppresses reasoning on openaicompat; unset would leave the gateway default in force. Good.
• Summarizer inherits s.Effort() (compact.go:342), read fresh from live state, forwarding only (EffortUnset stays unset). Matches streamTurn's own per-request read — consistent with the "read effort fresh every request" model.
• Tests drive production entry points — PursueGoal → runEvaluator and Session.Compact → runCompactionSummary, not the internal functions directly, satisfying the AGENTS.md "verification drives the production entry point" rule. Both pass under -race; the worker-vs-evaluator split keys on len(rq.Tools) (evaluator is tool-less), which is robust. No sleeps.
• AGENTS.md updated in the same commit, and the "Goal loop" / "Reasoning effort" sections now correctly document the openai-Responses residual (the evaluator can still reason on that route).

On the prior review finding (thinking-budget vs the 1024 summary cap)

The earlier review is right that a non-off effort lets the anthropic adapter bump MaxTokens above compactionMaxTokens (thinking budget + margin) and adds a thinking bill per compaction. That's a real cost/behavior change. But it's a deliberate quality tradeoff the author documented, and I'd frame the two sub-concerns as bounded, not blocking (details in the inline comment on compact.go:342):

1. ENABLE-direction 400 is not new. streamTurn already transcodes the full history — old tool_use turns made at unset effort included — with thinking on at every set-effort turn. Compaction sends a subset of that same shape, so if it rejected, the main loop would already be wedging. Consistent with the live-tolerated STATUS probe at provider/anthropic/transcode.go:187.
2. Silent truncation predates this PR. runCompactionSummary never checks StopReason before folding (compact.go:373-379), so even at EffortOff/1024 a max_tokens-truncated summary already folds. This change only raises the odds on openaicompat (no cap floor). The right home for the fix is the filed follow-up — add a StopReason == StopMaxTokens guard, which closes both the old and new cases at once.

Minor / optional

• The residual comment block in compact.go (16 lines) is thorough but long for a forwarding assignment. Fine as-is given the subtlety, but the AGENTS.md entry already carries the canonical version — the code comment could point there instead of restating it. Non-blocking.

Nothing here needs to change before merge. The follow-up issue for the summarizer cap/StopReason guard is the correct disposition.
· andybons/internal-effort

[claude]

Claude finished @andybons's task in 4m 38s —— View job

---

Review complete

• Read AGENTS.md priorities and changed files
• Analyze evaluator/summarizer effort changes for correctness
• Check tests against AGENTS.md testing rules
• Post review findings

The production change is two lines: runEvaluator pins message.EffortOff (engine/goal.go), and runCompactionSummary inherits s.Effort() (engine/compact.go:355). AGENTS.md and both tests were updated in the same commit, red-verified per the testing rules.

Evaluator change — correct and safe. The evaluator is a classifier with a 256-token budget; pinning EffortOff stops a reason-by-default gateway model from spending that budget before it emits MET/NOT MET. Its transcript is rendered text with no tool_use parts, so EffortOff cannot trip the thinking-block reject. No lock issue — runEvaluator runs with s.mu released.

Summarizer change — carries two documented residuals, both deferred; neither blocks merge. I re-derived both against the code and confirmed the prior rounds' conclusions:

1. ENABLE-direction reject (engine: compaction summarizer can newly hit the anthropic enable-direction thinking hazard #127) — not a new exposure. streamTurn (engine/engine.go:1275) already sends s.Effort() over the full history — including old tool_use turns emitted at unset effort — on every set-effort request. runCompactionSummary sends a strict subset of that same history. If thinking-over-a-thinkingless-tool_use actually 400'd, the main loop would already wedge before compaction ran. The STATUS note at provider/anthropic/transcode.go:187 records this live-tolerated (2026-08-11). So this is a documented-limitation surface, not a regression — consistent with the second prior review round.

2. Raised output cap / truncation (engine: compaction summarizer's inherited effort can bypass compactionMaxTokens, risking silent truncation #126) — real, and the truncation gap is genuine. A non-off effort bumps the anthropic/openai effective cap well above compactionMaxTokens (1024), and openaicompat applies no floor. I confirmed runCompactionSummary (engine/compact.go:389-402) only guards the stream-cut truncation path (io.EOF identity check, TestCompactTruncatedSummaryNeverFolds) — it has no StopReason == StopMaxTokens guard, so a summary that hits max_tokens cleanly still folds truncated. This predates the PR but the change makes it more likely on openaicompat. The right follow-up (tracked in engine: compaction summarizer's inherited effort can bypass compactionMaxTokens, risking silent truncation #126) is that StopReason guard, which closes both the pre-existing and new cases at once.

Testing. The two new tests assert the Effort field is forwarded through a fake provider — plumbing, not transcoder behavior. That is appropriate for what this PR changes (which site sends which effort), and matches how the sibling effort tests are written. The residual behaviors (#126/#127) live in the transcoders and are correctly filed as separate issues rather than guarded here, per AGENTS.md scope discipline.

I'm not filing new inline comments — the existing threads on engine/compact.go:355 already cover both residuals thoroughly, and re-posting would be duplicate noise. My verification agrees with the most recent round: the residuals are documented, live-tolerated where it matters, and do not block merge.

One optional nit if you want to tighten the note: the AGENTS.md "Reasoning effort" residual for #126 states openaicompat "applies no cap floor at all" — true — but readers may miss that the silent part is the absent StopReason guard in runCompactionSummary itself, not just the adapter. A half-sentence pointing at that function would make the follow-up self-locating.
· andybons/internal-effort