feat(tools): attribute expired deadlines to the layer that set them - #148
Merged
Conversation
ErrTimeout plus DeadlineCause/TimedOut let a layer tell its own expired budget from an enclosing deadline or a cancelled turn, which ctx.Err() reports identically. Applied to WithTimeout, code_interpreter, sql_agent and generate_video, which all previously misreported a cancel as their own timeout.
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.
What & why
Every deadline in the tool path used a plain
context.WithTimeout, so an expired context said onlycontext.DeadlineExceeded— with no way to tell whose deadline it was. A layer that sets its own budget cannot report one honestly without that distinction, and two sites were getting it wrong in a user-visible way:code_interpretercomputedtimedOut := runCtx.Err() == context.DeadlineExceeded. An enclosing deadline or a cancelled turn sets that identically, so pressing Stop told the model "your code timed out after 30s" when the code was fine.generate_videogated its poll-timeout message the same way, reporting "timed out after 5m" for a turn the caller cancelled.sql_agentsurfaced a query-budget timeout to the model as the bare driver string (context deadline exceeded), which is unactionable — the model retries the identical query rather than narrowing it.Changes
tools.ErrTimeout— one classification sentinel, deliberately distinct fromcontext.DeadlineExceededbecause any enclosing context also produces that.tools.DeadlineCause(what, d)— builds the cause to hand tocontext.WithTimeoutCause, naming the budget. Satisfieserrors.Is(err, ErrTimeout)while carrying the duration in its message, so a reporting site can name the budget without holding it.tools.TimedOut(ctx)—errors.Is(context.Cause(ctx), ErrTimeout). Answers "was it my deadline?"; an enclosing context that expires or is cancelled first leaves its own cause in place.tools.WithTimeoutmiddleware wraps a failure caused by its own deadline so the error names the elapsed budget. An outer deadline or cancellation is passed through untouched.code_interpreter,sql_agent(both query and mutation paths, via a newqueryErrText), andgenerate_video.One sentinel plus a per-site cause, rather than one exported sentinel per layer: callers get a single
errors.Ishandle and the message still says which budget elapsed. The sentinel lives inpkg/toolsbecause it is a tool-layer concept — putting it inpkg/agentwould makepkg/toolsdepend on it.Not swept, on purpose:
pkg/agent/hitl_gate.goalready hasErrHITLTimedOutplus a typedHITLTimedOutEvent, so it has this attribution;pkg/eval/runner.gois a test harness with its own reporting;generate_title.gohas no model-facing timeout report to misattribute.Non-breaking: the wrapped error still unwraps to whatever the tool returned, so existing
errors.Is(err, context.DeadlineExceeded)matches keep working — asserted by a test.Testing
TestTimedOut_OwnDeadline/_OuterDeadlineIsNotOurs/_CancellationIsNotATimeout/_LiveContext— the four-way distinction, including the case that motivates the whole change: an outer deadline whereinner.Err()isDeadlineExceededbutTimedOutcorrectly returns false.TestWithTimeout_AttributesOwnDeadline— matchesErrTimeout, still matchescontext.DeadlineExceeded, and names the budget.TestWithTimeout_LeavesOuterCancellationUnattributed— a cancelled parent gets noErrTimeoutattribution.TestDeadlineCause_NamesTheBudget,TestWithTimeout_SuccessIsUntouched.gofmt -l .empty,make lint0 issues,make build,make test,make test-raceall clean.Checklist
gofmt -l .prints nothingmake lintis cleanmake buildpassesmake test(andmake test-race) passesCHANGELOG.mdupdated if this PR is cut as a release tagpkg/llm/,pkg/history/, orpkg/telemetry/