mirror of
https://github.com/deepseek-ai/deepseek-harness
synced 2026-08-15 21:04:50 +00:00
Merge pull request #526 from deepseek-harness/worktree/pr338-preserve-chunk-envelope
fix(snapshot): preserve packed chunk timing on refresh
This commit is contained in:
@@ -18,7 +18,7 @@ The machinery lives in [`packages/support/acp-snapshot`](../../../../packages/su
|
||||
|
||||
**`src/normalize.ts`** — the pure normalizers, hook-free by policy: when a future event carries a new volatile field (an approval duration, say), the shared normalizer learns it in the same change, keeping one home for what "normalized" means rather than per-suite scrub extensions.
|
||||
|
||||
**`src/suite.ts`** — the `Scenario` type and `defineAcpSnapshotSuite(options)`, registering the per-scenario compares, record/refresh fixture write-back, the header pin with its live uniformity guard, and the fixture guard block (no orphan scenario dirs, required files present, exactly one pin per class, every JSONL a `scrubSystemPrompts` fixed point, non-pinning fixtures also `scrubRequestHeaders` fixed points). A scenario directory's `session.jsonl` plus contiguous `session.<n>.jsonl` siblings are its ordered primary/child inventory, so the scenario table declares policy without duplicating a child count. The pinned-header contract ([pinned-header Agent Note](2026-07-06-pin-request-header-content-in-one-scenario.md)) is per-suite: each header class flags exactly one `pinsHeader` scenario, whose `system-prompt.expected.md` and JSONL tool list split the composed header into reviewable artifacts; the uniformity guard compares both against every live header in that class. A pinning scenario declares any legitimate changed-header count, and its Markdown artifact records every full changed prompt. The pure helpers (`sessionFixtureNames`, `fixtureContext`, `normalizedHeaders`, `normalizedSystemPrompts`, `formatSystemPromptSnapshot`, `headerChangeCount`) are exported from the module for direct unit coverage.
|
||||
**`src/suite.ts`** — the `Scenario` type and `defineAcpSnapshotSuite(options)`, registering the per-scenario compares, record/refresh fixture write-back, the header pin with its live uniformity guard, and the fixture guard block (no orphan scenario dirs, required files present, exactly one pin per class, every JSONL a `scrubSystemPrompts` fixed point, non-pinning fixtures also `scrubRequestHeaders` fixed points). Refresh expands packed timing envelopes before aligning existing volatile event times, so switching between packed and unpacked layouts cannot shift later records; fresh chunk-fragment arrays remain authoritative because their boundaries are replay behavior. A scenario directory's `session.jsonl` plus contiguous `session.<n>.jsonl` siblings are its ordered primary/child inventory, so the scenario table declares policy without duplicating a child count. The pinned-header contract ([pinned-header Agent Note](2026-07-06-pin-request-header-content-in-one-scenario.md)) is per-suite: each header class flags exactly one `pinsHeader` scenario, whose `system-prompt.expected.md` and JSONL tool list split the composed header into reviewable artifacts; the uniformity guard compares both against every live header in that class. A pinning scenario declares any legitimate changed-header count, and its Markdown artifact records every full changed prompt. The pure helpers (`sessionFixtureNames`, `fixtureContext`, `normalizedHeaders`, `normalizedSystemPrompts`, `formatSystemPromptSnapshot`, `headerChangeCount`) are exported from the module for direct unit coverage.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
|
||||
@@ -5,14 +5,14 @@
|
||||
{"type":"step/start","seq":3,"time":1783600629542,"data":{"turn":1,"step":1}}
|
||||
{"type":"request/header","seq":4,"time":1783600629542,"data":{"header":{"config":{"provider":"deepseek","model":"deepseek-v4-flash"},"system":"{{system}}","tools":"{{tools}}"},"reason":"initial"}}
|
||||
{"type":"assistant/chunk","seq":5,"time":1783600630819,"data":{"turn":1,"step":1,"chunk":{"type":"block-start","index":0,"blockType":"reasoning"}}}
|
||||
{"type":"reasoning-chunks","seq0":6,"time0":1784709770168,"data":{"turn":1,"step":1,"index":0,"dt":[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1,0],"texts":["The"," user"," wants"," me"," to"," reply"," with"," exactly"," the"," word"," \"","P","ONG","\""," and"," not"," use"," any"," tools","."]}}
|
||||
{"type":"assistant/chunk","seq":26,"time":1783600630822,"data":{"turn":1,"step":1,"chunk":{"type":"block-start","index":1,"blockType":"text"}}}
|
||||
{"type":"assistant/chunk","seq":27,"time":1783600630852,"data":{"turn":1,"step":1,"chunk":{"type":"text-delta","index":1,"text":"P"}}}
|
||||
{"type":"assistant/chunk","seq":28,"time":1783600630852,"data":{"turn":1,"step":1,"chunk":{"type":"text-delta","index":1,"text":"ONG"}}}
|
||||
{"type":"assistant/chunk","seq":29,"time":1783600630852,"data":{"turn":1,"step":1,"chunk":{"type":"block-end","index":0,"block":{"type":"reasoning","text":"The user wants me to reply with exactly the word \"PONG\" and not use any tools."}}}}
|
||||
{"type":"assistant/chunk","seq":30,"time":1783600630852,"data":{"turn":1,"step":1,"chunk":{"type":"block-end","index":1,"block":{"type":"text","text":"PONG"}}}}
|
||||
{"type":"assistant/chunk","seq":31,"time":1783600630885,"data":{"turn":1,"step":1,"chunk":{"type":"usage","usage":{"inputTokens":3091,"outputTokens":23,"cacheReadTokens":0,"reasoningTokens":20}}}}
|
||||
{"type":"assistant/chunk","seq":32,"time":1783600630886,"data":{"turn":1,"step":1,"chunk":{"type":"finish","reason":{"kind":"stop"}}}}
|
||||
{"type":"assistant/message","seq":33,"time":1784709770169,"data":{"turn":1,"step":1,"content":[{"type":"reasoning","text":"The user wants me to reply with exactly the word \"PONG\" and not use any tools."},{"type":"text","text":"PONG"}],"provenance":{"provider":"deepseek","model":"deepseek-v4-flash"},"usage":{"inputTokens":3091,"outputTokens":23,"cacheReadTokens":0,"reasoningTokens":20}},"sourceEventSeqs":[5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32],"surfaceOp":"append"}
|
||||
{"type":"step/end","seq":34,"time":1784709770170,"data":{"turn":1,"step":1}}
|
||||
{"type":"turn/end","seq":35,"time":1784709770170,"data":{"turn":1,"reason":{"kind":"completed"}}}
|
||||
{"type":"reasoning-chunks","seq0":6,"time0":1783600630820,"data":{"turn":1,"step":1,"index":0,"dt":[2,30,0,0,0,33,1,40,0,0,0,0,0,18,0,36,0,0,0],"texts":["The"," user"," wants"," me"," to"," reply"," with"," exactly"," the"," word"," \"","P","ONG","\""," and"," not"," use"," any"," tools","."]}}
|
||||
{"type":"assistant/chunk","seq":26,"time":1783600630980,"data":{"turn":1,"step":1,"chunk":{"type":"block-start","index":1,"blockType":"text"}}}
|
||||
{"type":"assistant/chunk","seq":27,"time":1783600630980,"data":{"turn":1,"step":1,"chunk":{"type":"text-delta","index":1,"text":"P"}}}
|
||||
{"type":"assistant/chunk","seq":28,"time":1783600631006,"data":{"turn":1,"step":1,"chunk":{"type":"text-delta","index":1,"text":"ONG"}}}
|
||||
{"type":"assistant/chunk","seq":29,"time":1783600631008,"data":{"turn":1,"step":1,"chunk":{"type":"block-end","index":0,"block":{"type":"reasoning","text":"The user wants me to reply with exactly the word \"PONG\" and not use any tools."}}}}
|
||||
{"type":"assistant/chunk","seq":30,"time":1783600631009,"data":{"turn":1,"step":1,"chunk":{"type":"block-end","index":1,"block":{"type":"text","text":"PONG"}}}}
|
||||
{"type":"assistant/chunk","seq":31,"time":1783600631009,"data":{"turn":1,"step":1,"chunk":{"type":"usage","usage":{"inputTokens":3091,"outputTokens":23,"cacheReadTokens":0,"reasoningTokens":20}}}}
|
||||
{"type":"assistant/chunk","seq":32,"time":1783600631009,"data":{"turn":1,"step":1,"chunk":{"type":"finish","reason":{"kind":"stop"}}}}
|
||||
{"type":"assistant/message","seq":33,"time":1783600631011,"data":{"turn":1,"step":1,"content":[{"type":"reasoning","text":"The user wants me to reply with exactly the word \"PONG\" and not use any tools."},{"type":"text","text":"PONG"}],"provenance":{"provider":"deepseek","model":"deepseek-v4-flash"},"usage":{"inputTokens":3091,"outputTokens":23,"cacheReadTokens":0,"reasoningTokens":20}},"sourceEventSeqs":[5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32],"surfaceOp":"append"}
|
||||
{"type":"step/end","seq":34,"time":1783600631011,"data":{"turn":1,"step":1}}
|
||||
{"type":"turn/end","seq":35,"time":1783600631011,"data":{"turn":1,"reason":{"kind":"completed"}}}
|
||||
|
||||
@@ -7,7 +7,7 @@ Four layers, importable separately:
|
||||
- **`launchAcpTestAgent` (launcher)** — boots a source agent under tsx or a built `lib` agent under plain Node from a temp cwd, connects the SDK client over a raw-byte stdout tee, collects session updates and stderr, surfaces asynchronous spawn failures through startup, fails closed on unhandled permission requests, and owns graceful or signalled shutdown. Shutdown waits for process exit, inherited stdio closure, and ACP parser exhaustion before resolving or propagating a child error, so captures are complete and callers can remove owned paths after either outcome. When Windows accepts forced termination but publishes its exit marker asynchronously, shutdown gives that marker a bounded grace before treating fallback refusal as a second failure. Snapshot and ordinary e2e suites share this process boundary; a test supplies only agent paths, cwd, environment overrides, and any permission policy.
|
||||
- **`runScenario` (harness)** — drives ACP JSON-RPC stdio from a deterministic `input.json` script through the launcher, tees raw stdout for the expected-output and purity checks, and harvests every persisted raw JSONL session log (parent and subagent children, primary-first) after graceful stdin EOF. `AgentUnderTest` supplies absolute `binScript`, optional `libBinScript`, `configPath`, and `tsconfigPath` paths because the subprocess cwd is outside the repo. Startup failures preserve captured agent stderr in the rejected diagnostic.
|
||||
- **Normalizers** — pure functions turning the two captured surfaces into stable text: `normalizeStdout` (JSON-RPC ids → first-seen sequence; UUIDs/cwd → tokens; cwd-rooted separators selected as canonical `/` or host-native; `session_info_update.updatedAt` → `{{updatedAt}}`; doubles as the stdout-purity check), `normalizeSessionLog` (times zeroed, `seq` kept, the same cwd-path policy), `scrubSystemPrompts` (prompt text → `{{system}}`), `scrubToolSchemas` (schema bulk → `{{tools}}`), and `scrubRequestHeaders` (all header bulk → `{{system}}`/`{{tools}}`/`{{messagePrefix}}` outside each pin, structure kept — [pinned-header Agent Note](../../../.agents/notes/implemented/testing/2026-07-06-pin-request-header-content-in-one-scenario.md)).
|
||||
- **`defineAcpSnapshotSuite` (factory)** — registers the whole describe/it tree for a scenario table: per-scenario expected-output and re-persisted-log comparisons, record/refresh fixture write-back, rejection of structured `UNKNOWN_TOOL` results, the per-header-class pin (`system-prompt.expected.md` plus `tool-schemas.expected.json`) with its live uniformity guard, and the fixture guard block (no orphan scenario dirs, required files present, exactly one pin per class, every JSONL prompt/schema-scrubbed, non-pinning fixtures fully header-scrubbed). Refresh preserves existing volatile fields by event position and gives a newly inserted `session/title` its preceding event's time, so feature-driven insertions do not churn the remainder of a fixture. Each scenario directory's `session.jsonl` plus contiguous `session.<n>.jsonl` siblings are the ordered primary/child inventory; the scenario table does not duplicate their count. Must be called at vitest collection time.
|
||||
- **`defineAcpSnapshotSuite` (factory)** — registers the whole describe/it tree for a scenario table: per-scenario expected-output and re-persisted-log comparisons, record/refresh fixture write-back, rejection of structured `UNKNOWN_TOOL` results, the per-header-class pin (`system-prompt.expected.md` plus `tool-schemas.expected.json`) with its live uniformity guard, and the fixture guard block (no orphan scenario dirs, required files present, exactly one pin per class, every JSONL prompt/schema-scrubbed, non-pinning fixtures fully header-scrubbed). Refresh expands packed timing envelopes before aligning existing volatile event times, so switching between packed and unpacked layouts cannot shift later records; fresh chunk-fragment arrays remain authoritative. A newly inserted `session/title` receives its preceding event's time so feature-driven insertions do not churn the remainder of a fixture. Each scenario directory's `session.jsonl` plus contiguous `session.<n>.jsonl` siblings are the ordered primary/child inventory; the scenario table does not duplicate their count. Must be called at vitest collection time.
|
||||
|
||||
A consuming `*.snapshot.ts` is the scenario table plus one factory call:
|
||||
|
||||
|
||||
@@ -42,6 +42,8 @@ const WINDOWS_STDOUT_SNAPSHOT = 'stdout.expected.windows.jsonl'
|
||||
/** Stable session-log token standing in for the sidecar's initial schemas. */
|
||||
const TOOLS_TOKEN = '{{tools}}'
|
||||
|
||||
const PACKED_CHUNK_ROW_TYPES = new Set(['text-chunks', 'reasoning-chunks', 'tool-call-chunks'])
|
||||
|
||||
/** A snapshot scenario and how its fixtures are produced. */
|
||||
export interface Scenario {
|
||||
name: string
|
||||
@@ -398,6 +400,23 @@ function parseJsonlRecords(text: string): Record<string, unknown>[] {
|
||||
.map(line => JSON.parse(line) as Record<string, unknown>)
|
||||
}
|
||||
|
||||
/** One packed row's member times, or `undefined` for an ordinary record. */
|
||||
function packedTimes(record: Record<string, unknown>): number[] | undefined {
|
||||
if (!PACKED_CHUNK_ROW_TYPES.has(record.type as string)) return undefined
|
||||
const row = record as unknown as { time0: number; data: { dt: number[] } }
|
||||
const times = [row.time0]
|
||||
for (const gap of row.data.dt) times.push((times[times.length - 1] as number) + gap)
|
||||
return times
|
||||
}
|
||||
|
||||
/** Expand packed timing envelopes so refresh alignment follows logical events, not physical lines. */
|
||||
function logicalRecords(records: Record<string, unknown>[]): Record<string, unknown>[] {
|
||||
return records.flatMap((record) => {
|
||||
const times = packedTimes(record)
|
||||
return times === undefined ? [record] : times.map(time => ({ type: 'assistant/chunk', time }))
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Find tool calls whose structured result reports `UNKNOWN_TOOL`.
|
||||
*
|
||||
@@ -463,11 +482,32 @@ function preserveFixtureVolatiles(record: Record<string, unknown>, existing: Rec
|
||||
}
|
||||
}
|
||||
|
||||
/** Carry logical member times into a fresh packed row while leaving its fragment arrays untouched. */
|
||||
function preservePackedMemberTimes(
|
||||
record: Record<string, unknown>,
|
||||
existingMembers: Record<string, unknown>[],
|
||||
): void {
|
||||
if (!PACKED_CHUNK_ROW_TYPES.has(record.type as string)) return
|
||||
const row = record as unknown as { time0: number; data: { dt: number[] } }
|
||||
const firstTime = existingMembers[0]?.time
|
||||
if (!Number.isSafeInteger(firstTime)) return
|
||||
row.time0 = firstTime as number
|
||||
if (existingMembers.length !== row.data.dt.length + 1) return
|
||||
const times = existingMembers.map(member => Number.isSafeInteger(member.time) ? member.time as number : undefined)
|
||||
if (times.some(time => time === undefined)) return
|
||||
const memberTimes = times as number[]
|
||||
const gaps = memberTimes.slice(1).map((time, index) => time - (memberTimes[index] as number))
|
||||
if (gaps.some(gap => !Number.isSafeInteger(gap))) return
|
||||
row.data.dt = gaps
|
||||
}
|
||||
|
||||
/**
|
||||
* Rewrite a fresh replay-produced log so repeated refreshes do not churn
|
||||
* volatile fixture fields. Meaningful event payloads come from `fresh`; the
|
||||
* existing fixture lends session ids, cwd, creation times, event times, and
|
||||
* hook durations where the record shape still matches.
|
||||
* existing fixture lends session ids, cwd, creation times, logical event
|
||||
* times, and hook durations where the record shape still matches. Packed
|
||||
* timing envelopes expand for alignment, so packing does not shift later
|
||||
* records; fresh fragment arrays remain authoritative.
|
||||
*
|
||||
* @param fresh The newly harvested session JSONL.
|
||||
* @param existing The committed fixture JSONL being refreshed.
|
||||
@@ -477,21 +517,23 @@ function preserveFixtureVolatiles(record: Record<string, unknown>, existing: Rec
|
||||
export function stabilizeRefreshLog(fresh: string, existing: string, replacements: FixtureReplacement[]): string {
|
||||
let stable = fresh
|
||||
for (const { from, to } of replacements) stable = stable.split(from).join(to)
|
||||
const existingRecords = parseJsonlRecords(existing)
|
||||
const existingRecords = logicalRecords(parseJsonlRecords(existing))
|
||||
const records = parseJsonlRecords(stable)
|
||||
let existingIndex = 0
|
||||
let previousEventTime: unknown
|
||||
for (let i = 0; i < records.length; i++) {
|
||||
const record = records[i] as Record<string, unknown>
|
||||
const existingRecord = existingRecords[existingIndex]
|
||||
const memberCount = packedTimes(record)?.length ?? 1
|
||||
const insertedTitle = record.type === 'session/title' && existingRecord?.type !== 'session/title'
|
||||
if (insertedTitle) {
|
||||
/* v8 ignore next -- a title is turn-enclosed, so a preceding event time exists in every valid fixture. */
|
||||
if (typeof previousEventTime !== 'number') throw new Error('acp-snapshot: inserted title has no preceding event time')
|
||||
record.time = previousEventTime
|
||||
} else {
|
||||
preservePackedMemberTimes(record, existingRecords.slice(existingIndex, existingIndex + memberCount))
|
||||
preserveFixtureVolatiles(record, existingRecord)
|
||||
existingIndex += 1
|
||||
existingIndex += memberCount
|
||||
}
|
||||
if (typeof record.time === 'number') previousEventTime = record.time
|
||||
}
|
||||
|
||||
@@ -451,6 +451,76 @@ describe('refreshFixtureReplacements', () => {
|
||||
})
|
||||
|
||||
describe('stabilizeRefreshLog', () => {
|
||||
it('preserves unpacked member times when refresh first packs a chunk run', () => {
|
||||
const fresh = [
|
||||
'{"type":"session","id":"same","createdAt":200}',
|
||||
'{"type":"reasoning-chunks","seq0":2,"time0":200,"data":{"turn":1,"step":1,"index":0,"dt":[5,7],"texts":["new",""," split"]}}',
|
||||
'{"type":"assistant/message","seq":5,"time":220,"data":{}}',
|
||||
'',
|
||||
].join('\n')
|
||||
const existing = [
|
||||
'{"type":"session","id":"same","createdAt":100}',
|
||||
'{"type":"assistant/chunk","seq":2,"time":100,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":"old"}}}',
|
||||
'{"type":"assistant/chunk","seq":3,"time":101,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":"chunk"}}}',
|
||||
'{"type":"assistant/chunk","seq":4,"time":103,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":"shape"}}}',
|
||||
'{"type":"assistant/message","seq":5,"time":104,"data":{}}',
|
||||
'',
|
||||
].join('\n')
|
||||
|
||||
expect(stabilizeRefreshLog(fresh, existing, [])).toBe([
|
||||
'{"type":"session","id":"same","createdAt":100}',
|
||||
'{"type":"reasoning-chunks","seq0":2,"time0":100,"data":{"turn":1,"step":1,"index":0,"dt":[1,2],"texts":["new",""," split"]}}',
|
||||
'{"type":"assistant/message","seq":5,"time":104,"data":{}}',
|
||||
'',
|
||||
].join('\n'))
|
||||
})
|
||||
|
||||
it('preserves packed member times without flattening fresh chunk boundaries', () => {
|
||||
const fresh = [
|
||||
'{"type":"session","id":"same","createdAt":200}',
|
||||
'{"type":"text-chunks","seq0":2,"time0":200,"data":{"turn":1,"step":1,"index":0,"dt":[5,7],"texts":["new",""," split"]}}',
|
||||
'',
|
||||
].join('\n')
|
||||
const existing = [
|
||||
'{"type":"session","id":"same","createdAt":100}',
|
||||
'{"type":"text-chunks","seq0":2,"time0":100,"data":{"turn":1,"step":1,"index":0,"dt":[1,2],"texts":["old","chunk","shape"]}}',
|
||||
'',
|
||||
].join('\n')
|
||||
|
||||
expect(stabilizeRefreshLog(fresh, existing, [])).toBe([
|
||||
'{"type":"session","id":"same","createdAt":100}',
|
||||
'{"type":"text-chunks","seq0":2,"time0":100,"data":{"turn":1,"step":1,"index":0,"dt":[1,2],"texts":["new",""," split"]}}',
|
||||
'',
|
||||
].join('\n'))
|
||||
})
|
||||
|
||||
it.each([
|
||||
['the old run is absent', [], 200],
|
||||
['the old run is shorter', [100, 101], 100],
|
||||
['a later old time is invalid', [100, 'invalid', 103], 100],
|
||||
['an old gap is not exactly representable', [Number.MIN_SAFE_INTEGER, Number.MAX_SAFE_INTEGER - 1, Number.MAX_SAFE_INTEGER - 1], Number.MIN_SAFE_INTEGER],
|
||||
])('keeps fresh packed gaps when %s', (_case, existingTimes, expectedTime0) => {
|
||||
const freshRow = {
|
||||
type: 'reasoning-chunks',
|
||||
seq0: 2,
|
||||
time0: 200,
|
||||
data: { turn: 1, step: 1, index: 0, dt: [5, 7], texts: ['new', '', ' split'] },
|
||||
}
|
||||
const existingRows = existingTimes.map((time, index) => ({
|
||||
type: 'assistant/chunk',
|
||||
seq: index + 2,
|
||||
time,
|
||||
data: {},
|
||||
}))
|
||||
const output = stabilizeRefreshLog(
|
||||
`${JSON.stringify({ type: 'session', id: 'same', createdAt: 200 })}\n${JSON.stringify(freshRow)}\n`,
|
||||
`${JSON.stringify({ type: 'session', id: 'same', createdAt: 100 })}\n${existingRows.map(row => JSON.stringify(row)).join('\n')}\n`,
|
||||
[],
|
||||
).trim().split('\n').map(line => JSON.parse(line) as Record<string, unknown>)
|
||||
|
||||
expect(output[1]).toStrictEqual({ ...freshRow, time0: expectedTime0 })
|
||||
})
|
||||
|
||||
it('aligns volatile times across a newly inserted log event', () => {
|
||||
const fresh = [
|
||||
'{"type":"session","id":"same","createdAt":200}',
|
||||
|
||||
Reference in New Issue
Block a user