Skip to content

feat: record agent steps from stream() - #27

Closed
TheGreatAxios wants to merge 3 commits into
cl-11498-add-the-first-prebuilt-scorersfrom
cl-11504-record-agent-trajectory-in-fromintxagent
Closed

TheGreatAxios wants to merge 3 commits into
cl-11498-add-the-first-prebuilt-scorersfrom
cl-11504-record-agent-trajectory-in-fromintxagent

Conversation

@TheGreatAxios

Copy link
Copy Markdown
Contributor

Summary

  • Record model and tool steps from agent.stream() while send runs. send alone still returns no steps.
  • ranSteps([]) fails when the step list was never observed. An observed model-only run still counts as no tools.
  • A reply event that arrives on a later turn is kept, and a case aborted while it waits for the agent lock does not call send.

Verification

  • bun run check exited 0 (68 tests on the amended commit).
  • Review of 10ef921 found no defects. bun test e2e/agent-trajectory.e2e.test.ts exited 0 (8 passed).

Stacked on #26.

Fixes CL-11504

An empty step list is not an observed empty tool path, so ranSteps([]) fails unless a watched run actually used no tools.
@linear-code

linear-code Bot commented Oct 26, 2026

Copy link
Copy Markdown

CL-11504

@corbits-triage

corbits-triage Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Triage: Pass

No changes needed from you. A maintainer will review this pull request.

Stacked on #26: only this pull request's own changes were checked.

@TheGreatAxios TheGreatAxios left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review · Request changes

Records agent steps from stream(). An aborted turn’s tools land on the next case, and a policy-denied tool is invisible, so ranSteps([]) treats that deny as an observed empty tool path.

Findings

  • src/runs.ts:244 — sendAgent closes the stream and releases withAgentLock as soon as the in-flight send promise settles. @intx/agent 0.5 rejects that send on abort while the reactor keeps emitting. The next case subscribes and records the late tool.start. A stub with that timing produced steps ["tool:search"] on the following reply-only "lookup" case, which should have been ["llm:stub"]. Because saw is already true, the lookup reply is not synthesized. After a timeout, the next case inherits the aborted case’s tools and loses its own model step, so ranSteps(["search"]) can pass and ranSteps([]) can fail on a run that used no tools. bun test tmp/critique-tests/trajectory-holes.test.ts exited 1: expected ["llm:stub"], received ["tool:search"]. That probe was deleted.

  • src/runs.ts:92 — a policy deny emits tool.done and never tool.start (@intx/inference 0.5 beforeTool block). The tool name is only on inference.tool_call.end and the assistant tool_call block, and both are ignored. The recorded steps contain no tool, so ranSteps([]) passes on a turn that did call search, and ranSteps(["search"]) fails. The same probe expected tool names ["search"] and received [].

@TheGreatAxios

Copy link
Copy Markdown
Contributor Author

Review · Follow-up

Commit a5fd4a1 names a policy-denied tool, but abort isolation still does not hold.

Denied tools are fixed. inference.tool_call.end or an assistant tool_call block records search, and a tool.done with no name is not recorded. ranSteps([]) still fails on an unwatched empty step list.

  • src/runs.ts:309 — agent.send(...).then(onOk, onErr) always fulfills, so collect's rejection handler never sets sendFailed and holdForTurn stays false. The agent lock is released on the idle macrotask while the aborted turn can still emit. A probe where the timeout aborts "slow", then 30ms later emits tool.start {name:"search"} and connector.reply while the next "lookup" send is still in flight for 80ms, recorded [{name:"search", kind:"tool"}] on lookup instead of the model step. ranSteps([]) failed and ranSteps(["search"]) passed on that case. The new e2e passes only because lookup resolves before the 20ms emit, so the event is buffered and dropped rather than held.

@TheGreatAxios

Copy link
Copy Markdown
Contributor Author

Review · Follow-up

Commit 8b2795f is clean.

collect watches the raw agent.send promise, so an abort rejection sets sendFailed and the subscription stays open until connector.reply. On the 30ms/80ms timing, lookup records its model step, ranSteps([]) passes, and ranSteps(["search"]) fails. A deleted probe showed lookup's send starts only after the late tool emit, so the test cannot pass without the hold. Policy denies still record search, and a nameless tool.done is still omitted.

bun test e2e/agent-trajectory.e2e.test.ts exited 0 (10 pass).

@TheGreatAxios

Copy link
Copy Markdown
Contributor Author

Superseded by the 0.2.0 release: #32

Do not merge this pull request separately. The release branch cherry-picks the feature stack (except the 0.1.3 bump on #22) and sets @corbits/evals to 0.2.0 with @corbits/system-one ^0.4.0.

@TheGreatAxios

Copy link
Copy Markdown
Contributor Author

Superseded by the 0.2.0 release: #32

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant