Skip to content

Commit e2e41c9

Browse files
committed
fix(agent): give the harness its margin, and name the mandatory points correctly
Four review findings, all correct. `testTimeout` was set to exactly `DEFAULT_TURN_TIMEOUT_MS`, which does not fix the race it was meant to fix — equal deadlines only make it unbiased. The harness has to win for a slow turn to be classified as TIMEOUT instead of dying as a killed worker, so the vitest deadline now sits 30 s above it. `overBudget` listed shape changes, non-move events and parked-run ends, but the mandatory set also always holds the first and last sample: a two-point track under `maxPoints: 1` overflows for a reason the message did not name, and the model was told something false. Both the JSDoc and the message now say mandatory points, and enumerate all four kinds. And two comments: the test said the shape flips every 100 ms where `sweep` samples at 50 ms and alternates on every index, and a comment in `workbench/l0/` pointed at `README.md`, which from there resolves to a file that does not exist.
1 parent e9c482c commit e2e41c9

4 files changed

Lines changed: 23 additions & 19 deletions

File tree

src/lib/ai-edition/timeline/cursor-track.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,10 @@ describe("buildCursorTrack — compression", () => {
222222

223223
it("says so when the mandatory points push it over maxPoints", () => {
224224
// The ceiling is soft: `maxPoints` budgets the rate and the gap floor, and the
225-
// points nothing can put back are exempt. A recording that flips the pointer
226-
// every 100ms is all exempt points, so the budget cannot hold — and the track
227-
// has to say that rather than let the model read 100 rows as "within budget".
225+
// mandatory points are exempt. `sweep` samples every 50 ms and the shape here
226+
// alternates on every index, so every sample is mandatory and the budget cannot
227+
// hold — and the track has to say so rather than let the model read 100 rows as
228+
// "within budget".
228229
const flipping = sweep(200, { shape: (i) => (i % 2 === 0 ? "arrow" : "text") });
229230
const track = buildCursorTrack({
230231
assetId: "asset_1",

src/lib/ai-edition/timeline/cursor-track.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,12 @@ export interface CursorTrack {
6767
/** True when maxPoints forced a coarser rate than `hz` would give. */
6868
truncated: boolean;
6969
/** Present ONLY when the ceiling did not hold. `maxPoints` budgets the rate and
70-
* the gap floor; the points nothing can put back — a shape change, a non-move
71-
* event, the ends of a parked run — are exempt and stack on top, so a capture
72-
* rich in them lands above the ceiling. Absent means the budget held. It is a
73-
* separate field from `truncated` on purpose: that one says "you are seeing
74-
* less than you asked for", this one says the opposite. */
70+
* the gap floor; the MANDATORY points are exempt and stack on top — the first
71+
* and last sample, a pointer-shape change, a non-move event, the ends of a run
72+
* longer than the max gap — so a capture rich in them lands above the ceiling.
73+
* Absent means the budget held. It is a separate field from `truncated` on
74+
* purpose: that one says "you are seeing less than you asked for", this one
75+
* says the opposite. */
7576
overBudget?: string;
7677
/** When true, every point's virtual-timeline position equals its `atSec`, and
7778
* `virtualSec` is omitted from the points. Goes false as soon as a clip is
@@ -328,9 +329,10 @@ export function buildCursorTrack(options: CursorTrackOptions): CursorTrack {
328329
// field is absent when the budget held, so the common payload is unchanged.
329330
const overBudget =
330331
points.length > maxPoints
331-
? `${points.length} points for a ceiling of ${maxPoints}: pointer-shape changes, ` +
332-
`non-move events and the ends of a parked run are never dropped, and this ` +
333-
`recording has enough of them to land above the budget.`
332+
? `${points.length} points for a ceiling of ${maxPoints}: the mandatory points are ` +
333+
`never dropped — the first and last sample, pointer-shape changes, non-move ` +
334+
`events and the ends of a parked run — and this recording has enough of them ` +
335+
`to land above the budget.`
334336
: undefined;
335337

336338
return {

vitest.workbench.config.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,13 @@ export default defineConfig({
1616
globals: true,
1717
environment: "node",
1818
include: ["workbench/**/*.wb.ts"],
19-
// ponytail: imported, not recopied. A `.wb.ts` that drives a live turn is
20-
// cut by whichever cutoff fires first, and this one used to sit at 120 s
21-
// while the harness moved to 300 s — so vitest would have killed the turn
22-
// before the harness could say it had timed out, which is the failure the
23-
// harness comment exists to prevent. One constant, two enforcers.
24-
testTimeout: DEFAULT_TURN_TIMEOUT_MS,
19+
// ponytail: derived from the harness cutoff, and deliberately ABOVE it. A
20+
// `.wb.ts` driving a live turn is cut by whichever deadline fires first;
21+
// this one used to sit at 120 s while the harness moved to 300 s, so vitest
22+
// killed the turn before the harness could classify it. Equal values would
23+
// only make that race unbiased — the margin is what guarantees the harness
24+
// wins and the run gets a TIMEOUT verdict instead of a dead worker.
25+
testTimeout: DEFAULT_TURN_TIMEOUT_MS + 30_000,
2526
reporters: ["default"],
2627
// ponytail: the fixed cost of the suite is the dynamic
2728
// `await import("deepagents")` in chat-service.ts:346 — hundreds of

workbench/l0/real-fixture.wb.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,8 @@ describe("la fixture réelle — le document", () => {
7575
it("n'a PAS de piste caméra dans le document, quoi qu'il y ait sur le disque", () => {
7676
// Le dossier d'enregistrement contient bien un fichier webcam ; le document
7777
// écrit par l'app, lui, porte `cameraTrack: null`. La fixture ne corrige
78-
// pas ça — voir README.md § « La prise réelle ». Cette assertion existe pour
79-
// que personne ne « complète » la fixture sans s'en apercevoir : le modèle
78+
// pas ça — voir workbench/README.md § « La prise réelle ». Cette assertion
79+
// existe pour que personne ne « complète » la fixture sans le voir : le modèle
8080
// verra `hasCameraTrack: false`, et c'est l'état réel du projet.
8181
expect(realScreencastDocument().assets[0].cameraTrack).toBeNull();
8282
});

0 commit comments

Comments
 (0)