Skip to content

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

Open
jasmine-ab-tea wants to merge 2 commits into
mainfrom
yixint/pr271-followup-children-ergonomics
Open

jasmine-ab-tea wants to merge 2 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

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

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!

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>
@jasmine-ab-tea
jasmine-ab-tea requested a review from wrisa October 7, 2026 23:17
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.

1 participant