feat(agent): context trace, partial-success terminal, and scored self-critique - #140
Merged
Conversation
Pruning ran before every LLM call and reported nothing on the default unbudgeted path. ContextTraceEvent names each rewritten message, its reason, and the token delta — emitted only when something changed.
A tool whose artifact landed but whose derived state failed had to lie with Done or discard good work with Error. tools.Degradation raises it; DegradedEvent precedes the terminal, and the model is told not to retry the half that succeeded.
…e last Reflect accepted any textually different revision, so a critique pass could silently make the answer worse and the earlier text was gone. With a Scorer set, a revision must beat the incumbent to be adopted; nil Scorer keeps today's behavior.
A cache hit replayed the partial-success note to a later turn's model while short-circuiting before recordDegradation, so the host saw a clean turn for work that never ran.
A retry reset or stream error drops the spec entry before it is awaited, losing the report for a tool that really executed and half-succeeded — the double-write the feature exists to prevent.
float64 with omitempty erased a legitimate 0 from the wire and left it indistinguishable from an unscored round in Go — 0 is valid on a 0-100 rubric and optimal for a negated-latency scorer.
DegradedEvent trails the terminal frame on the cap/error sweep path, not precedes it. Duplicate ContextRef.Index is unreachable under current thresholds, and per-ref token estimates exclude tool-call arguments.
The *Degradation field made go vet reject %s/%q on a Result, failing builds for callers that never use the degradation feature. Also check the pointer before the ctx walk in recordDegradation.
OnToolResult can recover an error into a success, so filing at execution time dropped the report for a recovered speculative call. Consumed results now file once after the hook; orphaned speculations file at discard, where the hook never runs.
enforceTokenBudget concatenates the truncation and prune passes, so Index ascends within a group but not across the slice.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three additive capabilities on the agent loop, each closing a gap that a host cannot fill from outside the package.
ContextTraceEvent— context pruning is no longer invisible. The loop prunes before every LLM call, and the most common path (noMaxTokenBudgetset) reported nothing at all. When an adopter asked "why did the agent forget the schema from turn 2?", no artifact existed to answer from. The event names each rewritten message with a closed-enum reason (soft-trim/outlier-discarded/args-truncated), a policy (default/budget-warn/budget-emergency), and before/after token estimates.ContextRef.Indexlines up withSessions.History, andCorrelationIDmatches the one onToolCallEvent, so a trace joins back to both the stored transcript and the call that produced it.Emitted only when a prune actually changed something — a turn whose context fits emits nothing, and the token sweeps only run when there is something to report.
DegradedEvent— a terminal for work that half-landed. The vocabulary was binary:Done, or an error. There was no way to say "the artifact was written, the derived bookkeeping failed." ReportingDonehides a real inconsistency; reporting an error invites a retry that duplicates the expensive write. A tool raises one by returning a normal (non-error)tools.ResultwithDegraded: &tools.Degradation{Reason, Artifacts, Unreliable}.The event precedes the terminal rather than replacing it, so
DoneEventstill fires and consumers that ignore it are unaffected. The loop also appends a partial-success note to the text the model reads, telling it not to redo the half that landed — without that, the host knows and the model doesn't.Scorer— self-critique keeps the best round, not the last.Reflectaccepted any textually-different revision, so round 3 won even when round 1 was better, and round 1 was unrecoverable. The only guard was string equality, which a cosmetic rewrite defeats. With aScorerset, the original answer is round 0 and each revision must strictly beat the incumbent; a rejected revision is rolled back so the next round critiques the incumbent rather than the discarded draft. Ties keep the incumbent, and a revision the scorer cannot rank is discarded — an unranked candidate cannot be shown to be an improvement.Scoreris the shared interface planned for best-of-K, landed here with its first (sequential) consumer.Changed (breaking)
EventVisitorgainsVisitContextTraceandVisitDegraded. External implementers must add both. This is the documented purpose of the interface — a new payload type forces every visitor to handle it.ReflectedEvent.Scoreis*float64, notfloat64. Zero is a legitimate score (a 0–100 rubric can return 0; a negated-latency scorer treats 0 as optimal), andfloat64+omitemptyerased it from the wire and made it indistinguishable from "unscored" in Go.tools.ResultgainsDegraded *Degradationand aString()method. The method is load-bearing, not cosmetic: without it the new pointer field makesgo vetreject%s/%qon aResultin downstream modules, breaking builds for callers who never touch the feature. Verified against a separate consumer module. Note that%von aResultnow prints its text rather than the struct.Defaults and cost
All three are zero-cost when unused.
Scoreris nil by default, preserving the previous last-wins reflect behavior exactly.Degradedis nil on every existing tool. The context trace is silent unless a prune fires. Measured overhead on a Run with none of them active: 47 → 50 allocs/op (the per-Run degradation accumulator, which cannot be gated since whether a tool will degrade is unknowable up front).Review
Two
code-reviewerpasses. The first covered the three feature commits and found four real defects, each verified independently before fixing:ReflectedEvent.Scoreerasing a legitimate zero;DegradedEventalways precedes the terminal, which is false on the deferred-sweep path (it trailsErrorEvent/LimitExhaustedEventthere).The second pass covered those fixes and found a regression introduced by one of them:
OnToolResultcan recover an error into a success, so filing a degradation at execution time dropped the report for a recovered speculative call. Consumed results now file exactly once after the hook chain; orphaned speculations file at discard time, where the hook never runs. The two paths are mutually exclusive and cannot double-count.Tests
25 new tests. Each load-bearing assertion was mutation-checked — the fix reverted in a scratch copy to confirm the test actually fails. That caught one hollow test: the reflect rollback assertion originally passed with the rollback deleted, because it only checked the final answer. It now asserts which answer each critique round was handed.
gofmt -l .empty ·make vetclean ·make lint0 issues ·make build·make test·make test-raceall green.Follow-ups (not in this PR)
Reflect/Scorerare not configurable from YAML — Go-side fields only.Degradationand the demo does not render either new event, so both are discoverable only via godoc.BestOfK, the parallel consumer ofScorer, is still open.CHANGELOG.mdentry lands with the version tag.