Promote cancellation trace-context capture to a CancellationTraceCapture capability - #848
Open
mpfaffenberger wants to merge 2 commits into
Open
Promote cancellation trace-context capture to a CancellationTraceCapture capability#848mpfaffenberger wants to merge 2 commits into
mpfaffenberger wants to merge 2 commits into
Conversation
The Logfire cancellation-linking feature (#827) captured the live trace context by wrapping the run's event_stream_handler in an eager _observed_event_stream_handler closure inside _runtime._do_run. Promote the capture to a first-class pydantic-ai capability, CancellationTraceCapture, on the wrap_run_event_stream seam. - code_puppy/agents/_cancellation_trace.py: static entry capability with no seam override; for_run resolves the turn's CancellationTraceObservation (group_id + streaming gate) from a ContextVar installed by _do_run and returns a per-run _ActiveCancellationTraceCapture that captures the trace context once per streamed node event stream and passes events through unchanged. Disabled/absent observations resolve to the seam-less static entry, so non-streamed runs are not forced into streaming mode (gate parity). - The observation install is a plain ContextVar.set with no reset: it runs inside the turn's agent task, whose context dies with the task; nested run_with_mcp turns install their own in their own task. - clear_agent_context (turn-end finally) and emit_cancellation (await- site except* handlers via on_agent_run_cancel) stay eager: both run outside the run boundary where no capability seam fires. - Span-context parity pinned under real OTel instrumentation: the capture observes the identical span context the event stream handler is invoked in. 21 contract tests in tests/agents/test_cancellation_trace_capability.py.
Owner
Author
|
Review pass 1 (code-puppy-clone-1): APPROVE, zero blocking. Findings: 2 NON-BLOCKING (capture-timing divergence and guest-wrapper caveat — both already documented in the PR body), 1 NIT (docstring overstated how small the invocation-to-first-pull window must be for arbitrary handlers). NIT applied: docs now state first-pull semantics explicitly, noting the production StreamingTextDetector consumes immediately so the moments coincide in practice. |
Owner
Author
|
Review pass 2 (code-puppy-clone-1): APPROVE, zero remaining findings. Both NON-BLOCKING dispositions (documented divergences, no code change) explicitly confirmed. Not merging per instructions — awaiting human eyeballs. |
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
Eighteenth in the capability-conversion series (#828-#836, #838-#842, #844, #845, #847). Promotes the cancellation trace-context capture -- the newest feature on main (#827, "Link cancellation event to agent trace context") -- from an eager
_observed_event_stream_handlerclosure in_runtime._do_runto a first-class pydantic-ai capability,CancellationTraceCapture, on thewrap_run_event_streamseam.This is a disjoint second claim of the seam (#835
StreamRenderingclaims it for rendering; this claims it for trace capture -- same precedent as #847's second claim ofafter_run). Both are pass-through observers, so order between them is inert.Why this feature maps to this seam
emit_cancellationlinks the "Agent run cancelled" Logfire warning to the run's live trace by attaching the context captured during the most recent streamed model request. That capture is per-request, run-path work that main smuggled into theevent_stream_handlerwrapper -- the exact surfacewrap_run_event_streamformalizes.The other two thirds of the feature deliberately stay eager:
clear_agent_contextruns in the turn task'sfinally(must cover cancel/crash exits; a cancelled run never reachesafter_run).emit_cancellationfires from the await-siteexcept*handlers viaon_agent_run_cancel(cancellation can land between runs of a turn, where no seam is active; and round-15 scouting proved theCancelledErroratwrap_runcarries no usable snapshot).Design
code_puppy/agents/_cancellation_trace.py-- the static entryCancellationTraceCapturedoes NOT override the seam. Itsfor_runresolves a per-turnCancellationTraceObservation(group_id+ streaming gate) from a ContextVar installed by_do_run, returning a per-run_ActiveCancellationTraceCaptureonly when enabled. Disabled/absent observations returnself, so pydantic-ai's "overriding this seam auto-enables streaming" behavior never fires for gated-off runs -- byte-identical to the eager wrapper existing only whenget_enable_streaming()was true (the Deliver stream rendering via a StreamRendering capability #835 inert-resolution trick).capture_agent_context(group_id)once per wrapped node stream -- the same cadence as the old per-handler-invocation capture (pinned by a counting test), in the identical OTel span context (pinned under a real SDKTracerProvider: capture and handler record the same span id).set, no reset --_do_runexecutes inside the turn's agent task, whose context dies with the task. Nestedrun_with_mcpturns run in their own task and install their own observation (shadowing pinned).Noneinstalls shadow (the feat: InterruptedSubagentNotes capability (capability series, round 12) #840 lesson).captureseam defaulting to late-boundcode_puppy.observability.capture_agent_context, so existing module patches keep intercepting (feat(agents): SubagentSessionPersistence capability on the wrap_run seam #842's call-time-resolution pattern).Bounded divergences (documented)
pydantic_agent.run()call issued inside a turn's task (bypassingrun_with_mcp) would inherit the turn's observation and capture under its group id, where the eager wrapper (bound to the kwarg) did not. No such caller exists in the codebase; production installs are turn-task-scoped.Tests
21 contract tests in
tests/agents/test_cancellation_trace_capability.py: for_run resolution, no-forced-streaming (request-onlyFunctionModelproves inertness; stream-only proves streaming -- the #835 asymmetric-model trick), capture cadence == handler invocations, span-context parity under real OTel instrumentation, gate-off silence, event pass-through equality, sequential-run accumulation, ContextVar shadowing/task-death custody, late-binding patches, builder wiring via the publicapplyvisitor, production-shapedrun_with_mcpdrives (gate on/off, group id ==agent_run_startsession id), and source pins for the retired wrapper + eager emit/clear custody.Full suite: 7618 passed, 0 failed (28 skipped, 1 xpassed).
Merge notes
Shares the main
capabilities=[...]block with the seventeen open siblings -- whichever lands last eats a trivial rebase. Conflicts with #835's_do_runchanges are mechanical (both touch the handler setup; both preservecapture_agent_contextsemantics).