Skip to content

Commit 0bb506c

Browse files
committed
Harden PerfTrace eval harness asserts and negative coverage
Replace the live no-op relative-magnitude check with real wall ordering, add fail-path tests for assert helpers, drop pure rollup arithmetic re-tests, and document assert layer ownership vs rollup.test.ts.
1 parent 7e6d593 commit 0bb506c

2 files changed

Lines changed: 135 additions & 47 deletions

File tree

src/perf/assert-spans.test.ts

Lines changed: 119 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
/**
2-
* Latency eval harness: assert on PerfTrace phase presence and relative magnitudes.
2+
* Latency eval harness: assert helpers over PerfTrace snapshots and rollups.
3+
*
4+
* This layer owns:
5+
* - assert API behavior (pass paths + negative branches)
6+
* - golden multi-tool fixture equality (locked TurnSummary)
7+
* - one end-to-end smoke: reactor observer → snapshot → rollup → asserts
8+
*
9+
* Rollup arithmetic (phase totals, sessionTotals, percentiles) lives in
10+
* rollup.test.ts — do not re-test pure rollup math here.
311
*
412
* Covers CL-5174 outcomes:
513
* - phase presence + nesting helpers
@@ -23,7 +31,7 @@ import {
2331
} from "./fixtures/multi-tool-turn.js";
2432
import { ALLOWED_TAG_KEYS, clear, snapshot, type PerfSpan } from "./index.js";
2533
import { createPerfReactorObserver } from "./reactor-spans.js";
26-
import { rollupByPhase, rollupByTurn, sessionTotals } from "./rollup.js";
34+
import { rollupByPhase, rollupByTurn, type TurnSummary } from "./rollup.js";
2735

2836
afterEach(() => {
2937
clear();
@@ -50,6 +58,19 @@ function completed(spans: PerfSpan[]): PerfSpan[] {
5058
return spans.filter((s) => s.endNs !== undefined);
5159
}
5260

61+
function turnSummary(partial: Partial<TurnSummary> & Pick<TurnSummary, "turnId">): TurnSummary {
62+
return {
63+
turnNs: 1000,
64+
open: false,
65+
inferenceNs: 500,
66+
toolNs: 200,
67+
ttftNs: 100,
68+
streamNs: 400,
69+
toolCount: 1,
70+
...partial,
71+
};
72+
}
73+
5374
describe("assertPhasePresent / assertNesting", () => {
5475
test("assertPhasePresent finds phases on the golden fixture", () => {
5576
const spans = multiToolTurnFixture();
@@ -67,8 +88,9 @@ describe("assertPhasePresent / assertNesting", () => {
6788
);
6889
});
6990

70-
test("assertNesting verifies parent-child links", () => {
91+
test("assertNesting verifies parent-child links (child, parent arg order)", () => {
7192
const spans = multiToolTurnFixture();
93+
// child first, then expected parent
7294
assertNesting(spans, "inference", "turn");
7395
assertNesting(spans, "inference.ttft", "inference");
7496
assertNesting(spans, "inference.stream", "inference");
@@ -83,58 +105,109 @@ describe("assertPhasePresent / assertNesting", () => {
83105
});
84106
});
85107

86-
describe("golden multi-tool turn fixture", () => {
87-
test("rollupByTurn matches locked golden values", () => {
88-
const turns = rollupByTurn(multiToolTurnFixture());
89-
expect(turns).toHaveLength(1);
90-
expect(turns[0]).toEqual({ ...MULTI_TOOL_TURN_GOLDEN });
91-
});
92-
93-
test("fixture tags are privacy-safe (allowlisted keys only)", () => {
94-
for (const span of multiToolTurnFixture()) {
95-
if (span.tags === undefined) continue;
96-
for (const key of Object.keys(span.tags)) {
97-
expect(ALLOWED_TAG_KEY_SET.has(key)).toBe(true);
98-
}
99-
}
100-
});
101-
102-
test("phase rollup reports expected counts and totals", () => {
108+
describe("assertPhaseSummary", () => {
109+
test("passes on golden phase rollup with minCount and minTotalNs", () => {
103110
const phases = rollupByPhase(multiToolTurnFixture());
104111
assertPhaseSummary(phases, "turn", { minCount: 1, minTotalNs: 5000 });
105112
assertPhaseSummary(phases, "inference", { minCount: 1, minTotalNs: 2000 });
106113
assertPhaseSummary(phases, "tool", { minCount: 2, minTotalNs: 1200 });
107114
assertPhaseSummary(phases, "permission.wait", { minCount: 1, minTotalNs: 400 });
108115
});
116+
117+
test("throws when phase summary is missing", () => {
118+
const phases = rollupByPhase(multiToolTurnFixture());
119+
expect(() => assertPhaseSummary(phases, "subagent")).toThrow(
120+
/expected phase summary "subagent"/,
121+
);
122+
});
123+
124+
test("throws when count is below minCount", () => {
125+
const phases = rollupByPhase(multiToolTurnFixture());
126+
expect(() => assertPhaseSummary(phases, "tool", { minCount: 3 })).toThrow(
127+
/phase "tool": expected count >= 3/,
128+
);
129+
});
130+
131+
test("throws when totalNs is below minTotalNs", () => {
132+
const phases = rollupByPhase(multiToolTurnFixture());
133+
expect(() =>
134+
assertPhaseSummary(phases, "inference", { minTotalNs: 999_999 }),
135+
).toThrow(/phase "inference": expected totalNs >= 999999/);
136+
});
137+
});
138+
139+
describe("assertLessThan", () => {
140+
test("passes when left < right", () => {
141+
assertLessThan(400, 1600, "ttft vs stream");
142+
});
143+
144+
test("throws when left >= right", () => {
145+
expect(() => assertLessThan(1600, 400, "ttft vs stream")).toThrow(
146+
/ttft vs stream: expected 1600 < 400/,
147+
);
148+
expect(() => assertLessThan(5, 5, "eq")).toThrow(/eq: expected 5 < 5/);
149+
});
109150
});
110151

111-
describe("regression: turn has inference + tools when tools ran", () => {
112-
test("assertTurnHasInferenceAndTools passes on multi-tool golden rollup", () => {
152+
describe("assertTurnHasInferenceAndTools", () => {
153+
test("passes on multi-tool golden rollup", () => {
113154
const turns = rollupByTurn(multiToolTurnFixture());
114155
assertTurnHasInferenceAndTools(turns[0]!);
156+
assertTurnHasInferenceAndTools(turns[0]!, { minToolCount: 2 });
115157
});
116158

117-
test("assertTurnHasInferenceAndTools fails when tools did not run", () => {
159+
test("throws when inferenceNs is not positive", () => {
160+
expect(() =>
161+
assertTurnHasInferenceAndTools(turnSummary({ turnId: "t-no-inf", inferenceNs: 0 })),
162+
).toThrow(/turn t-no-inf: expected inferenceNs > 0/);
163+
});
164+
165+
test("throws when toolCount is below minimum", () => {
166+
const noTools = turnSummary({ turnId: "t-no-tools", toolCount: 0, toolNs: 0 });
167+
expect(() => assertTurnHasInferenceAndTools(noTools)).toThrow(
168+
/turn t-no-tools: expected toolCount >= 1/,
169+
);
170+
171+
const oneTool = turnSummary({ turnId: "t-one", toolCount: 1, toolNs: 100 });
172+
expect(() => assertTurnHasInferenceAndTools(oneTool, { minToolCount: 2 })).toThrow(
173+
/turn t-one: expected toolCount >= 2/,
174+
);
175+
});
176+
177+
test("throws when toolNs is not positive despite toolCount", () => {
178+
expect(() =>
179+
assertTurnHasInferenceAndTools(
180+
turnSummary({ turnId: "t-zero-tool-ns", toolCount: 1, toolNs: 0 }),
181+
),
182+
).toThrow(/turn t-zero-tool-ns: expected toolNs > 0/);
183+
});
184+
185+
test("fails when tools are filtered out of the golden fixture", () => {
118186
const spans: PerfSpan[] = multiToolTurnFixture().filter((s) => s.name !== "tool");
119187
const turns = rollupByTurn(spans);
120188
expect(() => assertTurnHasInferenceAndTools(turns[0]!)).toThrow(/toolCount/);
121189
});
190+
});
122191

123-
test("TTFT is less than stream on the golden fixture", () => {
124-
const turn = rollupByTurn(multiToolTurnFixture())[0]!;
125-
assertLessThan(turn.ttftNs, turn.streamNs, "ttft vs stream");
126-
expect(turn.ttftNs).toBe(400);
127-
expect(turn.streamNs).toBe(1600);
192+
describe("golden multi-tool turn fixture", () => {
193+
test("rollupByTurn matches locked golden values", () => {
194+
const turns = rollupByTurn(multiToolTurnFixture());
195+
expect(turns).toHaveLength(1);
196+
expect(turns[0]).toEqual({ ...MULTI_TOOL_TURN_GOLDEN });
197+
});
198+
199+
test("fixture tags are privacy-safe (allowlisted keys only)", () => {
200+
for (const span of multiToolTurnFixture()) {
201+
if (span.tags === undefined) continue;
202+
for (const key of Object.keys(span.tags)) {
203+
expect(ALLOWED_TAG_KEY_SET.has(key)).toBe(true);
204+
}
205+
}
128206
});
129207

130-
test("session totals include tool and inference cost", () => {
131-
const totals = sessionTotals(multiToolTurnFixture());
132-
expect(totals.turnCount).toBe(1);
133-
expect(totals.totalInferenceNs).toBe(2000);
134-
expect(totals.totalToolNs).toBe(1200);
135-
expect(totals.totalToolCount).toBe(2);
136-
expect(totals.ttftShare).toBeCloseTo(0.2, 5);
137-
expect(totals.streamShare).toBeCloseTo(0.8, 5);
208+
test("TTFT is strictly less than stream on the golden fixture", () => {
209+
const turn = rollupByTurn(multiToolTurnFixture())[0]!;
210+
assertLessThan(turn.ttftNs, turn.streamNs, "ttft vs stream");
138211
});
139212
});
140213

@@ -170,16 +243,18 @@ describe("observer pipeline → snapshot → rollup → assertions", () => {
170243

171244
const turns = rollupByTurn(spans);
172245
expect(turns).toHaveLength(1);
173-
assertTurnHasInferenceAndTools(turns[0]!);
246+
assertTurnHasInferenceAndTools(turns[0]!, { minToolCount: 2 });
174247
expect(turns[0]!.toolCount).toBe(2);
175248

176-
// Live clock: TTFT ends at/before stream starts, so ttftNs should be <= streamNs
177-
// only when both are positive; with real hrtime, stream wall is typically longer.
178-
if (turns[0]!.ttftNs > 0 && turns[0]!.streamNs > 0) {
179-
// Relative magnitude: first-token wait should not dominate a multi-token stream
180-
// in the happy path (stream duration is from first token to done).
181-
expect(turns[0]!.streamNs).toBeGreaterThanOrEqual(0);
182-
expect(turns[0]!.ttftNs).toBeGreaterThanOrEqual(0);
249+
// Live clock: duration magnitudes are non-deterministic under sync hrtime
250+
// (ttftNs can exceed streamNs). Assert wall ordering instead of a no-op
251+
// `>= 0` check: TTFT must end at or before stream starts when both exist.
252+
const ttft = spans.find((s) => s.name === "inference.ttft");
253+
const stream = spans.find((s) => s.name === "inference.stream");
254+
expect(ttft?.endNs).toBeDefined();
255+
expect(stream?.startNs).toBeDefined();
256+
if (ttft!.endNs !== undefined && stream !== undefined) {
257+
expect(ttft!.endNs <= stream.startNs).toBe(true);
183258
}
184259

185260
const phases = rollupByPhase(spans);

src/perf/assert-spans.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ export function assertPhasePresent(
2626
/**
2727
* Verify at least one span named `childName` is nested under a span named
2828
* `parentName` (via parentId → id).
29+
*
30+
* Arg order: (spans, child, parent) — the nested phase first, then its expected
31+
* parent. Example: `assertNesting(spans, "inference", "turn")` means an
32+
* inference span has parentId pointing at a turn span.
2933
*/
3034
export function assertNesting(
3135
spans: readonly PerfSpan[],
@@ -45,19 +49,28 @@ export function assertNesting(
4549
}
4650
}
4751

52+
export type TurnInferenceToolsOpts = {
53+
/** Minimum tool invocations required (default 1). */
54+
minToolCount?: number;
55+
};
56+
4857
/**
4958
* Regression: a turn that ran tools must report positive inference and tool cost.
5059
* Accepts a single TurnSummary (from rollupByTurn).
5160
*/
52-
export function assertTurnHasInferenceAndTools(turn: TurnSummary): void {
61+
export function assertTurnHasInferenceAndTools(
62+
turn: TurnSummary,
63+
opts?: TurnInferenceToolsOpts,
64+
): void {
65+
const minToolCount = opts?.minToolCount ?? 1;
5366
if (turn.inferenceNs <= 0) {
5467
throw new Error(
5568
`turn ${turn.turnId}: expected inferenceNs > 0, got ${turn.inferenceNs}`,
5669
);
5770
}
58-
if (turn.toolCount <= 0) {
71+
if (turn.toolCount < minToolCount) {
5972
throw new Error(
60-
`turn ${turn.turnId}: expected toolCount > 0 when tools ran, got ${turn.toolCount}`,
73+
`turn ${turn.turnId}: expected toolCount >= ${minToolCount} when tools ran, got ${turn.toolCount}`,
6174
);
6275
}
6376
if (turn.toolNs <= 0) {

0 commit comments

Comments
 (0)