feat(evals): Code Mode A/B workflow benchmark#1122
Closed
MQ37 wants to merge 14 commits into
Closed
Conversation
Add paired standard/code-mode test cases, per-test tool and Actor-target policy enforcement, agent/judge token usage tracking, bounded tool-result evidence for the judge, and append-only results storage. - evals/shared/types.ts: pairId/arm fields, disallowedTools, allowed/disallowedCallActorTargets, agentInstructions - evals/workflows/mcp_client.ts: enforce tool and call-actor target policy, reject violations instead of just hiding schemas - evals/workflows/conversation_executor.ts: append agentInstructions to system prompt, track cached/reasoning tokens per turn - evals/workflows/llm_client.ts: capture cached_tokens/reasoning_tokens from provider usage - evals/workflows/workflow_judge.ts: give judge bounded tool-result evidence (was tool calls + final response only) - evals/workflows/results_writer.ts: append-only attempts instead of overwriting by agentModel:judgeModel:testId key; record judge usage, tool-call trace, and policy violations separately from agent usage - evals/workflows/run_workflow_evals.ts: --results-path flag; force FAIL on any tool-policy violation - evals/workflows/code_mode_test_cases.json: 3 paired cases (Instagram scrape, Google Maps filter+sort, Maps-to-website chain)
Only the standard arm still forbids apify/code-runtime as a call-actor target. The code-mode arm no longer restricts call-actor targets at all: the agent may call any Actor directly for discovery (resolving which Actor to use, reading a schema, grabbing an ID/URL for a later step) while still being expected to run apify/code-runtime for the actual scrape/processing/chaining. Real usage is often mixed (direct calls for cheap lookups, Code Mode for the bulk work), and the previous hard allowlist forced FAIL on every code-mode attempt whenever the agent made any direct call at all - which was most of the time. - code_mode_test_cases.json: drop allowedCallActorTargets from the three runtime cases; reworded agentInstructions/reference to describe the expected discovery-vs-processing split - output_formatter.ts: add getCallActorTargets() (distinct Actor IDs targeted through call-actor) - results_writer.ts: record usedCodeRuntime (whether apify/code-runtime was called at least once) - informational only, does not gate pass/fail - README.md: document the relaxed policy and the new field
Reference now explicitly instructs the judge to check the tool-call trace for a call-actor invocation targeting apify/code-runtime and FAIL the code-mode arm if it was never called, regardless of whether the final answer looks correct. Direct Actor calls remain fine for discovery, just not as a substitute for running apify/code-runtime. This replaces the hard call-actor-target block removed earlier with a judge-side check, since the judge already sees every tool call and its arguments in the transcript.
The code-mode arm only had the generic call-actor route plus prose instructions telling it to use apify/code-runtime by name - no dedicated, schema-carrying tool for it. tools_loader.ts resolves any non-category/non-internal-tool entry in `tools` as an Actor name and fetches it as its own MCP tool (apify--code-runtime, with the Actor's real input schema attached), which is far more discoverable to the model than expecting it to intuit an Actor ID from free text. Add "apify/code-runtime" to the tools array for all three code-mode (runtime) test cases; standard-arm tools are unchanged. Since the agent can now reach code-runtime through either the generic call-actor tool or this dedicated one, usedCodeRuntime detection in results_writer.ts now checks both paths, with test coverage for both.
Reword agentInstructions for the three code-mode (runtime) test cases
from soft ('Prefer apify/code-runtime...') to imperative: the agent
MUST call apify/code-runtime at least once, told explicitly it is for
processing data efficiently inside the sandbox so only small filtered
results reach the model instead of raw datasets, and warned that
skipping it gets the response rejected regardless of accuracy.
Direct Actor calls remain fine for lightweight discovery only.
Transcripts show the agent calling apify/code-runtime directly, on first attempt, without ever fetching its README — it only guessed the API from unrelated Apify SDK knowledge. Add an explicit fetch-actor-details step to agentInstructions before the MUST-call directive.
results.json's toolCallTrace deliberately excludes tool result bodies to stay compact. For manually reviewing what actually happened in a test case (e.g. debugging why the code-mode arm fails), --traces <path> writes one JSON array with every turn's full tool-call arguments, full tool results, and full LLM final response, untruncated. Separate, disposable artifact — not meant to be committed or diffed like results.json.
Tests were running 15+ minutes with no upper bound — a stuck agent retry loop (e.g. the code-mode dangling-promise bug) could block the whole run indefinitely. --test-timeout bounds one test's whole conversation + judging; on timeout it's recorded as FAIL with a clear reason and the run moves to the next test. Extracted raceWithTimeout/TestTimeoutError into their own module rather than the CLI entrypoint file, since run_workflow_evals.ts runs main() (including process.exit) as an import side effect and can't be safely imported from a test.
cleanup()'s SIGKILL fallback was dead code: StdioClientTransport.close() clears its own _process reference synchronously the instant it's called, then spends up to ~4s internally (wait, SIGTERM, wait, SIGKILL) actually terminating the child. Our own race timeout (2s) fires before that finishes, so by the time the fallback checked this.transport for a process to kill, it was already undefined -- a no-op. Capture the child reference before calling close(), so the fallback can act on it regardless of the SDK's own timing. Without this, a child left running past cleanup() -- e.g. one still finishing a tool call that --test-timeout abandoned -- could write its response to a stdout pipe whose far end is already gone, throwing an unhandled EPIPE in that orphaned process after the harness itself had already exited.
A test that hit --test-timeout previously lost its entire in-progress conversation -- the timeout catch block built a fresh empty one. Traces for maps-filter-runtime and both maps-websites cases in the last run had zero turns, exactly when they were most needed for diagnosis. executeConversation() now accepts an optional shared turns array and pushes into it as turns happen, instead of a locally-scoped one. The timeout/error path reads the same array, so whatever completed before the cutoff is preserved in results.json and --traces output.
…call Traces had no timing below the whole-test durationMs -- couldn't tell whether a slow test was one nested Actor call eating 40s or the model itself thinking that long. ConversationTurn now carries llmStartedAt/ llmDurationMs, McpToolResult carries startedAt/durationMs. Both flow through to --traces (full ConversationHistory) and results.json's compact toolCallTrace.
resolveReadmeContent() always preferred the platform's auto-generated readmeSummary over the real README when present, with no way to override it. For apify/code-runtime, whose README documents an exact API contract (method names/shapes), the summary omits that section entirely -- confirmed via a live eval trace where the agent fetched the README, got the summary, and still guessed the wrong dataset API. fetchActorDetails() now reads an optional fullReadmeOnly boolean off buildInfo.actorDefinition (not part of apify-client's ActorDefinition type, read defensively) and resolveReadmeContent() returns the full readme whenever it's set, regardless of whether a summary exists. Unverified: whether an unrecognized top-level actor.json key survives the platform's own build validation into this API response -- the apify-client ActorDefinition type only lists recognized fields, and readmeSummary is documented as platform/store-generated, not sourced from actor.json at all. Needs a live check once actor-code-runtime sets fullReadmeOnly: true and is rebuilt.
A single run per test/arm can't distinguish a real difference from ordinary run-to-run noise -- confirmed directly this session (same test, same fixed code, --concurrency 1 passed / --concurrency 6 timed out; identical Google Maps query returned 20 places once and 5 the next, same code both times). --repeat <n> runs each filtered test case N times, through the same --concurrency limiter as any other job (repeats interleave with everything else; run with --concurrency 1 yourself for a low-noise comparison -- this flag doesn't invent a separate sequential mode). Aggregation, not averaging blindly: - passRate (strict PASS/N) and completionRate ((PASS+judge-FAIL)/N) are reported separately -- a wrong-but-complete answer and a timeout are different failure modes. - timedOut is counted separately from other errors (new EvaluationResult/ TestResultRecord field, set from TestTimeoutError rather than string-matching the error message). - duration/token/tool-byte stats (median+mean) are computed only over completed attempts, so a --test-timeout cap doesn't drag the 'typical duration' toward the cap. --traces and --results-path still record every attempt individually, tagged with attemptIndex/totalAttempts, never averaged -- the aggregated view is a separate, additional summary, not a replacement. Exit code: 0 regardless of individual outcomes when --repeat > 1 (the point is characterizing flakiness, not gating CI on one run); default (--repeat 1) keeps the existing strict all-must-pass exit code.
Replace the self-declared actor.json fullReadmeOnly flag with a hardcoded Actor-ID check (CODE_RUNTIME_ACTOR_ID). Any Actor could set fullReadmeOnly: true in its own actor.json to opt itself out of the auto-generated summary; hardcoding the one Actor ID that legitimately needs its full README (its docs are an exact API contract) removes that gaming vector. resolveReadmeContent() now takes actorInfo.id instead of a fullReadmeOnly bool, matching what callers already have in hand (ActorDetailsResult already carries actorInfo).
Contributor
Author
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
2 participants
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
Adds Code Mode A/B benchmark support to the workflow evals harness: per-turn traces, richer timeout/repeat controls, and an eval-only hardcode that forces
apify/code-runtime(Ewe0VxlktEE7SChSI) to always return its full README instead of the auto-generated summary.Why
Comparing "Code Mode" vs direct-tool-call arms needs multi-run evidence, per-turn traces, and bounded per-test timeouts to be trustworthy. The README hardcode replaces an earlier self-declared
fullReadmeOnlyactor.json flag — any Actor could set that flag to dodge summarization, so the bypass is now pinned to the one Actor ID whose docs are an exact API contract.Testing
Existing + new unit tests updated (
utils.actor_details,evals.*) —pnpm test.