Skip to content

feat(sdk): add StepRecorder for incremental trace/session Step trees - #287

Merged
jasmine-ab-tea merged 6 commits into
mainfrom
yixint/pr271-followup-children-ergonomics
Oct 13, 2026
Merged

jasmine-ab-tea merged 6 commits into
mainfrom
yixint/pr271-followup-children-ergonomics

Conversation

@jasmine-ab-tea

Copy link
Copy Markdown
Collaborator

Summary

PR #271 added Step.children and a recursive Galileo record factory, but the only way to populate children today is to build the whole Step tree by hand before calling evaluate_controls(children=...). The ace-demo (banking_multilevel_streamlit_app.py / banking_multilevel_cases.py) shows the cost: every leaf function hand-builds a side-channel record dict next to its real return value, parents manually assemble spans/traces lists, a bespoke recursive converter (build_agent_control_step) renames keys into Step, and the caller then unpacks that Step back into evaluate_controls(...) kwargs, which immediately repacks it into a new Step.

This PR lets callers build the tree directly, incrementally, as children execute - no intermediate dict, no converter, no unpack/repack.

  • Extract evaluate_step(step, ...) out of evaluate_controls (sdks/python/src/agent_control/evaluation.py): the shared Step -> evaluation tail (server check, target resolution, client, check_evaluation_with_local). evaluate_controls now builds its Step and delegates to it. Also widen children to accept Sequence[Step | Mapping[str, Any]] (dicts are coerced via pydantic).
  • Add agent_control.record_step() / StepRecorder (new step_recorder.py): a mutable builder since Step is frozen.
    • .child(type, name, **kwargs) nests a child recorder, attached on __exit__.
    • .add(step) attaches an already-built Step or 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 - no StepExecution-style wrapper needed. A failing call is still recorded (with the error in context) before re-raising.
    • .build() recursively constructs the frozen Step, with children=[] for a trace/session with no recorded children (matching the "allow empty trace and session children" fix).
    • .evaluate(stage=..., agent_name=...) builds and evaluates via evaluate_step(), defaulting agent_name from init().
  • Export evaluate_step, record_step, StepRecorder from agent_control.
  • README: add a "Building trace/session steps" before/after section.

Test plan

  • New tests/test_step_recorder.py (11 tests): nested trace/session trees, .call/.acall payload parity with the decorator, async .acall, exception recording + re-raise, empty-trace/session children=[], non-trace/session types stay children=None, .add() with Step/dict, .evaluate() forwarding + agent_name default resolution + missing-agent error.
  • Extended tests/test_evaluation.py: evaluate_controls now routes through evaluate_step (mocked), dict children coerce to Step.
  • New tests/test_step_recorder_galileo_integration.py (skips if galileo extras aren't installed): builds a trace and a session via StepRecorder and feeds them into agent_control_evaluator_galileo's record_from_step, asserting the same shapes the demo's validate_record checks (3-span trace of llm/tool/retriever; 2-trace session with >=2 spans each).
  • ruff check / mypy clean on all changed/new files.
  • Full SDK suite: 710 passed, 13 skipped (no local server), 1 pre-existing failure unrelated to this change (test_integration_health.py::test_client_context_manager, requires a live server; fails identically on main).

🤖 Generated with Claude Code

@codecov

codecov Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Comment thread sdks/python/src/agent_control/step_recorder.py Outdated
Comment thread sdks/python/tests/test_step_recorder_galileo_integration.py Outdated
Comment thread sdks/python/tests/test_step_recorder_galileo_integration.py Outdated
Comment thread sdks/python/README.md Outdated
jasmine-ab-tea and others added 5 commits October 12, 2026 12:06
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
jasmine-ab-tea force-pushed the yixint/pr271-followup-children-ergonomics branch from 93dfa4d to 31da9f2 Compare October 12, 2026 19:08
Comment thread sdks/python/src/agent_control/step_recorder.py
Comment thread sdks/python/src/agent_control/step_recorder.py Outdated
@josjeon
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>
@jasmine-ab-tea
jasmine-ab-tea merged commit 4f342ed into main Oct 13, 2026
6 checks passed
@jasmine-ab-tea
jasmine-ab-tea deleted the yixint/pr271-followup-children-ergonomics branch October 13, 2026 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants