Repository navigation
feat(trajectory): score how an agent got there, not just what it said (P3a) - #51
Conversation
Deploying eval-harness with
|
| Latest commit: |
b91950e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://06e24c0f.eval-harness.pages.dev |
| Branch Preview URL: | https://feat-p3a-trajectory.eval-harness.pages.dev |
Deploying laravel-patent-box-tracker with
|
| Latest commit: |
b91950e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2527d246.laravel-patent-box-tracker.pages.dev |
| Branch Preview URL: | https://feat-p3a-trajectory.laravel-patent-box-tracker.pages.dev |
There was a problem hiding this comment.
Pull request overview
Introduces SDK-agnostic “trajectory” capture and scoring so evals can judge how an agent reached an answer (tool usage, argument matching, order, step budget, and approvals), not just the final text. This extends the harness with offline-capable trajectory metrics while keeping the existing Metric contract intact via a TrajectoryMetric subtype and engine-side dispatch.
Changes:
- Add
Trajectory/ToolCallDTOs plus a container singletonTrajectoryRecorder, and thread optional trajectory data into per-sample reports and JSON rendering. - Add seven trajectory metrics (
tool-called,tool-not-called,tool-called-with,tool-call-order,steps-below,no-pending-approvals,approval-gated) and register aliases in the resolver. - Enable offline trajectory scoring via saved outputs carrying a
trajectoryblock; add extensive unit + engine end-to-end tests and documentation.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/Trajectory/TrajectoryTest.php | Unit tests for DTO parsing/matching semantics and recorder behavior. |
| tests/Unit/Metrics/TrajectoryMetricsTest.php | Unit tests covering all new trajectory metrics and missing-trajectory behavior. |
| tests/Unit/EvalEngineTrajectoryTest.php | End-to-end tests: recording, report JSON inclusion, offline scoring, repetition instability. |
| src/Trajectory/TrajectoryRecorder.php | New singleton recorder for per-sample/per-repetition trajectories. |
| src/Trajectory/Trajectory.php | New trajectory DTO with matching helpers and array serialization. |
| src/Trajectory/ToolCall.php | New tool-call DTO with subset/numeric-recursive argument matching and serialization. |
| src/Reports/SampleResult.php | Adds optional ?Trajectory to per-sample results. |
| src/Reports/JsonReportRenderer.php | Conditionally emits samples[].trajectory in rendered JSON when present. |
| src/Outputs/SavedOutputsLoader.php | Allows trajectory blocks to be carried through from saved outputs formats. |
| src/Outputs/SavedOutputs.php | Parses and stores trajectories keyed by sample id for offline scoring. |
| src/Metrics/Trajectory/TrajectoryMetric.php | Introduces TrajectoryMetric interface extending Metric. |
| src/Metrics/Trajectory/AbstractTrajectoryMetric.php | Shared trajectory-metric plumbing + missing-trajectory MetricException behavior. |
| src/Metrics/Trajectory/ToolCalledMetric.php | Metric: expected tools were called (with partial credit). |
| src/Metrics/Trajectory/ToolNotCalledMetric.php | Metric: forbidden tools were avoided (with partial credit). |
| src/Metrics/Trajectory/ToolCalledWithMetric.php | Metric: tool calls match declared argument subsets. |
| src/Metrics/Trajectory/ToolCallOrderMetric.php | Metric: declared tool order occurred as subsequence. |
| src/Metrics/Trajectory/StepsBelowMetric.php | Metric: step-count budget enforcement. |
| src/Metrics/Trajectory/NoPendingApprovalsMetric.php | Metric: ensures run completed with no pending approvals. |
| src/Metrics/Trajectory/ApprovalGatedMetric.php | Metric: required approval-gated actions are present (partial credit). |
| src/Metrics/MetricResolver.php | Registers string aliases for the new trajectory metrics. |
| src/EvalHarnessServiceProvider.php | Binds TrajectoryRecorder as a container singleton. |
| src/EvalEngine.php | Dispatches TrajectoryMetric::scoreTrajectory(), attaches trajectories to results, ingests saved-output trajectories. |
| README.md | Adds trajectory metrics overview to the README feature list. |
| docs/PROGRESS.md | Logs the P3a implementation milestone and rationale. |
| docs-site/docs/guides/agent-trajectories.md | New documentation page describing DTO, recording, metrics, and offline scoring. |
| docs-site/docmd.config.json | Adds the new guide to docs navigation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| $dataset = $this->getDataset($datasetName); | ||
| $outputs = $this->savedOutputsForDataset($datasetName, $dataset, $actualOutputs); | ||
| $this->recordSavedTrajectories($actualOutputs); | ||
| $actualOutputForSample = static fn (DatasetSample $sample, int $_index): string => $outputs[self::sampleIdKey($sample->id)]; |
| * Drop everything recorded so far. | ||
| * | ||
| * Called by the engine at the start of a run: the recorder is a singleton, | ||
| * and a second run in the same process must not score against the first | ||
| * run's tool calls. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: affae2c56f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (is_array($entry['trajectory'] ?? null)) { | ||
| $normalized['trajectory'] = $entry['trajectory']; |
There was a problem hiding this comment.
When saved outputs are loaded from the documented JSON format, json_decode(..., false) represents the nested trajectory mapping as stdClass; YAML mappings use the same object representation because of PARSE_OBJECT_FOR_MAP. Only the outer entry is converted, so this is_array() check silently discards every file-loaded trajectory, causing --outputs trajectory metrics to report that none was recorded. Convert the nested mapping, including its nested tool calls and arguments, before constructing SavedOutputs.
Useful? React with 👍 / 👎.
| public function record(string $sampleId, Trajectory $trajectory, ?int $repetition = null): void | ||
| { | ||
| $this->trajectories[self::key($sampleId, $repetition)] = $trajectory; |
There was a problem hiding this comment.
With a real asynchronous queue or Horizon worker, this records into the worker process's container singleton, while EvaluateSampleJob::handle() writes only the string actualOutput to BatchResultStore; the process assembling the report therefore sees a different, empty recorder. The sync queue hides this boundary, but production lazy-parallel runs will make every trajectory metric fail as missing and omit trajectories from reports unless the trajectory is carried through the shared batch result.
AGENTS.md reference: AGENTS.md:L19-L20
Useful? React with 👍 / 👎.
| $this->app->singleton(TrajectoryRecorder::class, static function (): TrajectoryRecorder { | ||
| return new TrajectoryRecorder; | ||
| }); |
There was a problem hiding this comment.
When the same Laravel container executes multiple evaluations, such as an eval set or a long-lived host process, this singleton retains trajectories keyed by sample ID indefinitely. Although TrajectoryRecorder::flush() says the engine calls it at the start of a run, there is no such call, so a later run whose runner or saved outputs omit a trajectory can reuse an earlier run's trajectory for the same ID and silently receive a passing score. Clear the recorder once at the start of each top-level runBatch() or scoreOutputs() invocation before recording new data.
Useful? React with 👍 / 👎.
… (P3a) Every metric in this package scored a string. That is the right unit for a RAG pipeline and the wrong one for an agent: the final answer is the one part a broken agent can still get right by accident. An agent that replies "your order ships Tuesday" without ever calling the order lookup has guessed — it will be correct often enough to look healthy, every text metric here scores that guess 1.0, and the day it is wrong it will be wrong with total confidence. Adds a Trajectory DTO (tool calls with arguments, order, steps, finish reason, approvals), a recorder the system under test writes into, and seven metrics that read it: tool-called, tool-not-called, tool-called-with, tool-call-order, steps-below, no-pending-approvals, approval-gated. Three decisions: - The DTO is SDK-agnostic. The obvious implementation accepts whatever the agent SDK returns, which is what the closest comparable tool does, and it means those assertions exist only inside that one SDK. This one is populated from laravel/ai, a custom orchestrator, MCP, laravel-flow saga steps, or a JSON file recorded last week — the adapter changes per runtime, the assertions do not. The laravel/ai adapter lives in a satellite package so this one stays free of an SDK dependency, which is design rationale #1 of the harness. - The Metric contract is untouched. TrajectoryMetric extends Metric and the engine hands a trajectory only to that family; widening the contract for one family would have changed every metric in the package and every one a host has written. - A missing trajectory raises a captured MetricException rather than scoring 0 or 1. Scoring 0 blames the agent for the harness's missing wiring; scoring 1 lets a whole dataset go green because nobody plugged the recorder in. Arguments match as a subset and order as a subsequence, so adding a trace id or an extra tool does not fail an eval that was about something else. Saved outputs carry an optional trajectory block, which makes the whole family usable offline with no agent runtime at all. approval-gated answers a question no text metric can: did the agent ask before it acted? Where an agent can spend money or change a record, an unapproved action is a compliance finding, not a UX detail. Tests: 1018 (was 981), 2792 assertions. PHPStan level 6 clean. Co-Authored-By: Claude <[email protected]>
affae2c to
d5643db
Compare
Third item from the 2026-08-24 competitor analysis, stacked on #50. Review #50 first — this branch contains it.
The gap
Every metric in this package scores a string. That is the right unit for a RAG pipeline and the wrong one for an agent: the final answer is the one part a broken agent can still get right by accident.
An agent that replies "your order ships Tuesday" without ever calling the order lookup has guessed. It will be correct often enough to look healthy, every text metric here scores that guess
1.0, and the day it is wrong it will be wrong with total confidence.What lands
A
TrajectoryDTO (tool calls with arguments/result/error, steps, finish reason, approvals), a recorder the system under test writes into, and seven metrics:tool-calledtool-not-calledtool-called-withtool-call-ordersteps-belowno-pending-approvalsapproval-gatedThree decisions worth reviewing
1. The DTO is SDK-agnostic. The obvious implementation accepts whatever the agent SDK returns — that is what the closest comparable tool does, and it means those assertions exist only inside that one SDK. This is a plain DTO, populated from
laravel/ai, a custom orchestrator, MCP,laravel-flowsaga steps, or a JSON file recorded last week. The adapter changes per runtime; the assertions do not. Thelaravel/aiadapter goes in the satellite package so this one stays free of an SDK dependency — design rationale #1 of the harness.2. The
Metriccontract is untouched.TrajectoryMetric extends Metric, and the engine callsscoreTrajectory()only for that family. Widening the base contract would have changed every metric in the package and every one a host has written, to serve one family.3. A missing trajectory is a captured failure, not a 0 and not a 1. Scoring
0blames the agent for the harness's missing wiring; scoring1lets a whole dataset go green because nobody plugged the recorder in. It raises aMetricExceptionwith the fix in the message.Matching rules, and why they are loose
{id: 7}is satisfied by a call that also passes a trace id, a locale and a retry counter. Requiring equality would break every assertion the first time a wrapper added a field. Numeric values compare across7and"7", because a JSON round-trip turns one into the other and a test should fail on behaviour, not on transport.[check_stock, charge_card]says stock came first; a currency conversion in between has not violated it. Requiring an exact sequence would make every new tool a failing eval.0.0throws away the only signal that says which direction the agent moved between two runs.Usable offline
Saved outputs carry an optional
trajectoryblock, so the whole family works with no agent runtime at all — record what an agent did once, and every later run scores it deterministically and for free. A bare list of names ("tool_calls": ["lookup_order", "answer"]) is accepted, because it is the cheapest thing a host can record.The compliance angle
approval-gatedanswers a question no text metric can. Where an agent can spend money, change a record or send something to a customer, an unapproved action is a finding, not a UX detail — under the EU AI Act's human-oversight expectations it is the difference between an assistant and an unsupervised actor. Action ids come from whatever the host records: alaravel-flowsaga step, alaravel-iam-agentsconsent id, or a custom string. Pairs directly with the adversarial lane: a red-team row that tries to talk the agent into acting without approval is exactly what this scores.Gates
composer validate --strictvendor/bin/phpunitvendor/bin/phpstan analyse(level 6)vendor/bin/pint --testNew tests:
TrajectoryTest,TrajectoryMetricsTest,EvalEngineTrajectoryTest(end-to-end incl. the lucky-guess case, offline scoring from saved outputs, and a row whose second repetition forgot the tool). Docs:docs-site/docs/guides/agent-trajectories.md+ nav, README bullet, PROGRESS.Generated by Claude Code