Repository navigation
feat(sdk): add StepRecorder for incremental trace/session Step trees - #287
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
3 tasks
Manually populating Step.children for a trace/session control today means hand-building a side-channel record per leaf call and converting it into Step objects afterward (see ace-demo's build_agent_control_step). Add agent_control.record_step()/StepRecorder: an incremental builder that constructs the Step tree directly as children execute, reusing @control()'s capture logic (_create_evaluation_payload) for .call()/ .acall(). Extract evaluate_step() out of evaluate_controls so both the manual API and the new recorder share the same Step -> evaluation path. Co-Authored-By: Claude Code <noreply@anthropic.com>
Codecov flagged 6 uncovered lines in step_recorder.py: acall()'s exception-recording path (mirrored from call() but never exercised) and build()'s context/tools/ground_truth passthrough branches. Add the two missing tests; step_recorder.py is now at 100% line coverage. Co-Authored-By: Claude Code <noreply@anthropic.com>
…dary - record_step()/.child() take an opt-in container_types param instead of hardcoding "trace"/"session" as the step types that serialize empty children as [] - step types are caller-defined strings, not a core vocabulary. - Move the galileo-dependent StepRecorder integration test out of the SDK into evaluators/contrib/galileo, where its dependency belongs. - Replace the README's trace/session example with a generic workflow/lookup one so the OSS SDK docs don't lean on Galileo's hierarchy vocabulary. Co-Authored-By: Claude Code <noreply@anthropic.com>
The moved StepRecorder integration test needs agent-control-sdk, which only lives in the galileo evaluator's dev extra. The test target never requested that extra, so CI installed a venv without it and failed to import agent_control. Co-Authored-By: Claude Code <noreply@anthropic.com>
call()/acall() run func(*args, **kwargs) and forward every unrecognized keyword straight to func, so a plain `context=` parameter on call() would collide with (and silently steal) a same-named kwarg on func - exactly the case for functions like run_banking_model(..., context=...). step_context is merged into the recorded child's Step.context instead, without touching what gets forwarded to func. On failure, the error key added to context takes precedence over the same key in step_context. Co-Authored-By: Claude Code <noreply@anthropic.com>
jasmine-ab-tea
force-pushed
the
yixint/pr271-followup-children-ergonomics
branch
from
October 12, 2026 19:08
93dfa4d to
31da9f2
Compare
josjeon
self-requested a review
October 12, 2026 23:31
- build() defaults a tool step's input to {} when omitted, matching
evaluate_controls()'s existing behavior and avoiding Step's "tool
steps require object input" validation error.
- __exit__ now receives the real exception value and merges
{"error": repr(exc)} into context, consistent with call()/acall()'s
failure path, so a child() block that raises is no longer
indistinguishable from one that simply has no output.
Addresses PR #287 review comments.
Co-Authored-By: Claude Code <noreply@anthropic.com>
namrataghadi-galileo
approved these changes
Oct 13, 2026
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
PR #271 added
Step.childrenand a recursive Galileo record factory, but the only way to populatechildrentoday is to build the wholeSteptree by hand before callingevaluate_controls(children=...). The ace-demo (banking_multilevel_streamlit_app.py/banking_multilevel_cases.py) shows the cost: every leaf function hand-builds a side-channelrecorddict next to its real return value, parents manually assemblespans/traceslists, a bespoke recursive converter (build_agent_control_step) renames keys intoStep, and the caller then unpacks thatStepback intoevaluate_controls(...)kwargs, which immediately repacks it into a newStep.This PR lets callers build the tree directly, incrementally, as children execute - no intermediate dict, no converter, no unpack/repack.
evaluate_step(step, ...)out ofevaluate_controls(sdks/python/src/agent_control/evaluation.py): the shared Step -> evaluation tail (server check, target resolution, client,check_evaluation_with_local).evaluate_controlsnow builds itsStepand delegates to it. Also widenchildrento acceptSequence[Step | Mapping[str, Any]](dicts are coerced via pydantic).agent_control.record_step()/StepRecorder(newstep_recorder.py): a mutable builder sinceStepis frozen..child(type, name, **kwargs)nests a child recorder, attached on__exit__..add(step)attaches an already-builtStepor dict..call(func, ...)/await .acall(...)run the function and record it as a child using the same capture logic@control()uses (_create_evaluation_payload), returning the real result - noStepExecution-style wrapper needed. A failing call is still recorded (with the error incontext) before re-raising..build()recursively constructs the frozenStep, withchildren=[]for a trace/session with no recorded children (matching the "allow empty trace and session children" fix)..evaluate(stage=..., agent_name=...)builds and evaluates viaevaluate_step(), defaultingagent_namefrominit().evaluate_step,record_step,StepRecorderfromagent_control.Test plan
tests/test_step_recorder.py(11 tests): nested trace/session trees,.call/.acallpayload parity with the decorator, async.acall, exception recording + re-raise, empty-trace/sessionchildren=[], non-trace/session types staychildren=None,.add()withStep/dict,.evaluate()forwarding + agent_name default resolution + missing-agent error.tests/test_evaluation.py:evaluate_controlsnow routes throughevaluate_step(mocked), dict children coerce toStep.tests/test_step_recorder_galileo_integration.py(skips if galileo extras aren't installed): builds a trace and a session viaStepRecorderand feeds them intoagent_control_evaluator_galileo'srecord_from_step, asserting the same shapes the demo'svalidate_recordchecks (3-span trace of llm/tool/retriever; 2-trace session with >=2 spans each).ruff check/mypyclean on all changed/new files.test_integration_health.py::test_client_context_manager, requires a live server; fails identically onmain).🤖 Generated with Claude Code