From 63878dd823981a8bd90ab66d6a62ba452a22f207 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jesus=20Nu=C3=B1ez?= Date: Thu, 6 Aug 2026 23:56:29 -0400 Subject: [PATCH 1/9] harden: explicit tool-call ids with id-preferred pairing Adds an optional toolCallId to Step. ToolCallEvaluator now pairs a tool_result to its tool_call by id when present (authoritative: an unmatched id is an orphan, no name fallback), falling back to the previous name+FIFO only when the result has no id. Backward compatible: id-less fixtures keep the old behavior. The Python emitter generates a toolCallId per call (call-N) and copies it onto the result via the ToolCall handle, so every emitted trace pairs precisely even with repeated calls to the same tool. Schema doc updated. Adds id-pairing tests (same-name calls, out-of-order results, unknown id) on both sides. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../langgraph-reference/docs/trace-schema.md | 21 +++++--- .../langgraph-reference/tests/test_emitter.py | 22 +++++++- .../trazo_emitter/models.py | 3 ++ .../trazo_emitter/recorder.py | 27 ++++++++-- evaluator/toolcalls.go | 35 ++++++++---- evaluator/toolcalls_test.go | 54 +++++++++++++++++++ trajectory/steps.go | 1 + 7 files changed, 142 insertions(+), 21 deletions(-) diff --git a/agents/langgraph-reference/docs/trace-schema.md b/agents/langgraph-reference/docs/trace-schema.md index 8a0e577..75c344c 100644 --- a/agents/langgraph-reference/docs/trace-schema.md +++ b/agents/langgraph-reference/docs/trace-schema.md @@ -49,7 +49,8 @@ Each element of `steps` deserializes into `trajectory.Step`. | `type` | string (StepType) | always | One of the four types below. Unknown values fail validation. | | `timestamp` | RFC3339 time | always | Must be non-zero for every step. | | `llm` | string | `type == llm_call` | Required for `llm_call`; omit otherwise. | -| `tool` | string | `tool_call`/`tool_result` | Required for both; also the pairing key (see below). | +| `tool` | string | `tool_call`/`tool_result` | Required for both; the fallback pairing key (see below). | +| `toolCallId` | string | optional | Correlates a `tool_result` with its `tool_call`. Preferred over the tool name when present. | | `node` | string | `type == node_transition` | Required for `node_transition`. | | `input` | raw JSON | optional | Any JSON value (object, array, string, number). Payload is opaque to the core. | | `output` | raw JSON | optional | Any JSON value. In fixtures it appears as an object, an array, and a bare string. | @@ -101,13 +102,17 @@ evaluation and reported separately. `ToolCallEvaluator` (`evaluator/toolcalls.go`) walks the steps in order and pairs tool calls with results. Key facts the Python emitter must honor: -- Pairing key is the `tool` **name**, not a correlation id. There is no - `callId` field in the schema. -- Matching is order-sensitive and FIFO per name: a `tool_result` matches the - earliest still-pending `tool_call` with the same `tool`. So emit a call before - its result, and do not interleave two pending calls of the *same* tool name if - you need them paired deterministically. -- A `tool_result` with no pending call of that name -> `neutral` finding +- Preferred key is `toolCallId`. When a `tool_result` carries a `toolCallId`, it + matches the pending `tool_call` with the same id, regardless of order or name. + The id is authoritative: a `toolCallId` that matches no pending call is an + orphan, with no name fallback. The emitter (`TraceRecorder`) generates a + `toolCallId` per call by default and copies it onto the result via the + `ToolCall` handle, so emitted traces always pair precisely. +- Fallback (no `toolCallId` on the result): the `tool` **name**, order-sensitive + and FIFO. A `tool_result` matches the earliest still-pending `tool_call` with + the same `tool`. So emit a call before its result, and do not interleave two + pending calls of the same tool name if you need them paired deterministically. +- A `tool_result` that matches no pending call -> `neutral` finding ("without matching tool_call"). - A `tool_call` with no later matching result -> `neutral` finding ("has no matching result"). diff --git a/agents/langgraph-reference/tests/test_emitter.py b/agents/langgraph-reference/tests/test_emitter.py index 65b4955..0ae1569 100644 --- a/agents/langgraph-reference/tests/test_emitter.py +++ b/agents/langgraph-reference/tests/test_emitter.py @@ -69,10 +69,30 @@ def test_tool_call_returns_handle_for_correlation() -> None: assert isinstance(handle, ToolCall) assert handle.tool == "fetch_issues" assert handle.step_index == 0 - # Passing the handle to the result reuses the tool name. + assert handle.id == "call-1" # auto-generated + # Passing the handle to the result reuses the tool name and the id. rec.record_tool_result(handle, output={"count": 2}, timestamp=_dt(2)) assert rec.steps[1].tool == "fetch_issues" assert rec.steps[1].type.value == "tool_result" + assert rec.steps[0].to_dict()["toolCallId"] == "call-1" + assert rec.steps[1].to_dict()["toolCallId"] == "call-1" + + +def test_tool_call_ids_are_unique_and_paired() -> None: + rec = TraceRecorder("run-1", "agent", "0.0.1", start_time=_dt(0)) + a = rec.record_tool_call("search", timestamp=_dt(1)) + b = rec.record_tool_call("search", timestamp=_dt(2)) + assert (a.id, b.id) == ("call-1", "call-2") + rec.record_tool_result(b, output="rb", timestamp=_dt(3)) + rec.record_tool_result(a, output="ra", timestamp=_dt(4)) + ids = [s.to_dict().get("toolCallId") for s in rec.steps] + assert ids == ["call-1", "call-2", "call-2", "call-1"] + + +def test_tool_result_by_bare_name_has_no_id() -> None: + rec = TraceRecorder("run-1", "agent", "0.0.1", start_time=_dt(0)) + rec.record_tool_result("db", error="boom2", timestamp=_dt(1)) + assert "toolCallId" not in rec.steps[0].to_dict() def test_tool_result_accepts_bare_name() -> None: diff --git a/agents/langgraph-reference/trazo_emitter/models.py b/agents/langgraph-reference/trazo_emitter/models.py index bed36ea..417fda4 100644 --- a/agents/langgraph-reference/trazo_emitter/models.py +++ b/agents/langgraph-reference/trazo_emitter/models.py @@ -43,6 +43,7 @@ class Step: duration_ms: int = 0 llm: Optional[str] = None tool: Optional[str] = None + tool_call_id: Optional[str] = None node: Optional[str] = None input: Optional[Any] = None output: Optional[Any] = None @@ -60,6 +61,8 @@ def to_dict(self) -> dict[str, Any]: d["llm"] = self.llm if self.tool: d["tool"] = self.tool + if self.tool_call_id: + d["toolCallId"] = self.tool_call_id if self.node: d["node"] = self.node if self.input is not None: diff --git a/agents/langgraph-reference/trazo_emitter/recorder.py b/agents/langgraph-reference/trazo_emitter/recorder.py index c4cf7bc..4a7f5f9 100644 --- a/agents/langgraph-reference/trazo_emitter/recorder.py +++ b/agents/langgraph-reference/trazo_emitter/recorder.py @@ -21,10 +21,13 @@ @dataclass(frozen=True) class ToolCall: - """Handle returned by record_tool_call, used to pair the later result.""" + """Handle returned by record_tool_call, used to pair the later result. The id + is written to both the call and its result as toolCallId, giving precise + pairing even when the same tool is called several times.""" tool: str step_index: int + id: str Clock = Callable[[], datetime] @@ -56,6 +59,7 @@ def __init__( self._clock: Clock = clock or _utc_now self.start_time: datetime = start_time or self._clock() self.steps: list[Step] = [] + self._tool_call_seq = 0 def _stamp(self, timestamp: Optional[datetime]) -> datetime: return timestamp if timestamp is not None else self._clock() @@ -109,17 +113,25 @@ def record_tool_call( input: Optional[Any] = None, duration_ms: int = 0, timestamp: Optional[datetime] = None, + call_id: Optional[str] = None, ) -> ToolCall: + """Record a tool call. A toolCallId is generated when not supplied, so the + emitted trace always pairs precisely; pass call_id to reuse the model's + own id when available.""" + if call_id is None: + self._tool_call_seq += 1 + call_id = f"call-{self._tool_call_seq}" self.steps.append( Step( type=StepType.TOOL_CALL, timestamp=self._stamp(timestamp), tool=tool, + tool_call_id=call_id, input=input, duration_ms=duration_ms, ) ) - return ToolCall(tool=tool, step_index=len(self.steps) - 1) + return ToolCall(tool=tool, step_index=len(self.steps) - 1, id=call_id) def record_tool_result( self, @@ -130,12 +142,21 @@ def record_tool_result( duration_ms: int = 0, timestamp: Optional[datetime] = None, ) -> None: - tool = call.tool if isinstance(call, ToolCall) else call + """Record a tool result. Passing the ToolCall handle carries its + toolCallId onto the result for id-based pairing; a bare tool name pairs by + name/order instead.""" + if isinstance(call, ToolCall): + tool = call.tool + call_id: Optional[str] = call.id + else: + tool = call + call_id = None self.steps.append( Step( type=StepType.TOOL_RESULT, timestamp=self._stamp(timestamp), tool=tool, + tool_call_id=call_id, output=output, error=error, duration_ms=duration_ms, diff --git a/evaluator/toolcalls.go b/evaluator/toolcalls.go index dc98aa3..962f94b 100644 --- a/evaluator/toolcalls.go +++ b/evaluator/toolcalls.go @@ -19,16 +19,11 @@ func (e *ToolCallEvaluator) EvaluateRun(run *trajectory.Run) (*Evaluation, error var pending []int for i, step := range run.Steps { - if step.Type == trajectory.StepTypeToolCall { + switch step.Type { + case trajectory.StepTypeToolCall: pending = append(pending, i) - } else if step.Type == trajectory.StepTypeToolResult { - matched := -1 - for j, pIdx := range pending { - if run.Steps[pIdx].Tool == step.Tool { - matched = j - break - } - } + case trajectory.StepTypeToolResult: + matched := matchPending(run, pending, step) if matched != -1 { pending = append(pending[:matched], pending[matched+1:]...) } else { @@ -56,3 +51,25 @@ func (e *ToolCallEvaluator) EvaluateRun(run *trajectory.Run) (*Evaluation, error } return eva, nil } + +// matchPending finds the index within pending of the tool_call that a +// tool_result answers, or -1 if none. When the result carries a toolCallId, the +// match is by id and is authoritative: no id match means an orphan, with no +// fallback. Only when the result has no id do we fall back to the earliest +// pending call with the same tool name (FIFO), preserving legacy behavior. +func matchPending(run *trajectory.Run, pending []int, result trajectory.Step) int { + if result.ToolCallID != "" { + for j, pIdx := range pending { + if run.Steps[pIdx].ToolCallID == result.ToolCallID { + return j + } + } + return -1 + } + for j, pIdx := range pending { + if run.Steps[pIdx].Tool == result.Tool { + return j + } + } + return -1 +} diff --git a/evaluator/toolcalls_test.go b/evaluator/toolcalls_test.go index dc70bf5..c86d520 100644 --- a/evaluator/toolcalls_test.go +++ b/evaluator/toolcalls_test.go @@ -77,6 +77,60 @@ func TestToolCallEvaluator_MissingResult(t *testing.T) { } } +func idToolCall(tool, id string) trajectory.Step { + return trajectory.Step{Type: trajectory.StepTypeToolCall, Tool: tool, ToolCallID: id} +} + +func idToolResult(tool, id string) trajectory.Step { + return trajectory.Step{Type: trajectory.StepTypeToolResult, Tool: tool, ToolCallID: id} +} + +func TestToolCallEvaluator_PairsByIDOutOfOrder(t *testing.T) { + // Two calls to the same tool, results returned in the opposite order. By id + // each result still finds its own call, so there are no orphans. + run := &trajectory.Run{ID: "r", Steps: []trajectory.Step{ + idToolCall("search", "c1"), + idToolCall("search", "c2"), + idToolResult("search", "c2"), + idToolResult("search", "c1"), + }} + eval, _ := (&ToolCallEvaluator{}).EvaluateRun(run) + if len(eval.Findings) != 0 { + t.Fatalf("expected 0 findings with id pairing, got %d: %+v", len(eval.Findings), eval.Findings) + } +} + +func TestToolCallEvaluator_ResultWithUnknownIDIsOrphan(t *testing.T) { + // The result's id matches no pending call: it is an orphan, and the call is + // left unanswered. The id is authoritative, so there is no name fallback. + run := &trajectory.Run{ID: "r", Steps: []trajectory.Step{ + idToolCall("t", "c1"), + idToolResult("t", "c2"), + }} + eval, _ := (&ToolCallEvaluator{}).EvaluateRun(run) + if len(eval.Findings) != 2 { + t.Fatalf("expected 2 neutral findings (orphan result + unanswered call), got %d: %+v", + len(eval.Findings), eval.Findings) + } + for _, f := range eval.Findings { + if f.Judgment != JudgmentNeutral { + t.Errorf("expected neutral, got %s", f.Judgment) + } + } +} + +func TestToolCallEvaluator_IDMatchWinsOverName(t *testing.T) { + // Even with different tool names, a matching id pairs them. + run := &trajectory.Run{ID: "r", Steps: []trajectory.Step{ + idToolCall("a", "x"), + idToolResult("b", "x"), + }} + eval, _ := (&ToolCallEvaluator{}).EvaluateRun(run) + if len(eval.Findings) != 0 { + t.Fatalf("expected id match to pair across names, got %d: %+v", len(eval.Findings), eval.Findings) + } +} + func TestToolCallEvaluator_OrphanResult(t *testing.T) { data, err := os.ReadFile("../testdata/sample_run_orphan_result.json") if err != nil { diff --git a/trajectory/steps.go b/trajectory/steps.go index 083a231..6dcd217 100644 --- a/trajectory/steps.go +++ b/trajectory/steps.go @@ -26,6 +26,7 @@ type Run struct { type Step struct { LLM string `json:"llm,omitempty"` Tool string `json:"tool,omitempty"` + ToolCallID string `json:"toolCallId,omitempty"` Node string `json:"node,omitempty"` Type StepType `json:"type"` Timestamp time.Time `json:"timestamp"` From c160795257e75077cc82bc15db2b1c9e1b2a0cf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jesus=20Nu=C3=B1ez?= Date: Fri, 7 Aug 2026 00:03:30 -0400 Subject: [PATCH 2/9] harden: deepen Run.Validate with structural invariants Validate now rejects non-negative violations (cost, inputTokens, outputTokens, durationMs), non-monotonic step timestamps, and steps falling outside the run's [startTime, endTime] interval, on top of the existing required-field checks. Strictness is pragmatic (validate-if-present): an absent version and an empty step list are still accepted, keeping older emitted traces valid. A missing step timestamp is flagged as missing but skipped for ordering so it cannot produce a spurious before-previous error. Schema-version compatibility stays out of scope here. Adds aggressive edge-case tests: negative quantities, out-of-order timestamps, steps before startTime / after endTime, the lenient-optional-fields policy, and the missing-timestamp-skips-ordering guard. Co-Authored-By: Claude Opus 4.8 (1M context) --- trajectory/validate.go | 42 +++++++++++++++++++++++++++ trajectory/validate_test.go | 57 +++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/trajectory/validate.go b/trajectory/validate.go index 0907bd2..39c9788 100644 --- a/trajectory/validate.go +++ b/trajectory/validate.go @@ -3,12 +3,20 @@ package trajectory import ( "errors" "fmt" + "time" ) // Validate checks structural invariants of a Run and returns every violation // joined into a single error, so callers see the full list at once. It does // not judge agent behavior (e.g. tool call/result pairing); that is the job // of evaluators. +// +// Strictness policy: hard invariants (non-negative quantities, monotonic step +// timestamps, steps within the run interval) reject the trace. Optional fields +// are validated only when present: an absent version or an empty step list is +// accepted, since older emitters produced such traces and they carry no +// ambiguity. Schema-version compatibility is a separate concern (see the +// schema doc), not enforced here. func (r *Run) Validate() error { var errs []error @@ -28,10 +36,44 @@ func (r *Run) Validate() error { errs = append(errs, errors.New("run: endTime is before startTime")) } + // prevTS tracks the last step with a usable (non-zero) timestamp, so the + // monotonicity check skips over steps whose timestamp is already flagged + // as missing instead of reporting a spurious ordering error against a zero. + var prevTS time.Time + var prevIdx int + havePrev := false + for i, step := range r.Steps { if step.Timestamp.IsZero() { errs = append(errs, fmt.Errorf("step %d: timestamp is missing", i)) + } else { + if havePrev && step.Timestamp.Before(prevTS) { + errs = append(errs, fmt.Errorf("step %d: timestamp is before step %d", i, prevIdx)) + } + if !r.StartTime.IsZero() && step.Timestamp.Before(r.StartTime) { + errs = append(errs, fmt.Errorf("step %d: timestamp is before run startTime", i)) + } + if !r.EndTime.IsZero() && step.Timestamp.After(r.EndTime) { + errs = append(errs, fmt.Errorf("step %d: timestamp is after run endTime", i)) + } + prevTS = step.Timestamp + prevIdx = i + havePrev = true + } + + if step.Cost < 0 { + errs = append(errs, fmt.Errorf("step %d: cost is negative (%g)", i, step.Cost)) + } + if step.InputTokens < 0 { + errs = append(errs, fmt.Errorf("step %d: inputTokens is negative (%d)", i, step.InputTokens)) + } + if step.OutputTokens < 0 { + errs = append(errs, fmt.Errorf("step %d: outputTokens is negative (%d)", i, step.OutputTokens)) } + if step.DurationMs < 0 { + errs = append(errs, fmt.Errorf("step %d: durationMs is negative (%d)", i, step.DurationMs)) + } + switch step.Type { case StepTypeCallLLM: if step.LLM == "" { diff --git a/trajectory/validate_test.go b/trajectory/validate_test.go index 8d8a639..bfadade 100644 --- a/trajectory/validate_test.go +++ b/trajectory/validate_test.go @@ -47,6 +47,13 @@ func TestValidate_InvalidRuns(t *testing.T) { {"tool_result without tool", func(r *Run) { r.Steps[3].Tool = "" }, "step 3: tool_result without tool"}, {"node_transition without node", func(r *Run) { r.Steps[0].Node = "" }, "step 0: node_transition without node"}, {"unknown step type", func(r *Run) { r.Steps[1].Type = "banana" }, `step 1: unknown step type "banana"`}, + {"negative cost", func(r *Run) { r.Steps[1].Cost = -0.01 }, "step 1: cost is negative"}, + {"negative inputTokens", func(r *Run) { r.Steps[1].InputTokens = -5 }, "step 1: inputTokens is negative"}, + {"negative outputTokens", func(r *Run) { r.Steps[1].OutputTokens = -5 }, "step 1: outputTokens is negative"}, + {"negative durationMs", func(r *Run) { r.Steps[1].DurationMs = -1 }, "step 1: durationMs is negative"}, + {"timestamp before previous", func(r *Run) { r.Steps[2].Timestamp = r.Steps[1].Timestamp.Add(-time.Second) }, "step 2: timestamp is before step 1"}, + {"step before run startTime", func(r *Run) { r.Steps[0].Timestamp = r.StartTime.Add(-time.Second) }, "step 0: timestamp is before run startTime"}, + {"step after run endTime", func(r *Run) { r.Steps[3].Timestamp = r.EndTime.Add(time.Second) }, "step 3: timestamp is after run endTime"}, } for _, tc := range cases { @@ -64,6 +71,56 @@ func TestValidate_InvalidRuns(t *testing.T) { } } +// TestValidate_LenientOptionalFields pins the pragmatic strictness policy: +// an absent version and an empty step list are accepted, since older emitters +// produced such traces and they carry no structural ambiguity. +func TestValidate_LenientOptionalFields(t *testing.T) { + t.Run("absent version", func(t *testing.T) { + run := validRun() + run.Version = "" + if err := run.Validate(); err != nil { + t.Errorf("absent version should be accepted, got: %v", err) + } + }) + t.Run("empty steps", func(t *testing.T) { + run := validRun() + run.Steps = nil + if err := run.Validate(); err != nil { + t.Errorf("empty step list should be accepted, got: %v", err) + } + }) + t.Run("zero cost and tokens", func(t *testing.T) { + run := validRun() + for i := range run.Steps { + run.Steps[i].Cost = 0 + run.Steps[i].InputTokens = 0 + run.Steps[i].OutputTokens = 0 + run.Steps[i].DurationMs = 0 + } + if err := run.Validate(); err != nil { + t.Errorf("zero quantities should be accepted, got: %v", err) + } + }) +} + +// TestValidate_MissingTimestampSkipsOrdering guards the monotonicity check: +// a step with a missing timestamp is flagged as missing but must not produce +// a spurious ordering error against the following step. +func TestValidate_MissingTimestampSkipsOrdering(t *testing.T) { + run := validRun() + run.Steps[1].Timestamp = time.Time{} + err := run.Validate() + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "step 1: timestamp is missing") { + t.Errorf("expected missing-timestamp error, got: %v", err) + } + if strings.Contains(err.Error(), "is before step") { + t.Errorf("missing timestamp must not trigger an ordering error, got: %v", err) + } +} + func TestValidate_CollectsAllErrors(t *testing.T) { run := validRun() run.ID = "" From b7bdadd194c98158fdb69b3440f95c4d281f3dbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jesus=20Nu=C3=B1ez?= Date: Fri, 7 Aug 2026 00:23:47 -0400 Subject: [PATCH 3/9] harden: thread context.Context through the Evaluator interface Evaluator.EvaluateRun now takes a context.Context as its first argument. Cancelling it aborts in-flight evaluation instead of running to completion: - The four compute evaluators (tool_calls, loops, cost_latency, node_transitions) check ctx.Err() on entry and return it promptly. - The LLM judge uses the caller's ctx as the parent of its per-call timeout, so a cancelled run also aborts the outbound Gemini HTTP request (previously it started from context.Background() and ignored the caller). - The runner propagates ctx to every file and every evaluator, short-circuiting a file to a context.Canceled FileError before touching disk. - The CLI wires signal.NotifyContext(os.Interrupt) so Ctrl+C cancels a run, which matters most for the network-bound judge. Adds cancellation tests: a runner-level test (cancelled ctx yields zero evaluations and a context.Canceled error per file) and an evaluator-level table test pinning that every evaluator honors a cancelled context. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/trazo/main.go | 9 ++++++- evaluator/Evaluator.go | 7 +++++- evaluator/context_test.go | 49 +++++++++++++++++++++++++++++++++++++ evaluator/cost.go | 6 ++++- evaluator/cost_test.go | 13 +++++----- evaluator/judge.go | 9 +++++-- evaluator/judge_test.go | 14 +++++------ evaluator/loops.go | 6 ++++- evaluator/loops_test.go | 15 ++++++------ evaluator/node.go | 6 ++++- evaluator/node_test.go | 13 +++++----- evaluator/toolcalls.go | 6 ++++- evaluator/toolcalls_test.go | 15 ++++++------ runner/runner.go | 16 +++++++++--- runner/runner_test.go | 31 +++++++++++++++++++++-- 15 files changed, 168 insertions(+), 47 deletions(-) create mode 100644 evaluator/context_test.go diff --git a/cmd/trazo/main.go b/cmd/trazo/main.go index 4f5d612..b1a3bb7 100644 --- a/cmd/trazo/main.go +++ b/cmd/trazo/main.go @@ -1,11 +1,13 @@ package main import ( + "context" "encoding/json" "flag" "fmt" "log" "os" + "os/signal" "strings" "github.com/Cro22/trazo/evaluator" @@ -66,7 +68,12 @@ func main() { evaluators = append(evaluators, &evaluator.LLMJudgeEvaluator{Client: client}) } - resp, err := runner.NewRunner(evaluators).Run(*dir) + // Cancel in-flight evaluation on Ctrl+C (SIGINT) or SIGTERM so a long run, + // notably one using the network-bound LLM judge, stops promptly. + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt) + defer stop() + + resp, err := runner.NewRunner(evaluators).Run(ctx, *dir) if err != nil { log.Fatalf("Error running: %v", err) } diff --git a/evaluator/Evaluator.go b/evaluator/Evaluator.go index efc4e1f..11dc7d6 100644 --- a/evaluator/Evaluator.go +++ b/evaluator/Evaluator.go @@ -1,11 +1,16 @@ package evaluator import ( + "context" + "github.com/Cro22/trazo/trajectory" ) +// Evaluator judges a single run and reports findings. Implementations must +// honor ctx: return ctx.Err() promptly if it is cancelled (the LLM judge, for +// instance, ties its network call to ctx so a cancelled run does not hang). type Evaluator interface { - EvaluateRun(run *trajectory.Run) (*Evaluation, error) + EvaluateRun(ctx context.Context, run *trajectory.Run) (*Evaluation, error) } type Judgment string diff --git a/evaluator/context_test.go b/evaluator/context_test.go new file mode 100644 index 0000000..dea3f66 --- /dev/null +++ b/evaluator/context_test.go @@ -0,0 +1,49 @@ +package evaluator + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/Cro22/trazo/trajectory" +) + +// TestEvaluators_HonorCancelledContext pins the interface contract added in the +// context.Context milestone: every evaluator must return ctx.Err() promptly when +// handed an already-cancelled context, rather than doing the work anyway. The +// LLM judge uses a fake client so no network is touched. +func TestEvaluators_HonorCancelledContext(t *testing.T) { + start := time.Date(2026, 8, 7, 9, 0, 0, 0, time.UTC) + run := &trajectory.Run{ + ID: "run-ctx", + Agent: "ctx_probe", + Version: "1.0.0", + StartTime: start, + EndTime: start.Add(2 * time.Second), + Steps: []trajectory.Step{ + {Node: "start", Type: trajectory.StepTypeNodeTransition, Timestamp: start}, + {LLM: "gpt-4o", Type: trajectory.StepTypeCallLLM, Timestamp: start.Add(time.Second)}, + }, + } + + evaluators := map[string]Evaluator{ + "tool_calls": &ToolCallEvaluator{}, + "loops": &LoopEvaluator{}, + "cost_latency": &CostLatencyEvaluator{}, + "node_transitions": &NodeTransitionEvaluator{}, + "llm_judge": &LLMJudgeEvaluator{Client: fakeJudge{reply: "unused"}}, + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + for name, e := range evaluators { + t.Run(name, func(t *testing.T) { + _, err := e.EvaluateRun(ctx, run) + if !errors.Is(err, context.Canceled) { + t.Errorf("expected context.Canceled, got %v", err) + } + }) + } +} diff --git a/evaluator/cost.go b/evaluator/cost.go index 5f31c0f..10af48f 100644 --- a/evaluator/cost.go +++ b/evaluator/cost.go @@ -1,6 +1,7 @@ package evaluator import ( + "context" "fmt" "github.com/Cro22/trazo/trajectory" @@ -58,7 +59,10 @@ func (e *CostLatencyEvaluator) maxRunLatencyMs() int64 { return e.MaxRunLatencyMs } -func (e *CostLatencyEvaluator) EvaluateRun(run *trajectory.Run) (*Evaluation, error) { +func (e *CostLatencyEvaluator) EvaluateRun(ctx context.Context, run *trajectory.Run) (*Evaluation, error) { + if err := ctx.Err(); err != nil { + return nil, err + } eva := &Evaluation{ EvaluatorName: "cost_latency", RunID: run.ID, diff --git a/evaluator/cost_test.go b/evaluator/cost_test.go index c208c8b..c3c05d2 100644 --- a/evaluator/cost_test.go +++ b/evaluator/cost_test.go @@ -1,6 +1,7 @@ package evaluator import ( + "context" "testing" "time" @@ -39,7 +40,7 @@ func countJudgment(eval *Evaluation, j Judgment) int { func TestCostLatency_StepCostOverBudget(t *testing.T) { run := costRun(1, llmStep(0.10, 100)) - eval, err := (&CostLatencyEvaluator{}).EvaluateRun(run) + eval, err := (&CostLatencyEvaluator{}).EvaluateRun(context.Background(), run) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -53,7 +54,7 @@ func TestCostLatency_StepCostOverBudget(t *testing.T) { func TestCostLatency_StepLatencyOverBudget(t *testing.T) { run := costRun(1, llmStep(0.001, 40000)) - eval, _ := (&CostLatencyEvaluator{}).EvaluateRun(run) + eval, _ := (&CostLatencyEvaluator{}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 1 { t.Fatalf("expected 1 finding, got %d", len(eval.Findings)) } @@ -69,7 +70,7 @@ func TestCostLatency_AggregateRunCost(t *testing.T) { llmStep(0.05, 10), llmStep(0.05, 10), llmStep(0.05, 10), llmStep(0.05, 10), llmStep(0.05, 10), ) - eval, _ := (&CostLatencyEvaluator{}).EvaluateRun(run) + eval, _ := (&CostLatencyEvaluator{}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 1 { t.Fatalf("expected 1 run-level finding, got %d: %+v", len(eval.Findings), eval.Findings) } @@ -80,7 +81,7 @@ func TestCostLatency_AggregateRunCost(t *testing.T) { func TestCostLatency_RunLatencyOverBudget(t *testing.T) { run := costRun(200, llmStep(0.001, 10)) // 200s wall-clock > 120s default - eval, _ := (&CostLatencyEvaluator{}).EvaluateRun(run) + eval, _ := (&CostLatencyEvaluator{}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 1 { t.Fatalf("expected 1 finding, got %d", len(eval.Findings)) } @@ -91,7 +92,7 @@ func TestCostLatency_RunLatencyOverBudget(t *testing.T) { func TestCostLatency_WithinBudget(t *testing.T) { run := costRun(5, llmStep(0.001, 500), llmStep(0.002, 800)) - eval, _ := (&CostLatencyEvaluator{}).EvaluateRun(run) + eval, _ := (&CostLatencyEvaluator{}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 0 { t.Errorf("expected 0 findings within budget, got %d: %+v", len(eval.Findings), eval.Findings) } @@ -100,7 +101,7 @@ func TestCostLatency_WithinBudget(t *testing.T) { func TestCostLatency_CustomThresholds(t *testing.T) { run := costRun(5, llmStep(0.01, 2000)) e := &CostLatencyEvaluator{MaxStepCost: 0.005, MaxStepLatencyMs: 1000} - eval, _ := e.EvaluateRun(run) + eval, _ := e.EvaluateRun(context.Background(), run) // Both step cost and step latency breach the tighter budgets. if countJudgment(eval, JudgmentNeutral) != 2 { t.Fatalf("expected 2 neutral findings, got %d: %+v", len(eval.Findings), eval.Findings) diff --git a/evaluator/judge.go b/evaluator/judge.go index 80791e6..743e80d 100644 --- a/evaluator/judge.go +++ b/evaluator/judge.go @@ -41,7 +41,10 @@ func (e *LLMJudgeEvaluator) timeout() time.Duration { return e.Timeout } -func (e *LLMJudgeEvaluator) EvaluateRun(run *trajectory.Run) (*Evaluation, error) { +func (e *LLMJudgeEvaluator) EvaluateRun(ctx context.Context, run *trajectory.Run) (*Evaluation, error) { + if err := ctx.Err(); err != nil { + return nil, err + } eva := &Evaluation{ EvaluatorName: "llm_judge", RunID: run.ID, @@ -56,7 +59,9 @@ func (e *LLMJudgeEvaluator) EvaluateRun(run *trajectory.Run) (*Evaluation, error return eva, nil // nothing to judge } - ctx, cancel := context.WithTimeout(context.Background(), e.timeout()) + // Bound the call by the judge timeout, but keep the caller's ctx as parent so + // a cancelled run (Ctrl+C, an aborting runner) also aborts the network call. + ctx, cancel := context.WithTimeout(ctx, e.timeout()) defer cancel() raw, err := e.Client.Complete(ctx, judgePrompt(run, output)) diff --git a/evaluator/judge_test.go b/evaluator/judge_test.go index 3251797..cc16b6b 100644 --- a/evaluator/judge_test.go +++ b/evaluator/judge_test.go @@ -35,7 +35,7 @@ func judgeRun(steps ...trajectory.Step) *trajectory.Run { func TestLLMJudge_GoodVerdict(t *testing.T) { run := judgeRun(llmOut(`"1 bug, 1 doc. Clear report."`)) e := &LLMJudgeEvaluator{Client: fakeJudge{reply: `{"judgment":"good","score":0.9,"comment":"clear"}`}} - eval, err := e.EvaluateRun(run) + eval, err := e.EvaluateRun(context.Background(), run) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -52,7 +52,7 @@ func TestLLMJudge_BadVerdictWithProse(t *testing.T) { // The judge wraps the JSON in prose and a code fence; we still parse it. reply := "Here is my grade:\n```json\n{\"judgment\": \"bad\", \"score\": 0.1, \"comment\": \"empty\"}\n```" e := &LLMJudgeEvaluator{Client: fakeJudge{reply: reply}} - eval, _ := e.EvaluateRun(judgeRun(llmOut(`""`))) + eval, _ := e.EvaluateRun(context.Background(), judgeRun(llmOut(`""`))) if len(eval.Findings) != 1 || eval.Findings[0].Judgment != JudgmentBad { t.Fatalf("expected 1 bad finding, got %+v", eval.Findings) } @@ -60,7 +60,7 @@ func TestLLMJudge_BadVerdictWithProse(t *testing.T) { func TestLLMJudge_UnparseableIsNeutral(t *testing.T) { e := &LLMJudgeEvaluator{Client: fakeJudge{reply: "I cannot comply."}} - eval, _ := e.EvaluateRun(judgeRun(llmOut(`"x"`))) + eval, _ := e.EvaluateRun(context.Background(), judgeRun(llmOut(`"x"`))) if len(eval.Findings) != 1 || eval.Findings[0].Judgment != JudgmentNeutral { t.Fatalf("expected 1 neutral finding, got %+v", eval.Findings) } @@ -68,7 +68,7 @@ func TestLLMJudge_UnparseableIsNeutral(t *testing.T) { func TestLLMJudge_NoLLMCallNoFindings(t *testing.T) { run := judgeRun(trajectory.Step{Type: trajectory.StepTypeToolCall, Tool: "t"}) - eval, err := (&LLMJudgeEvaluator{Client: fakeJudge{reply: "unused"}}).EvaluateRun(run) + eval, err := (&LLMJudgeEvaluator{Client: fakeJudge{reply: "unused"}}).EvaluateRun(context.Background(), run) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -79,14 +79,14 @@ func TestLLMJudge_NoLLMCallNoFindings(t *testing.T) { func TestLLMJudge_ClientErrorPropagates(t *testing.T) { e := &LLMJudgeEvaluator{Client: fakeJudge{err: errors.New("network down")}} - _, err := e.EvaluateRun(judgeRun(llmOut(`"x"`))) + _, err := e.EvaluateRun(context.Background(), judgeRun(llmOut(`"x"`))) if err == nil { t.Fatal("expected an error when the client fails") } } func TestLLMJudge_NilClientErrors(t *testing.T) { - _, err := (&LLMJudgeEvaluator{}).EvaluateRun(judgeRun(llmOut(`"x"`))) + _, err := (&LLMJudgeEvaluator{}).EvaluateRun(context.Background(), judgeRun(llmOut(`"x"`))) if err == nil { t.Fatal("expected an error when no client is configured") } @@ -103,7 +103,7 @@ func TestGeminiClient_CompleteAgainstFakeServer(t *testing.T) { defer server.Close() client := &GeminiClient{APIKey: "test", Model: "gemini-2.5-flash", Endpoint: server.URL, HTTPClient: server.Client()} - eval, err := (&LLMJudgeEvaluator{Client: client}).EvaluateRun(judgeRun(llmOut(`"a report"`))) + eval, err := (&LLMJudgeEvaluator{Client: client}).EvaluateRun(context.Background(), judgeRun(llmOut(`"a report"`))) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/evaluator/loops.go b/evaluator/loops.go index 76c5681..0cf0569 100644 --- a/evaluator/loops.go +++ b/evaluator/loops.go @@ -1,6 +1,7 @@ package evaluator import ( + "context" "encoding/json" "fmt" @@ -31,7 +32,10 @@ func (e *LoopEvaluator) maxRepeats() int { return e.MaxRepeats } -func (e *LoopEvaluator) EvaluateRun(run *trajectory.Run) (*Evaluation, error) { +func (e *LoopEvaluator) EvaluateRun(ctx context.Context, run *trajectory.Run) (*Evaluation, error) { + if err := ctx.Err(); err != nil { + return nil, err + } eva := &Evaluation{ EvaluatorName: "loops", RunID: run.ID, diff --git a/evaluator/loops_test.go b/evaluator/loops_test.go index 1bc807c..0954760 100644 --- a/evaluator/loops_test.go +++ b/evaluator/loops_test.go @@ -1,6 +1,7 @@ package evaluator import ( + "context" "encoding/json" "testing" @@ -29,7 +30,7 @@ func TestLoopEvaluator_RepeatedToolCallFlagged(t *testing.T) { toolCall("fetch_issues", `{"repo":"x/y"}`), toolCall("fetch_issues", `{"repo":"x/y"}`), ) - eval, err := (&LoopEvaluator{}).EvaluateRun(run) + eval, err := (&LoopEvaluator{}).EvaluateRun(context.Background(), run) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -52,7 +53,7 @@ func TestLoopEvaluator_DistinctInputsNotFlagged(t *testing.T) { toolCall("classify_issue", `{"title":"b"}`), toolCall("classify_issue", `{"title":"c"}`), ) - eval, _ := (&LoopEvaluator{}).EvaluateRun(run) + eval, _ := (&LoopEvaluator{}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 0 { t.Errorf("expected 0 findings for distinct inputs, got %d: %+v", len(eval.Findings), eval.Findings) } @@ -63,7 +64,7 @@ func TestLoopEvaluator_BelowThresholdNotFlagged(t *testing.T) { toolCall("fetch_issues", `{"repo":"x/y"}`), toolCall("fetch_issues", `{"repo":"x/y"}`), ) - eval, _ := (&LoopEvaluator{}).EvaluateRun(run) + eval, _ := (&LoopEvaluator{}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 0 { t.Errorf("expected 0 findings below threshold, got %d", len(eval.Findings)) } @@ -74,7 +75,7 @@ func TestLoopEvaluator_CustomThreshold(t *testing.T) { toolCall("fetch_issues", `{"repo":"x/y"}`), toolCall("fetch_issues", `{"repo":"x/y"}`), ) - eval, _ := (&LoopEvaluator{MaxRepeats: 2}).EvaluateRun(run) + eval, _ := (&LoopEvaluator{MaxRepeats: 2}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 1 { t.Fatalf("expected 1 finding with MaxRepeats=2, got %d", len(eval.Findings)) } @@ -90,7 +91,7 @@ func TestLoopEvaluator_CanonicalInputMatches(t *testing.T) { toolCall("t", `{ "b":2, "a":1 }`), toolCall("t", `{"a":1, "b":2}`), ) - eval, _ := (&LoopEvaluator{}).EvaluateRun(run) + eval, _ := (&LoopEvaluator{}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 1 { t.Fatalf("expected 1 finding for canonically equal inputs, got %d", len(eval.Findings)) } @@ -102,7 +103,7 @@ func TestLoopEvaluator_NodeCycleFlagged(t *testing.T) { nodeStep("router"), nodeStep("router"), ) - eval, _ := (&LoopEvaluator{}).EvaluateRun(run) + eval, _ := (&LoopEvaluator{}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 1 { t.Fatalf("expected 1 finding for node cycle, got %d", len(eval.Findings)) } @@ -119,7 +120,7 @@ func TestLoopEvaluator_OnlyFlagsOncePerKey(t *testing.T) { toolCall("t", `{"x":1}`), toolCall("t", `{"x":1}`), ) - eval, _ := (&LoopEvaluator{}).EvaluateRun(run) + eval, _ := (&LoopEvaluator{}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 1 { t.Fatalf("expected exactly 1 finding, got %d", len(eval.Findings)) } diff --git a/evaluator/node.go b/evaluator/node.go index 28984c7..3409c0a 100644 --- a/evaluator/node.go +++ b/evaluator/node.go @@ -1,6 +1,7 @@ package evaluator import ( + "context" "fmt" "strings" @@ -34,7 +35,10 @@ func (e *NodeTransitionEvaluator) terminalSet() map[string]bool { return set } -func (e *NodeTransitionEvaluator) EvaluateRun(run *trajectory.Run) (*Evaluation, error) { +func (e *NodeTransitionEvaluator) EvaluateRun(ctx context.Context, run *trajectory.Run) (*Evaluation, error) { + if err := ctx.Err(); err != nil { + return nil, err + } eva := &Evaluation{ EvaluatorName: "node_transitions", RunID: run.ID, diff --git a/evaluator/node_test.go b/evaluator/node_test.go index 3fc94bf..e8eeaf9 100644 --- a/evaluator/node_test.go +++ b/evaluator/node_test.go @@ -1,6 +1,7 @@ package evaluator import ( + "context" "testing" "github.com/Cro22/trazo/trajectory" @@ -8,7 +9,7 @@ import ( func TestNodeTransition_EndsAtTerminal(t *testing.T) { run := runWith(nodeStep("start"), nodeStep("router"), nodeStep("end")) - eval, err := (&NodeTransitionEvaluator{}).EvaluateRun(run) + eval, err := (&NodeTransitionEvaluator{}).EvaluateRun(context.Background(), run) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -19,7 +20,7 @@ func TestNodeTransition_EndsAtTerminal(t *testing.T) { func TestNodeTransition_EndsAtNonTerminalFlagged(t *testing.T) { run := runWith(nodeStep("start"), nodeStep("fallback_handler")) - eval, _ := (&NodeTransitionEvaluator{}).EvaluateRun(run) + eval, _ := (&NodeTransitionEvaluator{}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 1 { t.Fatalf("expected 1 finding, got %d", len(eval.Findings)) } @@ -30,7 +31,7 @@ func TestNodeTransition_EndsAtNonTerminalFlagged(t *testing.T) { func TestNodeTransition_CaseInsensitiveTerminal(t *testing.T) { run := runWith(nodeStep("START"), nodeStep("END")) - eval, _ := (&NodeTransitionEvaluator{}).EvaluateRun(run) + eval, _ := (&NodeTransitionEvaluator{}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 0 { t.Errorf("expected 0 findings for END, got %d", len(eval.Findings)) } @@ -40,7 +41,7 @@ func TestNodeTransition_NoNodeTransitions(t *testing.T) { run := runWith( trajectory.Step{Type: trajectory.StepTypeToolCall, Tool: "t"}, ) - eval, _ := (&NodeTransitionEvaluator{}).EvaluateRun(run) + eval, _ := (&NodeTransitionEvaluator{}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 0 { t.Errorf("expected 0 findings when there are no node transitions, got %d", len(eval.Findings)) } @@ -53,7 +54,7 @@ func TestNodeTransition_IgnoresTrailingNonNodeSteps(t *testing.T) { nodeStep("end"), trajectory.Step{Type: trajectory.StepTypeToolCall, Tool: "t"}, ) - eval, _ := (&NodeTransitionEvaluator{}).EvaluateRun(run) + eval, _ := (&NodeTransitionEvaluator{}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 0 { t.Errorf("expected 0 findings, got %d: %+v", len(eval.Findings), eval.Findings) } @@ -61,7 +62,7 @@ func TestNodeTransition_IgnoresTrailingNonNodeSteps(t *testing.T) { func TestNodeTransition_CustomTerminalSet(t *testing.T) { run := runWith(nodeStep("start"), nodeStep("complete")) - eval, _ := (&NodeTransitionEvaluator{TerminalNodes: []string{"complete"}}).EvaluateRun(run) + eval, _ := (&NodeTransitionEvaluator{TerminalNodes: []string{"complete"}}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 0 { t.Errorf("expected 0 findings with custom terminal set, got %d", len(eval.Findings)) } diff --git a/evaluator/toolcalls.go b/evaluator/toolcalls.go index 962f94b..648c5d9 100644 --- a/evaluator/toolcalls.go +++ b/evaluator/toolcalls.go @@ -1,6 +1,7 @@ package evaluator import ( + "context" "fmt" "github.com/Cro22/trazo/trajectory" @@ -9,7 +10,10 @@ import ( type ToolCallEvaluator struct { } -func (e *ToolCallEvaluator) EvaluateRun(run *trajectory.Run) (*Evaluation, error) { +func (e *ToolCallEvaluator) EvaluateRun(ctx context.Context, run *trajectory.Run) (*Evaluation, error) { + if err := ctx.Err(); err != nil { + return nil, err + } eva := &Evaluation{ EvaluatorName: "tool_calls", RunID: run.ID, diff --git a/evaluator/toolcalls_test.go b/evaluator/toolcalls_test.go index c86d520..53b977f 100644 --- a/evaluator/toolcalls_test.go +++ b/evaluator/toolcalls_test.go @@ -1,6 +1,7 @@ package evaluator import ( + "context" "os" "testing" @@ -19,7 +20,7 @@ func TestToolCallEvaluator_EvaluateRun(t *testing.T) { } evaluator := &ToolCallEvaluator{} - eval, err := evaluator.EvaluateRun(run) + eval, err := evaluator.EvaluateRun(context.Background(), run) if err != nil { t.Fatalf("unexpected error in EvaluateRun: %v", err) } @@ -43,7 +44,7 @@ func TestToolCallEvaluator_Success(t *testing.T) { } evaluator := &ToolCallEvaluator{} - eval, err := evaluator.EvaluateRun(run) + eval, err := evaluator.EvaluateRun(context.Background(), run) if err != nil { t.Fatalf("unexpected error in EvaluateRun: %v", err) } @@ -65,7 +66,7 @@ func TestToolCallEvaluator_MissingResult(t *testing.T) { } evaluator := &ToolCallEvaluator{} - eval, err := evaluator.EvaluateRun(run) + eval, err := evaluator.EvaluateRun(context.Background(), run) if err != nil { t.Fatalf("unexpected error in EvaluateRun: %v", err) } @@ -94,7 +95,7 @@ func TestToolCallEvaluator_PairsByIDOutOfOrder(t *testing.T) { idToolResult("search", "c2"), idToolResult("search", "c1"), }} - eval, _ := (&ToolCallEvaluator{}).EvaluateRun(run) + eval, _ := (&ToolCallEvaluator{}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 0 { t.Fatalf("expected 0 findings with id pairing, got %d: %+v", len(eval.Findings), eval.Findings) } @@ -107,7 +108,7 @@ func TestToolCallEvaluator_ResultWithUnknownIDIsOrphan(t *testing.T) { idToolCall("t", "c1"), idToolResult("t", "c2"), }} - eval, _ := (&ToolCallEvaluator{}).EvaluateRun(run) + eval, _ := (&ToolCallEvaluator{}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 2 { t.Fatalf("expected 2 neutral findings (orphan result + unanswered call), got %d: %+v", len(eval.Findings), eval.Findings) @@ -125,7 +126,7 @@ func TestToolCallEvaluator_IDMatchWinsOverName(t *testing.T) { idToolCall("a", "x"), idToolResult("b", "x"), }} - eval, _ := (&ToolCallEvaluator{}).EvaluateRun(run) + eval, _ := (&ToolCallEvaluator{}).EvaluateRun(context.Background(), run) if len(eval.Findings) != 0 { t.Fatalf("expected id match to pair across names, got %d: %+v", len(eval.Findings), eval.Findings) } @@ -143,7 +144,7 @@ func TestToolCallEvaluator_OrphanResult(t *testing.T) { } evaluator := &ToolCallEvaluator{} - eval, err := evaluator.EvaluateRun(run) + eval, err := evaluator.EvaluateRun(context.Background(), run) if err != nil { t.Fatalf("unexpected error in EvaluateRun: %v", err) } diff --git a/runner/runner.go b/runner/runner.go index 6132960..c9e015b 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -1,6 +1,7 @@ package runner import ( + "context" "os" "path/filepath" "runtime" @@ -39,7 +40,9 @@ func NewRunner(evals []evaluator.Evaluator) *Runner { // Run reads every .json file in dir and evaluates it. Files are processed // concurrently (bounded by the CPU count) but results are assembled in the // original directory order, so output is deterministic regardless of scheduling. -func (r *Runner) Run(dir string) (*Response, error) { +// ctx is propagated to every evaluator, so cancelling it (Ctrl+C, a CI timeout) +// aborts in-flight work rather than letting it run to completion. +func (r *Runner) Run(ctx context.Context, dir string) (*Response, error) { entries, err := os.ReadDir(dir) if err != nil { return nil, err @@ -72,7 +75,7 @@ func (r *Runner) Run(dir string) (*Response, error) { go func(i int, name string) { defer wg.Done() defer func() { <-sem }() - results[i] = r.processFile(dir, name) + results[i] = r.processFile(ctx, dir, name) }(i, name) } wg.Wait() @@ -86,9 +89,14 @@ func (r *Runner) Run(dir string) (*Response, error) { return &response, nil } -func (r *Runner) processFile(dir, name string) fileResult { +func (r *Runner) processFile(ctx context.Context, dir, name string) fileResult { var res fileResult + if err := ctx.Err(); err != nil { + res.errs = append(res.errs, FileError{File: name, Err: err}) + return res + } + fileBytes, err := os.ReadFile(filepath.Join(dir, name)) if err != nil { res.errs = append(res.errs, FileError{File: name, Err: err}) @@ -104,7 +112,7 @@ func (r *Runner) processFile(dir, name string) fileResult { return res } for _, judge := range r.evals { - eval, err := judge.EvaluateRun(run) + eval, err := judge.EvaluateRun(ctx, run) if err != nil { res.errs = append(res.errs, FileError{File: name, Err: err}) continue diff --git a/runner/runner_test.go b/runner/runner_test.go index d75a090..4feb727 100644 --- a/runner/runner_test.go +++ b/runner/runner_test.go @@ -1,6 +1,8 @@ package runner import ( + "context" + "errors" "fmt" "os" "path/filepath" @@ -13,7 +15,7 @@ func TestRunner_Run(t *testing.T) { evals := []evaluator.Evaluator{&evaluator.ToolCallEvaluator{}} runner := NewRunner(evals) - resp, err := runner.Run("../testdata/runs") + resp, err := runner.Run(context.Background(), "../testdata/runs") if err != nil { t.Fatalf("unexpected error in Run: %v", err) } @@ -37,6 +39,31 @@ func TestRunner_Run(t *testing.T) { } } +// TestRunner_Run_ContextCancelled asserts that an already-cancelled context +// short-circuits every file: no evaluations are produced and each eligible file +// surfaces a context.Canceled error instead of being evaluated. +func TestRunner_Run_ContextCancelled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + runner := NewRunner([]evaluator.Evaluator{&evaluator.ToolCallEvaluator{}}) + resp, err := runner.Run(ctx, "../testdata/runs") + if err != nil { + t.Fatalf("Run itself should not error on cancellation, got: %v", err) + } + if len(resp.Evaluations) != 0 { + t.Errorf("expected no evaluations under a cancelled context, got %d", len(resp.Evaluations)) + } + if len(resp.FileErrors) == 0 { + t.Fatal("expected file errors under a cancelled context, got none") + } + for _, fe := range resp.FileErrors { + if !errors.Is(fe.Err, context.Canceled) { + t.Errorf("file %s: expected context.Canceled, got %v", fe.File, fe.Err) + } + } +} + // TestRunner_Run_DeterministicOrder writes many valid runs whose ids follow the // sorted filename order, then runs several times. Files are processed // concurrently, so this asserts the assembled output still matches directory @@ -66,7 +93,7 @@ func TestRunner_Run_DeterministicOrder(t *testing.T) { runner := NewRunner([]evaluator.Evaluator{&evaluator.ToolCallEvaluator{}}) for attempt := 0; attempt < 5; attempt++ { - resp, err := runner.Run(dir) + resp, err := runner.Run(context.Background(), dir) if err != nil { t.Fatalf("unexpected error in Run: %v", err) } From b6cb7ef13003a01ad3e1f51b1e69c117b78d29d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jesus=20Nu=C3=B1ez?= Date: Fri, 7 Aug 2026 00:26:09 -0400 Subject: [PATCH 4/9] chore: stop tracking .idea and dedupe .gitignore Uncomment .idea/ so the JetBrains project directory is ignored, and remove the previously committed .idea files from the index. Also drop the duplicate *.so and .env entries the Python template added; both are still ignored by the Go template block above, so .env (which holds GEMINI_API_KEY) stays untracked. Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitignore | 4 +--- .idea/.gitignore | 10 ---------- .idea/copyright/profiles_settings.xml | 7 ------- .idea/git_toolbox_prj.xml | 15 --------------- .idea/go.imports.xml | 10 ---------- .idea/material_theme_project_new.xml | 18 ------------------ .idea/modules.xml | 8 -------- .idea/trazo.iml | 9 --------- .idea/vcs.xml | 12 ------------ 9 files changed, 1 insertion(+), 92 deletions(-) delete mode 100644 .idea/.gitignore delete mode 100644 .idea/copyright/profiles_settings.xml delete mode 100644 .idea/git_toolbox_prj.xml delete mode 100644 .idea/go.imports.xml delete mode 100644 .idea/material_theme_project_new.xml delete mode 100644 .idea/modules.xml delete mode 100644 .idea/trazo.iml delete mode 100644 .idea/vcs.xml diff --git a/.gitignore b/.gitignore index 7d71c9a..c4123ba 100644 --- a/.gitignore +++ b/.gitignore @@ -32,7 +32,6 @@ __pycache__/ *$py.class # C extensions -*.so # Distribution / packaging .Python @@ -150,7 +149,6 @@ celerybeat.pid *.sage.py # Environments -.env .venv env/ venv/ @@ -187,5 +185,5 @@ cython_debug/ # be found at https://github.com/github/gitignore/blob/main/Global/JetBrains.gitignore # and can be added to the global gitignore or merged into this file. For a more nuclear # option (not recommended) you can uncomment the following to ignore the entire idea folder. -#.idea/ +.idea/ diff --git a/.idea/.gitignore b/.idea/.gitignore deleted file mode 100644 index 30cf57e..0000000 --- a/.idea/.gitignore +++ /dev/null @@ -1,10 +0,0 @@ -# Default ignored files -/shelf/ -/workspace.xml -# Editor-based HTTP Client requests -/httpRequests/ -# Ignored default folder with query files -/queries/ -# Datasource local storage ignored files -/dataSources/ -/dataSources.local.xml diff --git a/.idea/copyright/profiles_settings.xml b/.idea/copyright/profiles_settings.xml deleted file mode 100644 index c803e3e..0000000 --- a/.idea/copyright/profiles_settings.xml +++ /dev/null @@ -1,7 +0,0 @@ - - - - - - - \ No newline at end of file diff --git a/.idea/git_toolbox_prj.xml b/.idea/git_toolbox_prj.xml deleted file mode 100644 index 02b915b..0000000 --- a/.idea/git_toolbox_prj.xml +++ /dev/null @@ -1,15 +0,0 @@ - - - - - - - \ No newline at end of file diff --git a/.idea/go.imports.xml b/.idea/go.imports.xml deleted file mode 100644 index 644cdf0..0000000 --- a/.idea/go.imports.xml +++ /dev/null @@ -1,10 +0,0 @@ - - - - - - \ No newline at end of file diff --git a/.idea/material_theme_project_new.xml b/.idea/material_theme_project_new.xml deleted file mode 100644 index 78c683f..0000000 --- a/.idea/material_theme_project_new.xml +++ /dev/null @@ -1,18 +0,0 @@ - - - - - - - \ No newline at end of file diff --git a/.idea/modules.xml b/.idea/modules.xml deleted file mode 100644 index 0510d3b..0000000 --- a/.idea/modules.xml +++ /dev/null @@ -1,8 +0,0 @@ - - - - - - - - \ No newline at end of file diff --git a/.idea/trazo.iml b/.idea/trazo.iml deleted file mode 100644 index 5e764c4..0000000 --- a/.idea/trazo.iml +++ /dev/null @@ -1,9 +0,0 @@ - - - - - - - - - \ No newline at end of file diff --git a/.idea/vcs.xml b/.idea/vcs.xml deleted file mode 100644 index 4c6280e..0000000 --- a/.idea/vcs.xml +++ /dev/null @@ -1,12 +0,0 @@ - - - - - - - - - - - - \ No newline at end of file From 94c01611172787d0680bc23ba2a84b01150a70d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jesus=20Nu=C3=B1ez?= Date: Fri, 7 Aug 2026 10:58:54 -0400 Subject: [PATCH 5/9] harden: publish JSON Schema for traces with cross-language conformance tests Add trajectory/trace.schema.json (draft 2020-12) as the strict external contract for the trace format, without adding any dependency to the Go core (still stdlib-only). Keep it honest against the source of truth from both sides: a Go test fails the build if the schema's type enum or per-type required fields drift from the trajectory constants, and a Python test validates every fixture and the emitter output against it. Document the two intentional gaps vs Run.Validate: the schema forbids unknown fields (loader is lenient) and cannot express cross-field temporal invariants (those stay Go-only). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../langgraph-reference/docs/trace-schema.md | 28 ++++ agents/langgraph-reference/requirements.txt | 2 + .../langgraph-reference/tests/test_schema.py | 96 ++++++++++++++ trajectory/schema_test.go | 109 ++++++++++++++++ trajectory/trace.schema.json | 123 ++++++++++++++++++ 5 files changed, 358 insertions(+) create mode 100644 agents/langgraph-reference/tests/test_schema.py create mode 100644 trajectory/schema_test.go create mode 100644 trajectory/trace.schema.json diff --git a/agents/langgraph-reference/docs/trace-schema.md b/agents/langgraph-reference/docs/trace-schema.md index 75c344c..30e5bee 100644 --- a/agents/langgraph-reference/docs/trace-schema.md +++ b/agents/langgraph-reference/docs/trace-schema.md @@ -11,6 +11,34 @@ description of the Go code, not a new contract. The source of truth is: If any statement here disagrees with the Go code, the Go code wins. Verified against the repo at commit level of branch `feature/0.0.1`. +## Machine-readable schema + +A JSON Schema (draft 2020-12) mirrors this document at +[`trajectory/trace.schema.json`](../../../trajectory/trace.schema.json). It is +derived from the Go types, not a competing source of truth: a Go test +(`trajectory/schema_test.go`) fails the build if the schema's step-type `enum` or +per-type required fields drift from the `trajectory` constants, and a Python test +(`tests/test_schema.py`) validates every committed fixture and the emitter output +against it. + +Two intentional gaps between the schema and `Run.Validate`: + +- The schema is **stricter** on unknown fields: it sets `additionalProperties: + false`, while the Go loader ignores unknown fields. The schema defines the + intended contract; the loader is lenient. +- The schema is **weaker** on cross-field temporal invariants: `endTime` not + before `startTime`, monotonic step timestamps, and steps within + `[startTime, endTime]` cannot be expressed in JSON Schema and are enforced only + by `Run.Validate` in Go. Passing the schema does not exempt a trace from + `Run.Validate`. + +Validate a file against it with any draft 2020-12 validator, e.g. from the +Python side: + +```bash +python -c "import json,jsonschema; s=json.load(open('trajectory/trace.schema.json')); jsonschema.validate(json.load(open('agents/langgraph-reference/docs/sample-trace.json')), s)" +``` + ## File layout - One run per file. One JSON object at the top level. diff --git a/agents/langgraph-reference/requirements.txt b/agents/langgraph-reference/requirements.txt index b31efe7..ac86295 100644 --- a/agents/langgraph-reference/requirements.txt +++ b/agents/langgraph-reference/requirements.txt @@ -6,3 +6,5 @@ langchain-google-genai==4.3.2 # Test tooling. pytest==8.3.4 +# Validates emitter output and fixtures against trajectory/trace.schema.json. +jsonschema==4.26.0 diff --git a/agents/langgraph-reference/tests/test_schema.py b/agents/langgraph-reference/tests/test_schema.py new file mode 100644 index 0000000..15f2259 --- /dev/null +++ b/agents/langgraph-reference/tests/test_schema.py @@ -0,0 +1,96 @@ +"""Schema conformance: fixtures and emitter output validate against the published +JSON Schema (trajectory/trace.schema.json). + +The Go types remain the source of truth; this proves the published schema agrees +with the real traces the core consumes and the ones the emitter produces. Skipped +automatically if jsonschema is not installed. +""" + +from __future__ import annotations + +import json +from datetime import datetime, timezone +from pathlib import Path + +import pytest + +jsonschema = pytest.importorskip("jsonschema") + +from trazo_emitter import TraceRecorder + +REPO_ROOT = Path(__file__).resolve().parents[3] +SCHEMA_PATH = REPO_ROOT / "trajectory" / "trace.schema.json" + +# Structurally valid traces. Some of these carry behavior that evaluators flag +# (orphan results, tool errors); that is an evaluator concern, not a schema one, +# so they must still pass structural validation here. +VALID_FIXTURES = [ + "agents/langgraph-reference/docs/sample-trace.json", + "testdata/runs/sample_run.json", + "testdata/runs/sample_run_ok.json", + "testdata/sample_run_orphan_result.json", + "testdata/sample_run_missing_result.json", + "testdata/ci/clean/triage_clean.json", + "testdata/ci/failing/tool_error.json", +] + + +@pytest.fixture(scope="module") +def validator() -> "jsonschema.protocols.Validator": + schema = json.loads(SCHEMA_PATH.read_text(encoding="utf-8")) + cls = jsonschema.validators.validator_for(schema) + cls.check_schema(schema) # the schema itself must be a valid JSON Schema + return cls(schema, format_checker=cls.FORMAT_CHECKER) + + +def _dt(second: int) -> datetime: + return datetime(2026, 8, 6, 10, 0, second, tzinfo=timezone.utc) + + +@pytest.mark.parametrize("rel_path", VALID_FIXTURES) +def test_valid_fixtures_conform(validator, rel_path: str) -> None: + trace = json.loads((REPO_ROOT / rel_path).read_text(encoding="utf-8")) + validator.validate(trace) # raises ValidationError on failure + + +def test_invalid_fixture_is_rejected(validator) -> None: + # invalid_run.json has an empty agent, an unknown "teleport" step type, and a + # tool_call missing its tool field. Each violates the schema. + trace = json.loads((REPO_ROOT / "testdata/runs/invalid_run.json").read_text(encoding="utf-8")) + errors = list(validator.iter_errors(trace)) + assert errors, "expected invalid_run.json to be rejected by the schema" + + +def test_emitter_output_conforms(validator, tmp_path) -> None: + rec = TraceRecorder("run-schema-check", "github-triage", "0.0.1", start_time=_dt(0)) + rec.record_node_transition("start", timestamp=_dt(0)) + rec.record_llm_call("gemini-2.5-flash", output="calling a tool", timestamp=_dt(1)) + call = rec.record_tool_call("fetch_issues", input={"repo": "golang/example"}, timestamp=_dt(2)) + rec.record_tool_result(call, output={"count": 0}, timestamp=_dt(3)) + rec.record_node_transition("end", timestamp=_dt(4)) + rec.flush(tmp_path, end_time=_dt(5)) + + written = list(tmp_path.glob("*.json")) + assert written, "emitter wrote no trace file" + for path in written: + validator.validate(json.loads(path.read_text(encoding="utf-8"))) + + +@pytest.mark.parametrize( + "mutate", + [ + pytest.param(lambda s: s.__setitem__("agent", ""), id="empty-agent"), + pytest.param(lambda s: s["steps"][0].__setitem__("type", "teleport"), id="unknown-type"), + pytest.param(lambda s: s["steps"][0].__setitem__("cost", -1), id="negative-cost"), + pytest.param(lambda s: s["steps"][0].__setitem__("surprise", True), id="unknown-field"), + pytest.param(lambda s: s["steps"][0].pop("node"), id="node-transition-without-node"), + ], +) +def test_schema_rejects_mutations(validator, mutate) -> None: + # Start from a known-good trace, break one invariant, expect rejection. + trace = json.loads( + (REPO_ROOT / "agents/langgraph-reference/docs/sample-trace.json").read_text(encoding="utf-8") + ) + mutate(trace) + errors = list(validator.iter_errors(trace)) + assert errors, "expected the mutated trace to be rejected" diff --git a/trajectory/schema_test.go b/trajectory/schema_test.go new file mode 100644 index 0000000..3828621 --- /dev/null +++ b/trajectory/schema_test.go @@ -0,0 +1,109 @@ +package trajectory + +import ( + "encoding/json" + "os" + "sort" + "testing" +) + +// schemaDoc captures only the parts of trace.schema.json this test asserts on: +// the step type enum and the per-type required-field rules encoded as if/then. +type schemaDoc struct { + Defs struct { + Step struct { + Properties struct { + Type struct { + Enum []string `json:"enum"` + } `json:"type"` + } `json:"properties"` + AllOf []struct { + If struct { + Properties struct { + Type struct { + Const string `json:"const"` + } `json:"type"` + } `json:"properties"` + } `json:"if"` + Then struct { + Required []string `json:"required"` + } `json:"then"` + } `json:"allOf"` + } `json:"step"` + } `json:"$defs"` +} + +func loadSchemaDoc(t *testing.T) schemaDoc { + t.Helper() + data, err := os.ReadFile("trace.schema.json") + if err != nil { + t.Fatalf("read trace.schema.json: %v", err) + } + var doc schemaDoc + if err := json.Unmarshal(data, &doc); err != nil { + t.Fatalf("parse trace.schema.json: %v", err) + } + return doc +} + +// TestSchemaTypeEnumMatchesConstants guards against the published schema drifting +// away from the Go StepType constants (the source of truth). If a StepType is +// added or renamed without touching the schema, this fails. +func TestSchemaTypeEnumMatchesConstants(t *testing.T) { + doc := loadSchemaDoc(t) + + want := []string{ + string(StepTypeCallLLM), + string(StepTypeToolCall), + string(StepTypeToolResult), + string(StepTypeNodeTransition), + } + got := append([]string(nil), doc.Defs.Step.Properties.Type.Enum...) + + sort.Strings(want) + sort.Strings(got) + + if len(got) != len(want) { + t.Fatalf("schema type enum has %d values %v, want %d %v", len(got), got, len(want), want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("schema type enum = %v, want %v", got, want) + } + } +} + +// TestSchemaRequiredFieldsMatchValidate guards the per-type required-field rules: +// the schema's if/then blocks must encode the same "type -> required field" map +// that Run.Validate enforces. Keep this in sync with the switch in Validate. +func TestSchemaRequiredFieldsMatchValidate(t *testing.T) { + doc := loadSchemaDoc(t) + + want := map[string]string{ + string(StepTypeCallLLM): "llm", + string(StepTypeToolCall): "tool", + string(StepTypeToolResult): "tool", + string(StepTypeNodeTransition): "node", + } + + got := map[string]string{} + for _, rule := range doc.Defs.Step.AllOf { + typ := rule.If.Properties.Type.Const + if typ == "" { + t.Fatalf("schema allOf entry with no type const: %+v", rule) + } + if len(rule.Then.Required) != 1 { + t.Fatalf("schema allOf for %q must require exactly one field, got %v", typ, rule.Then.Required) + } + got[typ] = rule.Then.Required[0] + } + + if len(got) != len(want) { + t.Fatalf("schema encodes %d per-type rules %v, want %d %v", len(got), got, len(want), want) + } + for typ, field := range want { + if got[typ] != field { + t.Fatalf("schema requires %q for %q, want %q", got[typ], typ, field) + } + } +} diff --git a/trajectory/trace.schema.json b/trajectory/trace.schema.json new file mode 100644 index 0000000..727b7a7 --- /dev/null +++ b/trajectory/trace.schema.json @@ -0,0 +1,123 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/Cro22/trazo/trajectory/trace.schema.json", + "title": "Trazo trace", + "description": "One agent run per file, as a single JSON object. The source of truth for this format is the Go code (trajectory/steps.go for the shape, trajectory/validate.go for the acceptance gate); this schema is the derived, strict external contract. It is deliberately stricter than the Go loader in one way: the loader ignores unknown fields, this schema forbids them (additionalProperties: false). Some invariants that Run.Validate enforces are cross-field and cannot be expressed in JSON Schema, so they are Go-only: endTime not before startTime, step timestamps monotonic non-decreasing, and every step timestamp within [startTime, endTime]. A document that passes this schema still has to pass Run.Validate.", + "type": "object", + "additionalProperties": false, + "required": ["id", "agent", "startTime", "endTime"], + "properties": { + "id": { + "type": "string", + "minLength": 1, + "description": "Run identifier. Non-empty. Surfaces as runId in evaluator output." + }, + "agent": { + "type": "string", + "minLength": 1, + "description": "Logical agent name. Non-empty." + }, + "version": { + "type": "string", + "description": "Schema/trace version. Not enforced by Run.Validate, but present in every emitted trace by convention; include it." + }, + "startTime": { + "type": "string", + "format": "date-time", + "description": "RFC3339 timestamp. Must be present and non-zero." + }, + "endTime": { + "type": "string", + "format": "date-time", + "description": "RFC3339 timestamp. Must be present, non-zero, and not before startTime (cross-field check enforced by Go)." + }, + "steps": { + "type": "array", + "description": "Ordered steps of the run. May be empty (a run with no steps passes validation but has nothing to evaluate). An absent steps field deserializes to an empty slice in Go.", + "items": {"$ref": "#/$defs/step"} + } + }, + "$defs": { + "step": { + "type": "object", + "additionalProperties": false, + "required": ["type", "timestamp"], + "properties": { + "type": { + "type": "string", + "enum": ["llm_call", "tool_call", "tool_result", "node_transition"], + "description": "Discriminates the step. Each value requires its own field (see allOf below)." + }, + "timestamp": { + "type": "string", + "format": "date-time", + "description": "RFC3339 timestamp. Required and non-zero for every step." + }, + "llm": { + "type": "string", + "description": "Model identifier. Required (non-empty) when type is llm_call; omit otherwise." + }, + "tool": { + "type": "string", + "description": "Tool name. Required (non-empty) when type is tool_call or tool_result. Fallback pairing key when toolCallId is absent." + }, + "toolCallId": { + "type": "string", + "description": "Correlates a tool_result with its tool_call. Authoritative for pairing when present, in which case no name/order fallback is used." + }, + "node": { + "type": "string", + "description": "Graph node name. Required (non-empty) when type is node_transition." + }, + "input": { + "description": "Opaque payload. Any JSON value; the core stores it verbatim and never parses it." + }, + "output": { + "description": "Opaque payload. Any JSON value; the core stores it verbatim and never parses it." + }, + "cost": { + "type": "number", + "minimum": 0, + "description": "USD cost of the step. Non-negative. Omitted when zero." + }, + "inputTokens": { + "type": "integer", + "minimum": 0, + "description": "Prompt tokens. Non-negative. Omitted when zero." + }, + "outputTokens": { + "type": "integer", + "minimum": 0, + "description": "Completion tokens. Non-negative. Omitted when zero." + }, + "durationMs": { + "type": "integer", + "minimum": 0, + "description": "Step duration in milliseconds. Non-negative. Serialized on every step, including zero." + }, + "error": { + "type": "string", + "description": "Non-empty means the step failed. On a tool_result it drives a bad finding." + } + }, + "allOf": [ + { + "if": {"properties": {"type": {"const": "llm_call"}}, "required": ["type"]}, + "then": {"required": ["llm"], "properties": {"llm": {"minLength": 1}}} + }, + { + "if": {"properties": {"type": {"const": "tool_call"}}, "required": ["type"]}, + "then": {"required": ["tool"], "properties": {"tool": {"minLength": 1}}} + }, + { + "if": {"properties": {"type": {"const": "tool_result"}}, "required": ["type"]}, + "then": {"required": ["tool"], "properties": {"tool": {"minLength": 1}}} + }, + { + "if": {"properties": {"type": {"const": "node_transition"}}, "required": ["type"]}, + "then": {"required": ["node"], "properties": {"node": {"minLength": 1}}} + } + ] + } + } +} From 6f1f5f5e74c894b0f738e142517a6226581a8de0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jesus=20Nu=C3=B1ez?= Date: Fri, 7 Aug 2026 11:05:55 -0400 Subject: [PATCH 6/9] harden: centralize CLI output in report package with golden tests Move the text and JSON renderers out of cmd/trazo/main.go into the report package alongside Markdown, so all three output formats live in one testable place and main.go just selects one. Behavior is unchanged. Add golden tests covering all three formats over a single fixture that exercises scored findings, run-level findings, Markdown escaping, empty findings, and file errors. Pin golden files to LF via .gitattributes so the byte-for-byte comparison holds on Windows (core.autocrlf=true) and CI alike. Also rename Evaluator.go to evaluator.go to match Go file-naming convention. Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitattributes | 4 ++ cmd/trazo/main.go | 51 ++------------ evaluator/{Evaluator.go => evaluator.go} | 0 report/golden_test.go | 88 ++++++++++++++++++++++++ report/json.go | 40 +++++++++++ report/testdata/json.golden | 43 ++++++++++++ report/testdata/markdown.golden | 20 ++++++ report/testdata/text.golden | 8 +++ report/text.go | 25 +++++++ 9 files changed, 234 insertions(+), 45 deletions(-) create mode 100644 .gitattributes rename evaluator/{Evaluator.go => evaluator.go} (100%) create mode 100644 report/golden_test.go create mode 100644 report/json.go create mode 100644 report/testdata/json.golden create mode 100644 report/testdata/markdown.golden create mode 100644 report/testdata/text.golden create mode 100644 report/text.go diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..be56e13 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,4 @@ +# Golden files are compared byte-for-byte against LF output generated by the +# renderers, so they must stay LF regardless of the checkout platform (the repo +# is developed on Windows with core.autocrlf=true). +report/testdata/*.golden text eol=lf diff --git a/cmd/trazo/main.go b/cmd/trazo/main.go index b1a3bb7..ebacd00 100644 --- a/cmd/trazo/main.go +++ b/cmd/trazo/main.go @@ -2,7 +2,6 @@ package main import ( "context" - "encoding/json" "flag" "fmt" "log" @@ -24,16 +23,6 @@ const ( exitFileError = 2 ) -type fileErrorJSON struct { - File string `json:"file"` - Error string `json:"error"` -} - -type responseJSON struct { - Evaluations []*evaluator.Evaluation `json:"evaluations"` - FileErrors []fileErrorJSON `json:"fileErrors"` -} - func main() { dir := flag.String("dir", "./testdata/runs", "directory containing run JSON files") asJSON := flag.Bool("json", false, "print results as JSON (alias for -format json)") @@ -84,11 +73,15 @@ func main() { } switch out { case "json": - printJSON(resp) + s, err := report.JSON(resp) + if err != nil { + log.Fatalf("Error encoding JSON: %v", err) + } + fmt.Println(s) case "md", "markdown": fmt.Print(report.Markdown(resp)) case "text": - printText(resp) + fmt.Print(report.Text(resp)) default: log.Fatalf("unknown -format %q (want text, json, or md)", out) } @@ -96,38 +89,6 @@ func main() { os.Exit(exitCode(resp)) } -func printText(resp *runner.Response) { - for _, findings := range resp.Evaluations { - fmt.Printf("RunID %s. Findings: %d Evaluator: %s\n", findings.RunID, len(findings.Findings), findings.EvaluatorName) - for _, s := range findings.Findings { - fmt.Printf("Step %d: Comment: %s, Score: %f, Judgment: %s\n", s.StepIndex, s.Comment, s.Score, s.Judgment) - } - } - - for _, fe := range resp.FileErrors { - fmt.Printf("File Error: %s → %v\n", fe.File, fe.Err) - } -} - -func printJSON(resp *runner.Response) { - out := responseJSON{ - Evaluations: resp.Evaluations, - FileErrors: []fileErrorJSON{}, - } - if out.Evaluations == nil { - out.Evaluations = []*evaluator.Evaluation{} - } - for _, fe := range resp.FileErrors { - out.FileErrors = append(out.FileErrors, fileErrorJSON{File: fe.File, Error: fe.Err.Error()}) - } - - data, err := json.MarshalIndent(out, "", " ") - if err != nil { - log.Fatalf("Error encoding JSON: %v", err) - } - fmt.Println(string(data)) -} - // splitCSV parses a comma-separated flag value into a trimmed, non-empty slice, // returning nil for an empty value so the evaluator falls back to its default. func splitCSV(s string) []string { diff --git a/evaluator/Evaluator.go b/evaluator/evaluator.go similarity index 100% rename from evaluator/Evaluator.go rename to evaluator/evaluator.go diff --git a/report/golden_test.go b/report/golden_test.go new file mode 100644 index 0000000..60d2e51 --- /dev/null +++ b/report/golden_test.go @@ -0,0 +1,88 @@ +package report + +import ( + "errors" + "flag" + "os" + "path/filepath" + "testing" + + "github.com/Cro22/trazo/evaluator" + "github.com/Cro22/trazo/runner" +) + +// update regenerates the golden files instead of comparing against them: +// +// go test ./report -run TestGolden -update +// +// Review the diff before committing; a golden change is an intentional change to +// user-facing output. +var update = flag.Bool("update", false, "update golden files") + +// goldenFixture is a single Response that exercises the interesting cases shared +// by all three renderers: multiple evaluators over one run, a scored finding, a +// run-level finding (step -1), a comment with a pipe and a newline (Markdown +// escaping), and a file error. +func goldenFixture() *runner.Response { + return &runner.Response{ + Evaluations: []*evaluator.Evaluation{ + { + EvaluatorName: "tool_calls", + RunID: "run-1", + Findings: []evaluator.Finding{ + {StepIndex: 3, Judgment: evaluator.JudgmentBad, Comment: "tool postgres_query fails: connection refused"}, + {StepIndex: 2, Judgment: evaluator.JudgmentNeutral, Comment: "tool_call postgres_query has no matching result"}, + }, + }, + { + EvaluatorName: "cost_latency", + RunID: "run-1", + Findings: []evaluator.Finding{ + {StepIndex: -1, Judgment: evaluator.JudgmentNeutral, Score: 1.5, Comment: "run cost 1.50 over budget|limit\nsecond line"}, + }, + }, + { + EvaluatorName: "tool_calls", + RunID: "run-ok", + Findings: []evaluator.Finding{}, + }, + }, + FileErrors: []runner.FileError{ + {File: "broken.json", Err: errors.New("unexpected end of JSON input")}, + }, + } +} + +func TestGolden(t *testing.T) { + resp := goldenFixture() + + jsonOut, err := JSON(resp) + if err != nil { + t.Fatalf("JSON: %v", err) + } + + cases := map[string]string{ + "text.golden": Text(resp), + "json.golden": jsonOut, + "markdown.golden": Markdown(resp), + } + + for name, got := range cases { + t.Run(name, func(t *testing.T) { + path := filepath.Join("testdata", name) + if *update { + if err := os.WriteFile(path, []byte(got), 0o644); err != nil { + t.Fatalf("write golden %s: %v", path, err) + } + return + } + want, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read golden %s: %v (run with -update to create it)", path, err) + } + if got != string(want) { + t.Errorf("%s mismatch (run with -update to accept):\n--- got ---\n%s\n--- want ---\n%s", name, got, want) + } + }) + } +} diff --git a/report/json.go b/report/json.go new file mode 100644 index 0000000..af3c306 --- /dev/null +++ b/report/json.go @@ -0,0 +1,40 @@ +package report + +import ( + "encoding/json" + + "github.com/Cro22/trazo/evaluator" + "github.com/Cro22/trazo/runner" +) + +type fileErrorJSON struct { + File string `json:"file"` + Error string `json:"error"` +} + +type responseJSON struct { + Evaluations []*evaluator.Evaluation `json:"evaluations"` + FileErrors []fileErrorJSON `json:"fileErrors"` +} + +// JSON renders the evaluation results as indented JSON. Empty slices serialize as +// [] rather than null so consumers can index without a nil check, and errors are +// flattened to strings. +func JSON(resp *runner.Response) (string, error) { + out := responseJSON{ + Evaluations: resp.Evaluations, + FileErrors: []fileErrorJSON{}, + } + if out.Evaluations == nil { + out.Evaluations = []*evaluator.Evaluation{} + } + for _, fe := range resp.FileErrors { + out.FileErrors = append(out.FileErrors, fileErrorJSON{File: fe.File, Error: fe.Err.Error()}) + } + + data, err := json.MarshalIndent(out, "", " ") + if err != nil { + return "", err + } + return string(data), nil +} diff --git a/report/testdata/json.golden b/report/testdata/json.golden new file mode 100644 index 0000000..a064bf2 --- /dev/null +++ b/report/testdata/json.golden @@ -0,0 +1,43 @@ +{ + "evaluations": [ + { + "evaluatorName": "tool_calls", + "runId": "run-1", + "findings": [ + { + "stepIndex": 3, + "judgment": "bad", + "comment": "tool postgres_query fails: connection refused" + }, + { + "stepIndex": 2, + "judgment": "neutral", + "comment": "tool_call postgres_query has no matching result" + } + ] + }, + { + "evaluatorName": "cost_latency", + "runId": "run-1", + "findings": [ + { + "stepIndex": -1, + "judgment": "neutral", + "score": 1.5, + "comment": "run cost 1.50 over budget|limit\nsecond line" + } + ] + }, + { + "evaluatorName": "tool_calls", + "runId": "run-ok", + "findings": [] + } + ], + "fileErrors": [ + { + "file": "broken.json", + "error": "unexpected end of JSON input" + } + ] +} \ No newline at end of file diff --git a/report/testdata/markdown.golden b/report/testdata/markdown.golden new file mode 100644 index 0000000..5775770 --- /dev/null +++ b/report/testdata/markdown.golden @@ -0,0 +1,20 @@ +# trazo evaluation report + +**Verdict: FAIL** + +- Evaluations: 3 +- Findings: 3 (bad 1, neutral 2, good 0) +- File errors: 1 + +## Findings + +| Run | Evaluator | Step | Severity | Comment | +|-----|-----------|------|----------|---------| +| run-1 | tool_calls | 3 | bad | tool postgres_query fails: connection refused | +| run-1 | tool_calls | 2 | neutral | tool_call postgres_query has no matching result | +| run-1 | cost_latency | run | neutral | run cost 1.50 over budget\|limit; second line | + +## File errors + +- `broken.json`: unexpected end of JSON input + diff --git a/report/testdata/text.golden b/report/testdata/text.golden new file mode 100644 index 0000000..94bb8ea --- /dev/null +++ b/report/testdata/text.golden @@ -0,0 +1,8 @@ +RunID run-1. Findings: 2 Evaluator: tool_calls +Step 3: Comment: tool postgres_query fails: connection refused, Score: 0.000000, Judgment: bad +Step 2: Comment: tool_call postgres_query has no matching result, Score: 0.000000, Judgment: neutral +RunID run-1. Findings: 1 Evaluator: cost_latency +Step -1: Comment: run cost 1.50 over budget|limit +second line, Score: 1.500000, Judgment: neutral +RunID run-ok. Findings: 0 Evaluator: tool_calls +File Error: broken.json → unexpected end of JSON input diff --git a/report/text.go b/report/text.go new file mode 100644 index 0000000..8d96c3d --- /dev/null +++ b/report/text.go @@ -0,0 +1,25 @@ +package report + +import ( + "fmt" + "strings" + + "github.com/Cro22/trazo/runner" +) + +// Text renders the evaluation results as the plain-text CLI format: one header +// line per evaluation, one line per finding, then any file errors. It is the +// default CLI output and is kept deliberately terse and greppable. +func Text(resp *runner.Response) string { + var b strings.Builder + for _, e := range resp.Evaluations { + fmt.Fprintf(&b, "RunID %s. Findings: %d Evaluator: %s\n", e.RunID, len(e.Findings), e.EvaluatorName) + for _, f := range e.Findings { + fmt.Fprintf(&b, "Step %d: Comment: %s, Score: %f, Judgment: %s\n", f.StepIndex, f.Comment, f.Score, f.Judgment) + } + } + for _, fe := range resp.FileErrors { + fmt.Fprintf(&b, "File Error: %s → %v\n", fe.File, fe.Err) + } + return b.String() +} From cc89f0203d453a0c89ec59c008a4ba5a1bafd7f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jesus=20Nu=C3=B1ez?= Date: Fri, 7 Aug 2026 12:28:34 -0400 Subject: [PATCH 7/9] harden: CLI accepts a single file, recursion, and a validate-only mode Rework the trazo CLI beyond a single -dir scan: - Accept a positional PATH (a trace file or a directory); -dir stays as the fallback default. - Add -recursive to descend into subdirectories. - Add -validate for a structure-only pass (loads and validates, skips evaluators) with a concise "N valid, M invalid" summary. - Validate -format up front (fail fast) and add a proper -help/usage block. Refactor the runner to separate file discovery (CollectFiles) from execution (RunFiles); Run(dir) is now a thin wrapper. FileError carries the path as read so errors stay unambiguous across subdirectories. Tests cover recursive and non-recursive discovery and the validate summary. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/trazo/main.go | 147 ++++++++++++++++++++++++++++++++-------- report/validate.go | 28 ++++++++ report/validate_test.go | 37 ++++++++++ runner/runner.go | 83 +++++++++++++++++------ runner/runner_test.go | 61 +++++++++++++++-- 5 files changed, 299 insertions(+), 57 deletions(-) create mode 100644 report/validate.go create mode 100644 report/validate_test.go diff --git a/cmd/trazo/main.go b/cmd/trazo/main.go index ebacd00..8a4c387 100644 --- a/cmd/trazo/main.go +++ b/cmd/trazo/main.go @@ -24,7 +24,12 @@ const ( ) func main() { - dir := flag.String("dir", "./testdata/runs", "directory containing run JSON files") + log.SetFlags(0) + log.SetPrefix("trazo: ") + + dir := flag.String("dir", "./testdata/runs", "directory of traces to scan when no PATH is given") + recursive := flag.Bool("recursive", false, "descend into subdirectories when PATH is a directory") + validate := flag.Bool("validate", false, "only check that traces load and pass structural validation; skip evaluators") asJSON := flag.Bool("json", false, "print results as JSON (alias for -format json)") format := flag.String("format", "text", "output format: text, json, or md") @@ -35,58 +40,140 @@ func main() { maxRunLatencyMs := flag.Int64("max-run-latency-ms", evaluator.DefaultMaxRunLatencyMs, "latency: max ms per run") terminalNodes := flag.String("terminal-nodes", "", "node: comma-separated terminal node names (default end,__end__,finish,done)") llmJudge := flag.Bool("llm-judge", false, "enable the LLM-as-judge evaluator (needs GEMINI_API_KEY)") - judgeModel := flag.String("judge-model", evaluator.DefaultJudgeModel, "model for --llm-judge") + judgeModel := flag.String("judge-model", evaluator.DefaultJudgeModel, "model for -llm-judge") + + flag.Usage = usage flag.Parse() - evaluators := []evaluator.Evaluator{ - &evaluator.ToolCallEvaluator{}, - &evaluator.LoopEvaluator{MaxRepeats: *maxRepeats}, - &evaluator.CostLatencyEvaluator{ - MaxStepCost: *maxStepCost, - MaxStepLatencyMs: *maxStepLatencyMs, - MaxRunCost: *maxRunCost, - MaxRunLatencyMs: *maxRunLatencyMs, - }, - &evaluator.NodeTransitionEvaluator{TerminalNodes: splitCSV(*terminalNodes)}, + if flag.NArg() > 1 { + log.Printf("at most one PATH may be given, got %d", flag.NArg()) + flag.Usage() + os.Exit(exitFileError) } - if *llmJudge { - client, err := evaluator.NewGeminiClient(*judgeModel) - if err != nil { - log.Fatalf("llm-judge: %v", err) - } - evaluators = append(evaluators, &evaluator.LLMJudgeEvaluator{Client: client}) + + out := *format + if *asJSON { + out = "json" + } + if out != "text" && out != "json" && out != "md" && out != "markdown" { + log.Fatalf("unknown -format %q (want text, json, or md)", out) + } + + // PATH (positional) takes precedence over -dir; -dir is the fallback default. + path := *dir + if flag.NArg() == 1 { + path = flag.Arg(0) } + files, err := resolveFiles(path, *recursive) + if err != nil { + log.Fatalf("%v", err) + } + if len(files) == 0 { + log.Printf("no .json traces found under %q", path) + } + + evaluators := buildEvaluators(*validate, evaluatorConfig{ + maxRepeats: *maxRepeats, + maxStepCost: *maxStepCost, + maxStepLatencyMs: *maxStepLatencyMs, + maxRunCost: *maxRunCost, + maxRunLatencyMs: *maxRunLatencyMs, + terminalNodes: splitCSV(*terminalNodes), + llmJudge: *llmJudge, + judgeModel: *judgeModel, + }) + // Cancel in-flight evaluation on Ctrl+C (SIGINT) or SIGTERM so a long run, // notably one using the network-bound LLM judge, stops promptly. ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt) defer stop() - resp, err := runner.NewRunner(evaluators).Run(ctx, *dir) + resp := runner.NewRunner(evaluators).RunFiles(ctx, files) + + render(out, *validate, resp, len(files)) + os.Exit(exitCode(resp)) +} + +func usage() { + w := flag.CommandLine.Output() + fmt.Fprintf(w, "trazo evaluates agent trace files (trazo JSON format) and reports findings.\n\n") + fmt.Fprintf(w, "Usage:\n trazo [flags] [PATH]\n\n") + fmt.Fprintf(w, "PATH is a single trace file or a directory of .json traces. If omitted, -dir is scanned.\n\n") + fmt.Fprintf(w, "Flags:\n") + flag.PrintDefaults() + fmt.Fprintf(w, "\nExit codes: 0 clean, 1 a bad finding, 2 a file error or invalid trace.\n") +} + +// resolveFiles turns a PATH into the concrete list of trace files to evaluate: a +// single file is used as-is, a directory is scanned (recursively when asked). +func resolveFiles(path string, recursive bool) ([]string, error) { + info, err := os.Stat(path) if err != nil { - log.Fatalf("Error running: %v", err) + return nil, err } - - out := *format - if *asJSON { - out = "json" + if info.IsDir() { + return runner.CollectFiles(path, recursive) } + return []string{path}, nil +} + +func render(out string, validate bool, resp *runner.Response, total int) { switch out { case "json": s, err := report.JSON(resp) if err != nil { - log.Fatalf("Error encoding JSON: %v", err) + log.Fatalf("encoding JSON: %v", err) } fmt.Println(s) case "md", "markdown": fmt.Print(report.Markdown(resp)) - case "text": - fmt.Print(report.Text(resp)) - default: - log.Fatalf("unknown -format %q (want text, json, or md)", out) + default: // text + if validate { + fmt.Print(report.ValidateSummary(resp, total)) + } else { + fmt.Print(report.Text(resp)) + } } +} - os.Exit(exitCode(resp)) +type evaluatorConfig struct { + maxRepeats int + maxStepCost float64 + maxStepLatencyMs int64 + maxRunCost float64 + maxRunLatencyMs int64 + terminalNodes []string + llmJudge bool + judgeModel string +} + +// buildEvaluators assembles the evaluator set. In validate-only mode it returns +// none, so the runner just loads and structurally validates each file. The LLM +// judge is opt-in and constructed last because it can fail (missing API key). +func buildEvaluators(validate bool, cfg evaluatorConfig) []evaluator.Evaluator { + if validate { + return nil + } + evaluators := []evaluator.Evaluator{ + &evaluator.ToolCallEvaluator{}, + &evaluator.LoopEvaluator{MaxRepeats: cfg.maxRepeats}, + &evaluator.CostLatencyEvaluator{ + MaxStepCost: cfg.maxStepCost, + MaxStepLatencyMs: cfg.maxStepLatencyMs, + MaxRunCost: cfg.maxRunCost, + MaxRunLatencyMs: cfg.maxRunLatencyMs, + }, + &evaluator.NodeTransitionEvaluator{TerminalNodes: cfg.terminalNodes}, + } + if cfg.llmJudge { + client, err := evaluator.NewGeminiClient(cfg.judgeModel) + if err != nil { + log.Fatalf("llm-judge: %v", err) + } + evaluators = append(evaluators, &evaluator.LLMJudgeEvaluator{Client: client}) + } + return evaluators } // splitCSV parses a comma-separated flag value into a trimmed, non-empty slice, diff --git a/report/validate.go b/report/validate.go new file mode 100644 index 0000000..ba44479 --- /dev/null +++ b/report/validate.go @@ -0,0 +1,28 @@ +package report + +import ( + "fmt" + "strings" + + "github.com/Cro22/trazo/runner" +) + +// ValidateSummary renders the outcome of a validate-only run: how many trace +// files were checked, how many passed structural validation, and the details of +// each that failed. total is the number of files considered, which the runner +// Response alone does not carry (valid files produce no evaluations in this +// mode). +func ValidateSummary(resp *runner.Response, total int) string { + invalid := len(resp.FileErrors) + valid := total - invalid + if valid < 0 { + valid = 0 + } + + var b strings.Builder + fmt.Fprintf(&b, "Validated %d file(s): %d valid, %d invalid.\n", total, valid, invalid) + for _, fe := range resp.FileErrors { + fmt.Fprintf(&b, " %s: %s\n", fe.File, cell(fe.Err.Error())) + } + return b.String() +} diff --git a/report/validate_test.go b/report/validate_test.go new file mode 100644 index 0000000..5a329be --- /dev/null +++ b/report/validate_test.go @@ -0,0 +1,37 @@ +package report + +import ( + "errors" + "strings" + "testing" + + "github.com/Cro22/trazo/runner" +) + +func TestValidateSummary_MixedValidInvalid(t *testing.T) { + resp := &runner.Response{ + FileErrors: []runner.FileError{ + {File: "testdata/broken.json", Err: errors.New("unexpected end of JSON input")}, + {File: "testdata/invalid.json", Err: errors.New("run: agent is empty\nstep 0: tool_call without tool")}, + }, + } + got := ValidateSummary(resp, 5) + + if !strings.HasPrefix(got, "Validated 5 file(s): 3 valid, 2 invalid.\n") { + t.Errorf("unexpected summary line:\n%s", got) + } + if !strings.Contains(got, "testdata/broken.json: unexpected end of JSON input") { + t.Errorf("missing broken.json line:\n%s", got) + } + // Multi-line validation errors are flattened onto one line for readability. + if !strings.Contains(got, "run: agent is empty; step 0: tool_call without tool") { + t.Errorf("expected flattened error line:\n%s", got) + } +} + +func TestValidateSummary_AllValid(t *testing.T) { + got := ValidateSummary(&runner.Response{}, 3) + if got != "Validated 3 file(s): 3 valid, 0 invalid.\n" { + t.Errorf("unexpected summary: %q", got) + } +} diff --git a/runner/runner.go b/runner/runner.go index c9e015b..56bb0a1 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -2,6 +2,7 @@ package runner import ( "context" + "io/fs" "os" "path/filepath" "runtime" @@ -37,18 +38,36 @@ func NewRunner(evals []evaluator.Evaluator) *Runner { return &Runner{evals: evals} } -// Run reads every .json file in dir and evaluates it. Files are processed -// concurrently (bounded by the CPU count) but results are assembled in the -// original directory order, so output is deterministic regardless of scheduling. -// ctx is propagated to every evaluator, so cancelling it (Ctrl+C, a CI timeout) -// aborts in-flight work rather than letting it run to completion. -func (r *Runner) Run(ctx context.Context, dir string) (*Response, error) { - entries, err := os.ReadDir(dir) +// CollectFiles returns the .json files under root as paths joined with root, +// in a deterministic order. When recursive is true it descends into +// subdirectories (lexical order, courtesy of filepath.WalkDir); otherwise it +// reads only the top level (sorted, courtesy of os.ReadDir). Non-.json files are +// skipped, so docs and other artifacts can sit alongside traces. +func CollectFiles(root string, recursive bool) ([]string, error) { + if recursive { + var files []string + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + return nil + } + if filepath.Ext(d.Name()) == ".json" { + files = append(files, path) + } + return nil + }) + if err != nil { + return nil, err + } + return files, nil + } + + entries, err := os.ReadDir(root) if err != nil { return nil, err } - - // Collect eligible files first so their index fixes the output order. var files []string for _, entry := range entries { if entry.IsDir() { @@ -57,9 +76,29 @@ func (r *Runner) Run(ctx context.Context, dir string) (*Response, error) { if filepath.Ext(entry.Name()) != ".json" { continue } - files = append(files, entry.Name()) + files = append(files, filepath.Join(root, entry.Name())) } + return files, nil +} +// Run reads every .json file in dir (non-recursively) and evaluates it. It is a +// convenience wrapper over CollectFiles + RunFiles; callers needing a single +// file, recursion, or a precomputed list should use those directly. +func (r *Runner) Run(ctx context.Context, dir string) (*Response, error) { + files, err := CollectFiles(dir, false) + if err != nil { + return nil, err + } + return r.RunFiles(ctx, files), nil +} + +// RunFiles evaluates an explicit list of trace file paths. Files are processed +// concurrently (bounded by the CPU count) but results are assembled in the +// input order, so output is deterministic regardless of scheduling. ctx is +// propagated to every evaluator, so cancelling it (Ctrl+C, a CI timeout) aborts +// in-flight work rather than letting it run to completion. Each FileError +// carries the path as given, so errors are unambiguous across subdirectories. +func (r *Runner) RunFiles(ctx context.Context, files []string) *Response { results := make([]fileResult, len(files)) workers := runtime.NumCPU() @@ -69,14 +108,14 @@ func (r *Runner) Run(ctx context.Context, dir string) (*Response, error) { sem := make(chan struct{}, max(workers, 1)) var wg sync.WaitGroup - for i, name := range files { + for i, path := range files { wg.Add(1) sem <- struct{}{} - go func(i int, name string) { + go func(i int, path string) { defer wg.Done() defer func() { <-sem }() - results[i] = r.processFile(ctx, dir, name) - }(i, name) + results[i] = r.processFile(ctx, path) + }(i, path) } wg.Wait() @@ -86,35 +125,35 @@ func (r *Runner) Run(ctx context.Context, dir string) (*Response, error) { response.Evaluations = append(response.Evaluations, res.evals...) response.FileErrors = append(response.FileErrors, res.errs...) } - return &response, nil + return &response } -func (r *Runner) processFile(ctx context.Context, dir, name string) fileResult { +func (r *Runner) processFile(ctx context.Context, path string) fileResult { var res fileResult if err := ctx.Err(); err != nil { - res.errs = append(res.errs, FileError{File: name, Err: err}) + res.errs = append(res.errs, FileError{File: path, Err: err}) return res } - fileBytes, err := os.ReadFile(filepath.Join(dir, name)) + fileBytes, err := os.ReadFile(path) if err != nil { - res.errs = append(res.errs, FileError{File: name, Err: err}) + res.errs = append(res.errs, FileError{File: path, Err: err}) return res } run, err := trajectory.LoadRun(fileBytes) if err != nil { - res.errs = append(res.errs, FileError{File: name, Err: err}) + res.errs = append(res.errs, FileError{File: path, Err: err}) return res } if err := run.Validate(); err != nil { - res.errs = append(res.errs, FileError{File: name, Err: err}) + res.errs = append(res.errs, FileError{File: path, Err: err}) return res } for _, judge := range r.evals { eval, err := judge.EvaluateRun(ctx, run) if err != nil { - res.errs = append(res.errs, FileError{File: name, Err: err}) + res.errs = append(res.errs, FileError{File: path, Err: err}) continue } res.evals = append(res.evals, eval) diff --git a/runner/runner_test.go b/runner/runner_test.go index 4feb727..b7ff79a 100644 --- a/runner/runner_test.go +++ b/runner/runner_test.go @@ -27,15 +27,66 @@ func TestRunner_Run(t *testing.T) { } // broken.json is malformed and invalid_run.json fails Validate => both land - // in FileErrors, not aborting the batch. ReadDir returns names sorted. + // in FileErrors, not aborting the batch. ReadDir returns names sorted, and + // FileError.File carries the path as read (joined with the input dir). if len(resp.FileErrors) != 2 { t.Fatalf("expected 2 file errors, got %d", len(resp.FileErrors)) } - if resp.FileErrors[0].File != "broken.json" { - t.Errorf("expected first file error on broken.json, got %s", resp.FileErrors[0].File) + wantBroken := filepath.Join("../testdata/runs", "broken.json") + wantInvalid := filepath.Join("../testdata/runs", "invalid_run.json") + if resp.FileErrors[0].File != wantBroken { + t.Errorf("expected first file error on %s, got %s", wantBroken, resp.FileErrors[0].File) } - if resp.FileErrors[1].File != "invalid_run.json" { - t.Errorf("expected second file error on invalid_run.json, got %s", resp.FileErrors[1].File) + if resp.FileErrors[1].File != wantInvalid { + t.Errorf("expected second file error on %s, got %s", wantInvalid, resp.FileErrors[1].File) + } +} + +func TestCollectFiles_NonRecursiveSkipsSubdirs(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "a.json"), "{}") + writeFile(t, filepath.Join(dir, "notes.txt"), "ignore me") + sub := filepath.Join(dir, "nested") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + writeFile(t, filepath.Join(sub, "b.json"), "{}") + + files, err := CollectFiles(dir, false) + if err != nil { + t.Fatalf("CollectFiles: %v", err) + } + if len(files) != 1 || files[0] != filepath.Join(dir, "a.json") { + t.Fatalf("non-recursive should return only a.json, got %v", files) + } +} + +func TestCollectFiles_RecursiveDescends(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "a.json"), "{}") + sub := filepath.Join(dir, "nested") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + writeFile(t, filepath.Join(sub, "b.json"), "{}") + + files, err := CollectFiles(dir, true) + if err != nil { + t.Fatalf("CollectFiles: %v", err) + } + if len(files) != 2 { + t.Fatalf("recursive should find both files, got %v", files) + } + // WalkDir yields lexical order: the top-level a.json before nested/b.json. + if files[0] != filepath.Join(dir, "a.json") || files[1] != filepath.Join(sub, "b.json") { + t.Fatalf("recursive order unexpected: %v", files) + } +} + +func writeFile(t *testing.T, path, content string) { + t.Helper() + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", path, err) } } From 1ee5c0dd72ad1659c94e7c7a1d1b20745e8e73f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jesus=20Nu=C3=B1ez?= Date: Fri, 7 Aug 2026 13:56:31 -0400 Subject: [PATCH 8/9] harden: group text output by run with a summary footer Replace the flat per-finding text output with a human-facing report: findings grouped by run (agent shown in the header), severity up front in aligned columns, runs with no findings marked "clean", a one-line summary, and file errors last. The zero-value Score noise is gone. JSON and Markdown output are unchanged; JSON stays keyed on runId (the new Evaluation.Agent field is json:"-"). Update the golden text fixture and the schema doc's expected output. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../langgraph-reference/docs/trace-schema.md | 7 +- evaluator/evaluator.go | 10 +- report/golden_test.go | 3 + report/testdata/text.golden | 19 ++- report/text.go | 158 +++++++++++++++++- runner/runner.go | 2 + 6 files changed, 178 insertions(+), 21 deletions(-) diff --git a/agents/langgraph-reference/docs/trace-schema.md b/agents/langgraph-reference/docs/trace-schema.md index 30e5bee..08e0b68 100644 --- a/agents/langgraph-reference/docs/trace-schema.md +++ b/agents/langgraph-reference/docs/trace-schema.md @@ -168,10 +168,13 @@ go run ./cmd/trazo -dir ./agents/langgraph-reference/docs Expected output: ``` -RunID run-triage-demo-01. Findings: 0 Evaluator: tool_calls +run-triage-demo-01 (github-triage) clean + +Summary: 1 run, 0 bad, 0 neutral, 0 good, 0 file errors ``` -Add `-json` for machine-readable output. +Add `-json` for machine-readable output, or `-validate` for a structure-only +pass that skips the evaluators. ### Exit codes diff --git a/evaluator/evaluator.go b/evaluator/evaluator.go index 11dc7d6..6dfaac1 100644 --- a/evaluator/evaluator.go +++ b/evaluator/evaluator.go @@ -31,9 +31,13 @@ const ( ) type Evaluation struct { - EvaluatorName string `json:"evaluatorName"` - RunID string `json:"runId"` - Findings []Finding `json:"findings"` + EvaluatorName string `json:"evaluatorName"` + RunID string `json:"runId"` + // Agent is the logical agent name of the run, copied from the trace by the + // runner for human-facing output. It is not serialized (json:"-") so the + // machine-readable JSON output stays keyed on runId alone. + Agent string `json:"-"` + Findings []Finding `json:"findings"` } type Finding struct { diff --git a/report/golden_test.go b/report/golden_test.go index 60d2e51..2cc9a44 100644 --- a/report/golden_test.go +++ b/report/golden_test.go @@ -29,6 +29,7 @@ func goldenFixture() *runner.Response { { EvaluatorName: "tool_calls", RunID: "run-1", + Agent: "data_extractor", Findings: []evaluator.Finding{ {StepIndex: 3, Judgment: evaluator.JudgmentBad, Comment: "tool postgres_query fails: connection refused"}, {StepIndex: 2, Judgment: evaluator.JudgmentNeutral, Comment: "tool_call postgres_query has no matching result"}, @@ -37,6 +38,7 @@ func goldenFixture() *runner.Response { { EvaluatorName: "cost_latency", RunID: "run-1", + Agent: "data_extractor", Findings: []evaluator.Finding{ {StepIndex: -1, Judgment: evaluator.JudgmentNeutral, Score: 1.5, Comment: "run cost 1.50 over budget|limit\nsecond line"}, }, @@ -44,6 +46,7 @@ func goldenFixture() *runner.Response { { EvaluatorName: "tool_calls", RunID: "run-ok", + Agent: "data_extractor", Findings: []evaluator.Finding{}, }, }, diff --git a/report/testdata/text.golden b/report/testdata/text.golden index 94bb8ea..2ef862c 100644 --- a/report/testdata/text.golden +++ b/report/testdata/text.golden @@ -1,8 +1,11 @@ -RunID run-1. Findings: 2 Evaluator: tool_calls -Step 3: Comment: tool postgres_query fails: connection refused, Score: 0.000000, Judgment: bad -Step 2: Comment: tool_call postgres_query has no matching result, Score: 0.000000, Judgment: neutral -RunID run-1. Findings: 1 Evaluator: cost_latency -Step -1: Comment: run cost 1.50 over budget|limit -second line, Score: 1.500000, Judgment: neutral -RunID run-ok. Findings: 0 Evaluator: tool_calls -File Error: broken.json → unexpected end of JSON input +run-1 (data_extractor) + [BAD] step 3 tool_calls tool postgres_query fails: connection refused + [NEUTRAL] step 2 tool_calls tool_call postgres_query has no matching result + [NEUTRAL] run cost_latency run cost 1.50 over budget|limit; second line + +run-ok (data_extractor) clean + +Summary: 2 runs, 1 bad, 2 neutral, 0 good, 1 file error + +File errors: + broken.json: unexpected end of JSON input diff --git a/report/text.go b/report/text.go index 8d96c3d..e8f4987 100644 --- a/report/text.go +++ b/report/text.go @@ -4,22 +4,164 @@ import ( "fmt" "strings" + "github.com/Cro22/trazo/evaluator" "github.com/Cro22/trazo/runner" ) -// Text renders the evaluation results as the plain-text CLI format: one header -// line per evaluation, one line per finding, then any file errors. It is the -// default CLI output and is kept deliberately terse and greppable. +// Text renders the evaluation results as a human-facing report: findings grouped +// by run with the severity up front and aligned columns, runs with no findings +// marked "clean", a one-line summary, and any file errors last. For +// machine-readable output use JSON instead. func Text(resp *runner.Response) string { + groups, order := groupByRun(resp) + rows := allRows(groups, order) + sevW, stepW, evalW := columnWidths(rows) + var b strings.Builder + for _, runID := range order { + g := groups[runID] + b.WriteString(runHeader(runID, g.agent)) + if len(g.rows) == 0 { + b.WriteString(" clean\n") + b.WriteString("\n") + continue + } + b.WriteString("\n") + for _, r := range g.rows { + fmt.Fprintf(&b, " %-*s %-*s %-*s %s\n", + sevW, r.severity, stepW, r.step, evalW, r.evaluator, r.comment) + } + b.WriteString("\n") + } + + b.WriteString(summaryLine(groups, order, len(resp.FileErrors))) + if len(resp.FileErrors) > 0 { + b.WriteString("\nFile errors:\n") + for _, fe := range resp.FileErrors { + fmt.Fprintf(&b, " %s: %s\n", fe.File, oneline(fe.Err.Error())) + } + } + return b.String() +} + +type textRow struct { + severity string + step string + evaluator string + comment string +} + +type runGroup struct { + agent string + rows []textRow +} + +// groupByRun collects findings across every evaluator into one group per run, +// preserving the first-seen run order so output is deterministic. +func groupByRun(resp *runner.Response) (map[string]*runGroup, []string) { + groups := map[string]*runGroup{} + var order []string for _, e := range resp.Evaluations { - fmt.Fprintf(&b, "RunID %s. Findings: %d Evaluator: %s\n", e.RunID, len(e.Findings), e.EvaluatorName) + g, ok := groups[e.RunID] + if !ok { + g = &runGroup{agent: e.Agent} + groups[e.RunID] = g + order = append(order, e.RunID) + } + if g.agent == "" { + g.agent = e.Agent + } for _, f := range e.Findings { - fmt.Fprintf(&b, "Step %d: Comment: %s, Score: %f, Judgment: %s\n", f.StepIndex, f.Comment, f.Score, f.Judgment) + g.rows = append(g.rows, textRow{ + severity: severityTag(f.Judgment), + step: stepText(f.StepIndex), + evaluator: e.EvaluatorName, + comment: oneline(f.Comment), + }) } } - for _, fe := range resp.FileErrors { - fmt.Fprintf(&b, "File Error: %s → %v\n", fe.File, fe.Err) + return groups, order +} + +func allRows(groups map[string]*runGroup, order []string) []textRow { + var rows []textRow + for _, runID := range order { + rows = append(rows, groups[runID].rows...) } - return b.String() + return rows +} + +func columnWidths(rows []textRow) (sev, step, eval int) { + for _, r := range rows { + sev = maxInt(sev, len(r.severity)) + step = maxInt(step, len(r.step)) + eval = maxInt(eval, len(r.evaluator)) + } + return sev, step, eval +} + +func runHeader(runID, agent string) string { + if agent == "" { + return runID + } + return fmt.Sprintf("%s (%s)", runID, agent) +} + +func severityTag(j evaluator.Judgment) string { + switch j { + case evaluator.JudgmentBad: + return "[BAD]" + case evaluator.JudgmentNeutral: + return "[NEUTRAL]" + case evaluator.JudgmentGood: + return "[GOOD]" + default: + return "[" + strings.ToUpper(string(j)) + "]" + } +} + +// stepText renders the step index, using "run" for the run-level sentinel (-1). +func stepText(idx int) string { + if idx < 0 { + return "run" + } + return fmt.Sprintf("step %d", idx) +} + +func summaryLine(groups map[string]*runGroup, order []string, fileErrors int) string { + var bad, neutral, good int + for _, runID := range order { + for _, r := range groups[runID].rows { + switch r.severity { + case "[BAD]": + bad++ + case "[NEUTRAL]": + neutral++ + case "[GOOD]": + good++ + } + } + } + return fmt.Sprintf("Summary: %s, %d bad, %d neutral, %d good, %s\n", + plural(len(order), "run"), bad, neutral, good, plural(fileErrors, "file error")) +} + +// oneline flattens a possibly multi-line message onto a single line so table +// rows stay aligned. +func oneline(s string) string { + return strings.ReplaceAll(s, "\n", "; ") +} + +func plural(n int, unit string) string { + if n == 1 { + return fmt.Sprintf("%d %s", n, unit) + } + return fmt.Sprintf("%d %ss", n, unit) +} + +func maxInt(a, b int) int { + if a > b { + return a + } + return b } diff --git a/runner/runner.go b/runner/runner.go index 56bb0a1..d490d98 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -156,6 +156,8 @@ func (r *Runner) processFile(ctx context.Context, path string) fileResult { res.errs = append(res.errs, FileError{File: path, Err: err}) continue } + // Carry the agent name for human-facing output; evaluators only set RunID. + eval.Agent = run.Agent res.evals = append(res.evals, eval) } return res From 891c879de1ad6e43720bdf626211b10209367a9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jesus=20Nu=C3=B1ez?= Date: Fri, 7 Aug 2026 17:01:56 -0400 Subject: [PATCH 9/9] harden: make schema version required and gate compatibility by major Introduce real schema versioning. trajectory.SchemaVersion ("0.1.0") is the canonical version, and Run.Validate now requires a semver version and rejects a trace whose major differs from the build's supported major. Minor/patch differences within the major stay compatible (adding toolCallId was such an additive bump), so older 0.x traces still load. The JSON Schema marks version required with a semver pattern, and a Go test keeps the schema, the SchemaVersion constant, and the compatibility check in sync. Normalize every fixture to 0.1.0 and flip the old "absent version is accepted" test to assert the new gate, with explicit accept/reject version cases. Python side: the emitter exposes SCHEMA_VERSION mirroring the Go constant and stamps it by default; the agent no longer conflates its own version with the schema version. Also fix a stale recorder docstring that predated toolCallId. Update the schema doc with a Versioning section. Co-Authored-By: Claude Opus 4.8 (1M context) --- agents/langgraph-reference/agent/app.py | 5 +- .../docs/sample-trace.json | 2 +- .../langgraph-reference/docs/trace-schema.md | 39 ++++++++--- .../tests/test_cross_language.py | 6 +- .../trazo_emitter/__init__.py | 3 +- .../trazo_emitter/recorder.py | 20 ++++-- runner/runner_test.go | 2 +- testdata/ci/clean/triage_clean.json | 2 +- testdata/ci/failing/tool_error.json | 2 +- testdata/runs/invalid_run.json | 2 +- testdata/runs/sample_run.json | 2 +- testdata/runs/sample_run_ok.json | 2 +- testdata/sample_run.json | 2 +- testdata/sample_run_missing_result.json | 2 +- testdata/sample_run_ok.json | 2 +- testdata/sample_run_orphan_result.json | 2 +- trajectory/schema_test.go | 34 +++++++++- trajectory/trace.schema.json | 5 +- trajectory/validate.go | 13 ++-- trajectory/validate_test.go | 57 +++++++++++++---- trajectory/version.go | 64 +++++++++++++++++++ 21 files changed, 217 insertions(+), 51 deletions(-) create mode 100644 trajectory/version.go diff --git a/agents/langgraph-reference/agent/app.py b/agents/langgraph-reference/agent/app.py index a7084f3..1e907e7 100644 --- a/agents/langgraph-reference/agent/app.py +++ b/agents/langgraph-reference/agent/app.py @@ -30,7 +30,6 @@ def _default_handler(recorder: TraceRecorder, model: str) -> BaseCallbackHandler return TracingCallbackHandler(recorder, model=model) AGENT_NAME = "github-triage" -AGENT_VERSION = "0.0.1" @dataclass @@ -53,7 +52,9 @@ def run_triage( clock: Optional[Callable[[], datetime]] = None, handler_factory: Optional[HandlerFactory] = None, ) -> TriageResult: - recorder = TraceRecorder(run_id, AGENT_NAME, AGENT_VERSION, clock=clock) + # The trace version is the schema version (SCHEMA_VERSION default), not the + # agent's own version; the Go core gates compatibility on it. + recorder = TraceRecorder(run_id, AGENT_NAME, clock=clock) recorder.record_node_transition("start") tools = build_tools(source) diff --git a/agents/langgraph-reference/docs/sample-trace.json b/agents/langgraph-reference/docs/sample-trace.json index b63a671..af31a23 100644 --- a/agents/langgraph-reference/docs/sample-trace.json +++ b/agents/langgraph-reference/docs/sample-trace.json @@ -1,7 +1,7 @@ { "id": "run-triage-demo-01", "agent": "github-triage", - "version": "0.0.1", + "version": "0.1.0", "startTime": "2026-08-06T10:00:00Z", "endTime": "2026-08-06T10:00:07Z", "steps": [ diff --git a/agents/langgraph-reference/docs/trace-schema.md b/agents/langgraph-reference/docs/trace-schema.md index 08e0b68..6079121 100644 --- a/agents/langgraph-reference/docs/trace-schema.md +++ b/agents/langgraph-reference/docs/trace-schema.md @@ -55,13 +55,12 @@ The top-level object deserializes into `trajectory.Run`. |-------------|-------------|----------|-------| | `id` | string | yes | Non-empty. Used as `runId` in evaluator output. | | `agent` | string | yes | Non-empty. Logical agent name. | -| `version` | string | no* | Not checked by `Validate`, but present in every fixture. Treat as required by convention. | +| `version` | string | yes | Semver `MAJOR.MINOR.PATCH`. Required and gated for compatibility; see [Versioning](#versioning). | | `startTime` | RFC3339 time| yes | Must be non-zero. | | `endTime` | RFC3339 time| yes | Must be non-zero and not before `startTime`. | -| `steps` | array | yes** | May be empty and still pass `Validate`, but a run with no steps has nothing to evaluate. | +| `steps` | array | yes* | May be empty and still pass `Validate`, but a run with no steps has nothing to evaluate. | -\* Not enforced by `Validate`; include it anyway. -\** An absent `steps` deserializes to an empty slice; it passes validation but is +\* An absent `steps` deserializes to an empty slice; it passes validation but is degenerate. Timestamps are Go `time.Time`, so any RFC3339 string Go's JSON decoder accepts is @@ -115,16 +114,40 @@ bad file reports all problems at once. It checks structure only; it does not judge agent behavior. Rules: 1. `id` non-empty, `agent` non-empty. -2. `startTime` and `endTime` non-zero; `endTime` not before `startTime`. -3. Every step `timestamp` non-zero. -4. Per-type required field present: `llm_call`->`llm`, `tool_call`/`tool_result` +2. `version` present and semver-compatible with this build; see [Versioning](#versioning). +3. `startTime` and `endTime` non-zero; `endTime` not before `startTime`. +4. Every step `timestamp` non-zero. +5. Per-type required field present: `llm_call`->`llm`, `tool_call`/`tool_result` ->`tool`, `node_transition`->`node`. -5. `type` is one of the four known values. +6. `type` is one of the four known values. The runner (`runner/runner.go`) treats a file as a `fileError` if it cannot be read, cannot be unmarshaled, or fails `Validate`. Such files are skipped for evaluation and reported separately. +## Versioning + +The `version` field carries the trace **schema** version (not the agent's own +version), as semver `MAJOR.MINOR.PATCH`. The Go core declares a canonical +`trajectory.SchemaVersion` (currently `0.1.0`) and a supported MAJOR. + +Compatibility gate (`trajectory/checkVersion`, part of `Run.Validate`): + +- The version is required. An empty or non-semver version is rejected. +- A trace is accepted when its MAJOR equals the build's supported MAJOR, + regardless of MINOR/PATCH. MINOR/PATCH bumps are additive and backward + compatible (for example, adding the optional `toolCallId` field bumped the + MINOR), so a `0.0.1` trace and a `0.1.0` trace are both accepted by a `0.x` + build. +- A trace whose MAJOR differs is rejected with an actionable message naming the + supported version. Bump the MAJOR only for a breaking change (a removed or + renamed field, or a changed meaning). + +The Python emitter mirrors this constant as `trazo_emitter.SCHEMA_VERSION` and +stamps it on every trace by default. Keep the two constants in sync; a Go test +(`trajectory/schema_test.go`) checks the JSON Schema requires `version` and that +`SchemaVersion` passes the gate. + ## Tool call / result pairing (what the emitter must respect) `ToolCallEvaluator` (`evaluator/toolcalls.go`) walks the steps in order and pairs diff --git a/agents/langgraph-reference/tests/test_cross_language.py b/agents/langgraph-reference/tests/test_cross_language.py index 5b64b52..0806488 100644 --- a/agents/langgraph-reference/tests/test_cross_language.py +++ b/agents/langgraph-reference/tests/test_cross_language.py @@ -47,7 +47,7 @@ def _findings_for(result: dict, run_id: str) -> list[dict]: def test_clean_run_evaluates_with_no_findings(tmp_path) -> None: - rec = TraceRecorder("run-clean", "github-triage", "0.0.1", start_time=_dt(0)) + rec = TraceRecorder("run-clean", "github-triage", start_time=_dt(0)) rec.record_node_transition("start", timestamp=_dt(0)) call = rec.record_tool_call("fetch_issues", input={"repo": "golang/example"}, timestamp=_dt(1)) rec.record_tool_result(call, output={"count": 0}, timestamp=_dt(2)) @@ -60,7 +60,7 @@ def test_clean_run_evaluates_with_no_findings(tmp_path) -> None: def test_tool_error_produces_bad_finding(tmp_path) -> None: - rec = TraceRecorder("run-toolerr", "github-triage", "0.0.1", start_time=_dt(0)) + rec = TraceRecorder("run-toolerr", "github-triage", start_time=_dt(0)) call = rec.record_tool_call("fetch_issues", timestamp=_dt(0)) rec.record_tool_result(call, error="rate limited: 403", timestamp=_dt(1)) rec.flush(tmp_path, end_time=_dt(2)) @@ -70,7 +70,7 @@ def test_tool_error_produces_bad_finding(tmp_path) -> None: def test_orphan_tool_call_produces_neutral_finding(tmp_path) -> None: - rec = TraceRecorder("run-orphan", "github-triage", "0.0.1", start_time=_dt(0)) + rec = TraceRecorder("run-orphan", "github-triage", start_time=_dt(0)) rec.record_tool_call("fetch_issues", timestamp=_dt(0)) # no result emitted rec.flush(tmp_path, end_time=_dt(1)) diff --git a/agents/langgraph-reference/trazo_emitter/__init__.py b/agents/langgraph-reference/trazo_emitter/__init__.py index c972d3e..8a4f631 100644 --- a/agents/langgraph-reference/trazo_emitter/__init__.py +++ b/agents/langgraph-reference/trazo_emitter/__init__.py @@ -6,7 +6,7 @@ """ from .models import Run, Step, StepType, to_rfc3339 -from .recorder import ToolCall, TraceRecorder +from .recorder import SCHEMA_VERSION, ToolCall, TraceRecorder __all__ = [ "Run", @@ -14,5 +14,6 @@ "StepType", "TraceRecorder", "ToolCall", + "SCHEMA_VERSION", "to_rfc3339", ] diff --git a/agents/langgraph-reference/trazo_emitter/recorder.py b/agents/langgraph-reference/trazo_emitter/recorder.py index 4a7f5f9..fdc48fc 100644 --- a/agents/langgraph-reference/trazo_emitter/recorder.py +++ b/agents/langgraph-reference/trazo_emitter/recorder.py @@ -1,11 +1,11 @@ """TraceRecorder: build a Run step by step and flush it to disk. -Correlation note: the Go schema has no call id. tool_call and tool_result are -paired by tool name in FIFO order (see evaluator/toolcalls.go and -../docs/trace-schema.md). This recorder mirrors that: record_tool_call returns a -ToolCall handle that carries the tool name, and record_tool_result accepts either -that handle or a bare tool name. Correlation is therefore logical, at this layer; -no extra field is written to the JSON. +Correlation note: tool_call and tool_result are paired by an explicit call id. +record_tool_call returns a ToolCall handle carrying a generated id, which is +written to both the call and its result as toolCallId, so pairing is precise even +when the same tool is called several times. record_tool_result also accepts a +bare tool name, in which case the Go core falls back to name/FIFO pairing (see +evaluator/toolcalls.go and ../docs/trace-schema.md). """ from __future__ import annotations @@ -18,6 +18,12 @@ from .models import Run, Step, StepType +# SCHEMA_VERSION mirrors trajectory.SchemaVersion in the Go core and is the +# default version stamped on emitted traces. Keep the two in sync: the Go loader +# rejects a trace whose major differs from its supported major. Bump the minor +# for additive changes, the major for breaking ones. +SCHEMA_VERSION = "0.1.0" + @dataclass(frozen=True) class ToolCall: @@ -48,7 +54,7 @@ def __init__( self, run_id: str, agent: str, - version: str, + version: str = SCHEMA_VERSION, *, clock: Optional[Clock] = None, start_time: Optional[datetime] = None, diff --git a/runner/runner_test.go b/runner/runner_test.go index b7ff79a..938a92b 100644 --- a/runner/runner_test.go +++ b/runner/runner_test.go @@ -127,7 +127,7 @@ func TestRunner_Run_DeterministicOrder(t *testing.T) { content := fmt.Sprintf(`{ "id": %q, "agent": "stress", - "version": "0.0.1", + "version": "0.1.0", "startTime": "2026-06-12T14:00:00Z", "endTime": "2026-06-12T14:00:05Z", "steps": [ diff --git a/testdata/ci/clean/triage_clean.json b/testdata/ci/clean/triage_clean.json index 6bdf5b6..a22eb42 100644 --- a/testdata/ci/clean/triage_clean.json +++ b/testdata/ci/clean/triage_clean.json @@ -1,7 +1,7 @@ { "id": "ci-clean-01", "agent": "github-triage", - "version": "0.0.1", + "version": "0.1.0", "startTime": "2026-08-06T10:00:00Z", "endTime": "2026-08-06T10:00:05Z", "steps": [ diff --git a/testdata/ci/failing/tool_error.json b/testdata/ci/failing/tool_error.json index 4de426f..8905859 100644 --- a/testdata/ci/failing/tool_error.json +++ b/testdata/ci/failing/tool_error.json @@ -1,7 +1,7 @@ { "id": "ci-failing-tool-error", "agent": "github-triage", - "version": "0.0.1", + "version": "0.1.0", "startTime": "2026-08-06T10:00:00Z", "endTime": "2026-08-06T10:00:03Z", "steps": [ diff --git a/testdata/runs/invalid_run.json b/testdata/runs/invalid_run.json index cc55502..4a858e0 100644 --- a/testdata/runs/invalid_run.json +++ b/testdata/runs/invalid_run.json @@ -1,7 +1,7 @@ { "id": "run-invalid-9f", "agent": "", - "version": "1.2.0", + "version": "0.1.0", "startTime": "2026-06-12T14:00:00Z", "endTime": "2026-06-12T14:00:05Z", "steps": [ diff --git a/testdata/runs/sample_run.json b/testdata/runs/sample_run.json index 5e25351..7da6c77 100644 --- a/testdata/runs/sample_run.json +++ b/testdata/runs/sample_run.json @@ -1,7 +1,7 @@ { "id": "run-a1b2", "agent": "data_extractor", - "version": "1.2.0", + "version": "0.1.0", "startTime": "2026-06-12T14:00:00Z", "endTime": "2026-06-12T14:00:08Z", "steps": [ diff --git a/testdata/runs/sample_run_ok.json b/testdata/runs/sample_run_ok.json index 2275171..c27aad7 100644 --- a/testdata/runs/sample_run_ok.json +++ b/testdata/runs/sample_run_ok.json @@ -1,7 +1,7 @@ { "id": "run-ok-123", "agent": "data_extractor", - "version": "1.2.0", + "version": "0.1.0", "startTime": "2026-06-12T14:00:00Z", "endTime": "2026-06-12T14:00:05Z", "steps": [ diff --git a/testdata/sample_run.json b/testdata/sample_run.json index 5e25351..7da6c77 100644 --- a/testdata/sample_run.json +++ b/testdata/sample_run.json @@ -1,7 +1,7 @@ { "id": "run-a1b2", "agent": "data_extractor", - "version": "1.2.0", + "version": "0.1.0", "startTime": "2026-06-12T14:00:00Z", "endTime": "2026-06-12T14:00:08Z", "steps": [ diff --git a/testdata/sample_run_missing_result.json b/testdata/sample_run_missing_result.json index 1dfbe80..7bcad42 100644 --- a/testdata/sample_run_missing_result.json +++ b/testdata/sample_run_missing_result.json @@ -1,7 +1,7 @@ { "id": "run-timeout-404", "agent": "data_extractor", - "version": "1.2.0", + "version": "0.1.0", "startTime": "2026-06-12T14:00:00Z", "endTime": "2026-06-12T14:00:05Z", "steps": [ diff --git a/testdata/sample_run_ok.json b/testdata/sample_run_ok.json index 2275171..c27aad7 100644 --- a/testdata/sample_run_ok.json +++ b/testdata/sample_run_ok.json @@ -1,7 +1,7 @@ { "id": "run-ok-123", "agent": "data_extractor", - "version": "1.2.0", + "version": "0.1.0", "startTime": "2026-06-12T14:00:00Z", "endTime": "2026-06-12T14:00:05Z", "steps": [ diff --git a/testdata/sample_run_orphan_result.json b/testdata/sample_run_orphan_result.json index 9e81ddf..eaf9293 100644 --- a/testdata/sample_run_orphan_result.json +++ b/testdata/sample_run_orphan_result.json @@ -1,7 +1,7 @@ { "id": "run-orphan-7c", "agent": "data_extractor", - "version": "1.2.0", + "version": "0.1.0", "startTime": "2026-06-12T14:00:00Z", "endTime": "2026-06-12T14:00:06Z", "steps": [ diff --git a/trajectory/schema_test.go b/trajectory/schema_test.go index 3828621..1f78d3c 100644 --- a/trajectory/schema_test.go +++ b/trajectory/schema_test.go @@ -8,8 +8,15 @@ import ( ) // schemaDoc captures only the parts of trace.schema.json this test asserts on: -// the step type enum and the per-type required-field rules encoded as if/then. +// the top-level required fields and version pattern, the step type enum, and the +// per-type required-field rules encoded as if/then. type schemaDoc struct { + Required []string `json:"required"` + Properties struct { + Version struct { + Pattern string `json:"pattern"` + } `json:"version"` + } `json:"properties"` Defs struct { Step struct { Properties struct { @@ -46,6 +53,31 @@ func loadSchemaDoc(t *testing.T) schemaDoc { return doc } +// TestSchemaRequiresVersion guards that the published schema requires the version +// field (with a semver pattern) and that the canonical SchemaVersion both matches +// that pattern and passes the Go compatibility gate. This keeps the schema, the +// SchemaVersion constant, and checkVersion from drifting apart. +func TestSchemaRequiresVersion(t *testing.T) { + doc := loadSchemaDoc(t) + + found := false + for _, r := range doc.Required { + if r == "version" { + found = true + break + } + } + if !found { + t.Errorf("schema top-level required must include \"version\", got %v", doc.Required) + } + if doc.Properties.Version.Pattern == "" { + t.Error("schema version property must declare a semver pattern") + } + if err := checkVersion(SchemaVersion); err != nil { + t.Errorf("canonical SchemaVersion %q must pass checkVersion: %v", SchemaVersion, err) + } +} + // TestSchemaTypeEnumMatchesConstants guards against the published schema drifting // away from the Go StepType constants (the source of truth). If a StepType is // added or renamed without touching the schema, this fails. diff --git a/trajectory/trace.schema.json b/trajectory/trace.schema.json index 727b7a7..4c88d3f 100644 --- a/trajectory/trace.schema.json +++ b/trajectory/trace.schema.json @@ -5,7 +5,7 @@ "description": "One agent run per file, as a single JSON object. The source of truth for this format is the Go code (trajectory/steps.go for the shape, trajectory/validate.go for the acceptance gate); this schema is the derived, strict external contract. It is deliberately stricter than the Go loader in one way: the loader ignores unknown fields, this schema forbids them (additionalProperties: false). Some invariants that Run.Validate enforces are cross-field and cannot be expressed in JSON Schema, so they are Go-only: endTime not before startTime, step timestamps monotonic non-decreasing, and every step timestamp within [startTime, endTime]. A document that passes this schema still has to pass Run.Validate.", "type": "object", "additionalProperties": false, - "required": ["id", "agent", "startTime", "endTime"], + "required": ["id", "agent", "version", "startTime", "endTime"], "properties": { "id": { "type": "string", @@ -19,7 +19,8 @@ }, "version": { "type": "string", - "description": "Schema/trace version. Not enforced by Run.Validate, but present in every emitted trace by convention; include it." + "pattern": "^[0-9]+\\.[0-9]+\\.[0-9]+$", + "description": "Schema version, semver MAJOR.MINOR.PATCH. Required. This schema only enforces the shape; Run.Validate additionally rejects a trace whose MAJOR differs from the build's supported major (see the Versioning section of the schema doc)." }, "startTime": { "type": "string", diff --git a/trajectory/validate.go b/trajectory/validate.go index 39c9788..728681c 100644 --- a/trajectory/validate.go +++ b/trajectory/validate.go @@ -12,11 +12,11 @@ import ( // of evaluators. // // Strictness policy: hard invariants (non-negative quantities, monotonic step -// timestamps, steps within the run interval) reject the trace. Optional fields -// are validated only when present: an absent version or an empty step list is -// accepted, since older emitters produced such traces and they carry no -// ambiguity. Schema-version compatibility is a separate concern (see the -// schema doc), not enforced here. +// timestamps, steps within the run interval) reject the trace. The version is +// required and must be semver-compatible with this build's supported major (see +// checkVersion); this is the schema-compatibility gate. An empty step list is +// still accepted, since a run with no steps carries no ambiguity, only nothing +// to evaluate. func (r *Run) Validate() error { var errs []error @@ -26,6 +26,9 @@ func (r *Run) Validate() error { if r.Agent == "" { errs = append(errs, errors.New("run: agent is empty")) } + if err := checkVersion(r.Version); err != nil { + errs = append(errs, err) + } if r.StartTime.IsZero() { errs = append(errs, errors.New("run: startTime is missing")) } diff --git a/trajectory/validate_test.go b/trajectory/validate_test.go index bfadade..b00dc0f 100644 --- a/trajectory/validate_test.go +++ b/trajectory/validate_test.go @@ -12,7 +12,7 @@ func validRun() *Run { return &Run{ ID: "run-valid", Agent: "data_extractor", - Version: "1.2.0", + Version: "0.1.0", StartTime: start, EndTime: start.Add(5 * time.Second), Steps: []Step{ @@ -71,17 +71,52 @@ func TestValidate_InvalidRuns(t *testing.T) { } } -// TestValidate_LenientOptionalFields pins the pragmatic strictness policy: -// an absent version and an empty step list are accepted, since older emitters -// produced such traces and they carry no structural ambiguity. +// TestValidate_Version pins the schema-compatibility gate: version is required +// and must be semver sharing the build's supported major. Same-major minor/patch +// differences are accepted (additive, backward compatible); a missing version, +// non-semver, or cross-major version is rejected. +func TestValidate_Version(t *testing.T) { + accepted := []string{"0.1.0", "0.0.1", "0.9.9", "0.1.5"} + for _, v := range accepted { + t.Run("accept "+v, func(t *testing.T) { + run := validRun() + run.Version = v + if err := run.Validate(); err != nil { + t.Errorf("version %q should be accepted, got: %v", v, err) + } + }) + } + + rejected := []struct { + v string + wantMsg string + }{ + {"", "version is empty"}, + {"1.0.0", "unsupported"}, + {"2.3.4", "unsupported"}, + {"0.1", "not semver"}, + {"v0.1.0", "not semver"}, + {"0.1.x", "not semver"}, + } + for _, tc := range rejected { + t.Run("reject "+tc.v, func(t *testing.T) { + run := validRun() + run.Version = tc.v + err := run.Validate() + if err == nil { + t.Fatalf("version %q should be rejected", tc.v) + } + if !strings.Contains(err.Error(), tc.wantMsg) { + t.Errorf("version %q: expected error containing %q, got: %v", tc.v, tc.wantMsg, err) + } + }) + } +} + +// TestValidate_LenientOptionalFields pins the remaining pragmatic leniency: an +// empty step list and zero quantities are accepted, since they carry no +// structural ambiguity (a run with no steps simply has nothing to evaluate). func TestValidate_LenientOptionalFields(t *testing.T) { - t.Run("absent version", func(t *testing.T) { - run := validRun() - run.Version = "" - if err := run.Validate(); err != nil { - t.Errorf("absent version should be accepted, got: %v", err) - } - }) t.Run("empty steps", func(t *testing.T) { run := validRun() run.Steps = nil diff --git a/trajectory/version.go b/trajectory/version.go new file mode 100644 index 0000000..77748c6 --- /dev/null +++ b/trajectory/version.go @@ -0,0 +1,64 @@ +package trajectory + +import ( + "fmt" + "strconv" + "strings" +) + +// SchemaVersion is the trace schema version this build emits and treats as +// canonical. It is semantic (MAJOR.MINOR.PATCH). Bump the MINOR for additive, +// backward-compatible changes (a new optional field, like toolCallId) and the +// MAJOR for a breaking change (a removed/renamed field, a changed meaning). +const SchemaVersion = "0.1.0" + +// supportedMajor is the schema MAJOR version this build can evaluate. Traces are +// accepted across MINOR/PATCH differences within this major, since those are +// additive and backward compatible, and rejected across a MAJOR boundary, where +// the shape may have changed incompatibly. Keep it equal to SchemaVersion's +// major. +const supportedMajor = 0 + +// checkVersion validates a run's version field: it must be present, be valid +// semver, and share this build's supported MAJOR. A cross-major trace is +// reported with the supported version so the message is actionable. +func checkVersion(v string) error { + if v == "" { + return fmt.Errorf("run: version is empty (expected semver compatible with %s)", SchemaVersion) + } + major, _, _, err := parseSemver(v) + if err != nil { + return fmt.Errorf("run: version %q is not semver MAJOR.MINOR.PATCH: %w", v, err) + } + if major != supportedMajor { + return fmt.Errorf("run: schema version %q (major %d) is unsupported; this build accepts major %d (%s)", + v, major, supportedMajor, SchemaVersion) + } + return nil +} + +// parseSemver splits a MAJOR.MINOR.PATCH string. It is intentionally strict: +// exactly three dot-separated non-negative integers, no pre-release or build +// metadata, matching what the emitter produces and what the JSON Schema pattern +// enforces. +func parseSemver(v string) (major, minor, patch int, err error) { + parts := strings.Split(v, ".") + if len(parts) != 3 { + return 0, 0, 0, fmt.Errorf("want 3 dot-separated parts, got %d", len(parts)) + } + nums := [3]int{} + for i, p := range parts { + if p == "" { + return 0, 0, 0, fmt.Errorf("part %d is empty", i+1) + } + n, convErr := strconv.Atoi(p) + if convErr != nil { + return 0, 0, 0, fmt.Errorf("part %d %q is not an integer", i+1, p) + } + if n < 0 { + return 0, 0, 0, fmt.Errorf("part %d %q is negative", i+1, p) + } + nums[i] = n + } + return nums[0], nums[1], nums[2], nil +}