Procházet zdrojové kódy

test(acp-snapshot): replace the authored-implies-override guard with an explicit overridden flag

kingwl před 2 měsíci
rodič
revize
93d5e4c560

+ 1 - 1
docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md

@@ -68,7 +68,7 @@ The replay plugin lives in its own package, `@deepseek-ai/dsh-llm-replay` (`pack
 
 ### Two subcommands, replay in the default gate
 
-`pnpm run test:snapshot` runs replay (keyless) and is composed into the default `pnpm run test` gate so every PR gets the regression check (the main `vitest.config.ts` include stays narrow; the gate is `test && test:snapshot`). `pnpm run test:snapshot:record` requires `DEEPSEEK_API_KEY` (loaded from repo `.env` first), hits the real API, harvests the produced `session.jsonl` (the replay source AND the expected-log artifact), and `--update`s the stdout golden in one pass. Both forward a scenario filter. A missing fixture in replay **fails loud** with a "record first" message rather than self-skipping (the e2e self-skip rule is a CI-secret accommodation, not appropriate here — a committed-fixture test that silently vanishes is a coverage hole). A no-model scenario's `session.jsonl` simply has no `assistant/chunk` events (empty derived script); fail-loud still applies if a model call happens with no entry. An orphan-fixture guard test fails on a golden/fixture not referenced by any scenario (Vitest does not prune orphaned raw goldens), and a per-kind required-fixture guard asserts each scenario ships exactly the files its kind needs (`input.json` + `stdout.golden.jsonl` + `session.jsonl` for ALL scenarios — the harness passes `<dir>/session.jsonl` to `llm-replay` unconditionally, so even a no-model scenario needs its header-only fixture or `loadReplayScript()` fails; `replay.override.json` additionally for authored model scenarios).
+`pnpm run test:snapshot` runs replay (keyless) and is composed into the default `pnpm run test` gate so every PR gets the regression check (the main `vitest.config.ts` include stays narrow; the gate is `test && test:snapshot`). `pnpm run test:snapshot:record` requires `DEEPSEEK_API_KEY` (loaded from repo `.env` first), hits the real API, harvests the produced `session.jsonl` (the replay source AND the expected-log artifact), and `--update`s the stdout golden in one pass. Both forward a scenario filter. A missing fixture in replay **fails loud** with a "record first" message rather than self-skipping (the e2e self-skip rule is a CI-secret accommodation, not appropriate here — a committed-fixture test that silently vanishes is a coverage hole). A no-model scenario's `session.jsonl` simply has no `assistant/chunk` events (empty derived script); fail-loud still applies if a model call happens with no entry. An orphan-fixture guard test fails on a golden/fixture not referenced by any scenario (Vitest does not prune orphaned raw goldens), and a per-kind required-fixture guard asserts each scenario ships exactly the files its kind needs (`input.json` + `stdout.golden.jsonl` + `session.jsonl` for ALL scenarios — the harness passes `<dir>/session.jsonl` to `llm-replay` unconditionally, so even a no-model scenario needs its header-only fixture or `loadReplayScript()` fails; `replay.override.json` exactly for the scenarios whose table entry sets `overridden` — required with the flag, forbidden without it, because the harness forwards the sidecar purely on file existence and an unregistered stray would silently replace the derived script).
 
 ## Alternatives considered
 

+ 22 - 11
packages/support/acp-snapshot/src/suite.ts

@@ -52,12 +52,22 @@ export interface Scenario {
   /**
    * Whether `test:snapshot:record` regenerates this scenario's `session.jsonl`
    * from the LIVE API. `recorded` scenarios are model-driven and reproducible;
-   * `authored` scenarios (a hand-written `replay.override.json` sidecar drives
-   * replay — e.g. a provider error or a cancel, which the live API can't be
-   * coaxed into deterministically — or a deterministic hook scenario whose
-   * derived empty script needs no sidecar) are NEVER re-recorded.
+   * `authored` scenarios (fixtures hand-written or hand-harvested — e.g. a
+   * provider error or a cancel the live API can't be coaxed into
+   * deterministically, a deterministic hook scenario, or a scripted repetition
+   * a live model won't reproduce) are NEVER re-recorded.
    */
   recorded: boolean
+  /**
+   * Whether replay is driven by a hand-written `replay.override.json` sidecar
+   * (a `ReplayEntry[]` that REPLACES the script derived from `session.jsonl`)
+   * — the throw/hang cases chunks cannot express. The fixture guard requires
+   * the sidecar exactly when this is set: the harness forwards the file purely
+   * on existence, so an unregistered stray sidecar would silently replace the
+   * derived script — the guard fails loud on either mismatch. Defaults to
+   * false (replay derives from the fixture's `assistant/chunk` events).
+   */
+  overridden?: boolean
   /**
    * How many SUBAGENT child sessions this scenario records beyond the top-level
    * one (0 for a single-session scenario). Each child rides in a sibling fixture
@@ -317,17 +327,18 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void {
       // throws "fixture not found" when it is absent and no override replaces it.
       // A no-model scenario ships a header-only `session.jsonl` (it derives to an
       // empty script — no model call is made); a model scenario's fixture also
-      // doubles as the expected-log artifact the run is diffed against. An authored
-      // (non-`recorded`) model scenario additionally ships a `replay.override.json`
-      // sidecar for the throw/hang cases a derived script cannot express.
-      for (const { name, hasModelTurn, recorded, childSessions } of scenarios) {
+      // doubles as the expected-log artifact the run is diffed against. The
+      // `replay.override.json` sidecar is matched BOTH ways against the table's
+      // `overridden` flag: required when set, forbidden when not — the harness
+      // forwards the file purely on existence, so an unregistered stray sidecar
+      // would silently replace the derived script.
+      for (const { name, overridden, childSessions } of scenarios) {
         const dir = join(snapshotsDir, name)
         expect(existsSync(join(dir, 'input.json')), `${name}/input.json`).toBe(true)
         expect(existsSync(join(dir, 'stdout.golden.jsonl')), `${name}/stdout.golden.jsonl`).toBe(true)
         expect(existsSync(join(dir, 'session.jsonl')), `${name}/session.jsonl`).toBe(true)
-        if (hasModelTurn && !recorded) {
-          expect(existsSync(join(dir, 'replay.override.json')), `${name}/replay.override.json`).toBe(true)
-        }
+        expect(existsSync(join(dir, 'replay.override.json')), `${name}/replay.override.json presence must match \`overridden\``)
+          .toBe(overridden === true)
         // A nested-agent scenario ships one child fixture per recorded subagent
         // session (`session.1.jsonl` …), the replay source for that child session.
         for (const childFixture of childFixturePaths(dir, childSessions ?? 0)) {

+ 2 - 2
packages/support/acp-snapshot/tests/suite.spec.ts

@@ -39,14 +39,14 @@ const REPLAY_SCENARIOS: Scenario[] = [
   { name: 'plain-turn', hasModelTurn: true, recorded: true, childSessions: 1 },
   { name: 'no-model', hasModelTurn: false, recorded: false },
   { name: 'blocked-log', hasModelTurn: false, comparesLog: true, recorded: false },
-  { name: 'authored-error', hasModelTurn: true, recorded: false },
+  { name: 'authored-error', hasModelTurn: true, recorded: false, overridden: true },
 ]
 
 const RECORD_SCENARIOS: Scenario[] = [
   { name: 'rec-pin', hasModelTurn: true, recorded: true, pinsHeader: true },
   { name: 'rec-child', hasModelTurn: true, recorded: true, childSessions: 1 },
   // recorded:false in record mode → registered but skipped (never re-recorded).
-  { name: 'rec-skip', hasModelTurn: true, recorded: false },
+  { name: 'rec-skip', hasModelTurn: true, recorded: false, overridden: true },
 ]
 
 // Record mode mutates its snapshots dir, so run it on a throwaway copy —