mirror of
https://github.com/deepseek-ai/deepseek-harness
synced 2026-08-15 21:04:50 +00:00
fix(session): break chunk runs on time gaps that cannot subtract exactly
Two safe-integer timestamps can differ by more than 2^53-1 (e.g. MIN_SAFE_INTEGER to MAX_SAFE_INTEGER-1), so the dt subtraction rounds and the packed row decodes to a timestamp one off the original -- violating the codec's lossless contract. Unreachable from a real clock (the gap needs ~285k years) but reachable from hand-written fixtures, and the decoder accepts hand-written rows. continues() now refuses to extend a run across such a gap (the check is exact both ways: an in-range true gap subtracts without rounding and passes; an out-of-range one rounds to an out-of-range value and fails), splitting the run instead -- whitelist philosophy, compression lost, data never. The decoder tightens to match the encoder's image: time0 and dt must be safe integers, and reconstructed member seqs/times must stay in safe range, so float reconstruction is exact wherever validation passes. The round-trip property now draws times from the full safe-integer range (it previously generated only small gaps, which is how this escaped); the bot's counterexample is pinned as an example test. Found by ds-review-bot on #338.
This commit is contained in:
@@ -135,6 +135,12 @@ function indexOf(event: DeltaEvent): number {
|
||||
/** Whether `next` extends a run ending in `prev` (same kind already checked by the caller). */
|
||||
function continues(prev: DeltaEvent, next: DeltaEvent, kind: DeltaKind): boolean {
|
||||
if (next.seq !== prev.seq + 1) return false
|
||||
// Two safe-integer times can sit further apart than a double subtracts
|
||||
// exactly (2^53-1 and its negation differ by ~2^54); a rounded gap would
|
||||
// decode to a different timestamp. The check is exact in both directions: a
|
||||
// true gap within safe range subtracts without rounding and passes, while a
|
||||
// true gap beyond it rounds to a value that is itself beyond and fails.
|
||||
if (!Number.isSafeInteger(next.time - prev.time)) return false
|
||||
if (next.data.turn !== prev.data.turn || next.data.step !== prev.data.step) return false
|
||||
if (indexOf(next) !== indexOf(prev)) return false
|
||||
if (kind !== 'tool-call-delta') return true
|
||||
@@ -219,8 +225,8 @@ function malformed(tag: string, why: string): never {
|
||||
throw new Error(`malformed ${tag} storage row: ${why}`)
|
||||
}
|
||||
|
||||
/** Validate the shared run-data fields and the payload/dt arity. */
|
||||
function validateRunData(tag: string, data: Record<string, unknown>, payloadKey: 'texts' | 'args'): void {
|
||||
/** Validate the shared run-data fields and the payload/dt arity; returns the member payload. */
|
||||
function validateRunData(tag: string, data: Record<string, unknown>, payloadKey: 'texts' | 'args'): string[] {
|
||||
if (typeof data.turn !== 'number' || typeof data.step !== 'number' || typeof data.index !== 'number') {
|
||||
malformed(tag, 'turn/step/index must be numbers')
|
||||
}
|
||||
@@ -229,12 +235,13 @@ function validateRunData(tag: string, data: Record<string, unknown>, payloadKey:
|
||||
malformed(tag, `${payloadKey} must be a non-empty string array`)
|
||||
}
|
||||
const dt = data.dt
|
||||
if (!Array.isArray(dt) || dt.some(gap => typeof gap !== 'number' || !Number.isFinite(gap))) {
|
||||
malformed(tag, 'dt must be an array of finite numbers')
|
||||
if (!Array.isArray(dt) || dt.some(gap => !Number.isSafeInteger(gap))) {
|
||||
malformed(tag, 'dt must be an array of safe integers')
|
||||
}
|
||||
if (dt.length !== payload.length - 1) {
|
||||
malformed(tag, `dt length ${dt.length} does not match ${payload.length} members`)
|
||||
}
|
||||
return payload as string[]
|
||||
}
|
||||
|
||||
/** Validate a row-tagged parsed value's envelope and data, throwing on any malformation. */
|
||||
@@ -245,11 +252,12 @@ function validateRow(value: Record<string, unknown>, tag: ChunkRow['type']): Chu
|
||||
if (!Number.isSafeInteger(value.seq0) || (value.seq0 as number) < 0) {
|
||||
malformed(tag, 'seq0 must be a non-negative safe integer')
|
||||
}
|
||||
if (typeof value.time0 !== 'number' || !Number.isFinite(value.time0)) {
|
||||
malformed(tag, 'time0 must be a finite number')
|
||||
if (!Number.isSafeInteger(value.time0)) {
|
||||
malformed(tag, 'time0 must be a safe integer')
|
||||
}
|
||||
const data = value.data
|
||||
if (!isRecord(data)) malformed(tag, 'data must be an object')
|
||||
let payload: string[]
|
||||
if (tag === 'tool-call-chunks') {
|
||||
const withName = hasExactKeys(data, ['turn', 'step', 'index', 'id', 'name', 'dt', 'args'])
|
||||
if (!withName && !hasExactKeys(data, ['turn', 'step', 'index', 'id', 'dt', 'args'])) {
|
||||
@@ -258,12 +266,25 @@ function validateRow(value: Record<string, unknown>, tag: ChunkRow['type']): Chu
|
||||
if (typeof data.id !== 'string' || (withName && typeof data.name !== 'string')) {
|
||||
malformed(tag, 'id (and name when present) must be strings')
|
||||
}
|
||||
validateRunData(tag, data, 'args')
|
||||
payload = validateRunData(tag, data, 'args')
|
||||
} else {
|
||||
if (!hasExactKeys(data, ['turn', 'step', 'index', 'dt', 'texts'])) {
|
||||
malformed(tag, 'data must be exactly {turn, step, index, dt, texts}')
|
||||
}
|
||||
validateRunData(tag, data, 'texts')
|
||||
payload = validateRunData(tag, data, 'texts')
|
||||
}
|
||||
// Reconstruction bounds. The encoder only packs runs whose member seqs and
|
||||
// times are all safe integers, so a running value that leaves safe range is
|
||||
// outside any encoder's image: float arithmetic would round it to a
|
||||
// different number than exact arithmetic, a silent corruption. Within safe
|
||||
// range every step is exact, so the first departure is always caught.
|
||||
if (!Number.isSafeInteger((value.seq0 as number) + payload.length - 1)) {
|
||||
malformed(tag, 'member seqs must stay safe integers')
|
||||
}
|
||||
let time = value.time0 as number
|
||||
for (const gap of data.dt as number[]) {
|
||||
time += gap
|
||||
if (!Number.isSafeInteger(time)) malformed(tag, 'member times must stay safe integers')
|
||||
}
|
||||
return value as unknown as ChunkRow
|
||||
}
|
||||
|
||||
@@ -106,6 +106,22 @@ describe('packChunkRuns', () => {
|
||||
expect(packChunkRuns(events)).toStrictEqual(events)
|
||||
})
|
||||
|
||||
it('breaks a run on a time gap beyond safe-integer range (subtraction would round)', () => {
|
||||
// Both endpoints are safe integers, but their true difference (~2^54)
|
||||
// exceeds exact double range: b - a rounds, so a + (b - a) !== b and a
|
||||
// packed row would decode to a different timestamp.
|
||||
const a = Number.MIN_SAFE_INTEGER
|
||||
const b = Number.MAX_SAFE_INTEGER - 1
|
||||
expect(a + (b - a)).not.toBe(b) // the rounding this guard exists for
|
||||
const events = [
|
||||
chunkEvent(0, a, { type: 'text-delta', index: 0, text: 'x' }),
|
||||
chunkEvent(1, b, { type: 'text-delta', index: 0, text: 'y' }),
|
||||
chunkEvent(2, b + 1, { type: 'text-delta', index: 0, text: 'z' }),
|
||||
]
|
||||
expect(packChunkRuns(events)).toStrictEqual(events) // split at the gap; halves too short
|
||||
expect(decodeAll(packChunkRuns(events))).toStrictEqual(events)
|
||||
})
|
||||
|
||||
it('stores a delta with an off-whitelist data envelope verbatim (parsed-fixture shapes)', () => {
|
||||
const mk = (seq: number, data: unknown): SessionEvent =>
|
||||
({ type: 'assistant/chunk', seq, time: 1000, data } as SessionEvent)
|
||||
@@ -144,11 +160,15 @@ describe('decodeStorageRecord', () => {
|
||||
['an envelope with extra keys', { type: 'text-chunks', seq0: 0, time0: 1, data: { turn: 1, step: 1, index: 0, dt: [], texts: ['a'] }, extra: 1 }],
|
||||
['a negative seq0', { type: 'text-chunks', seq0: -1, time0: 1, data: { turn: 1, step: 1, index: 0, dt: [], texts: ['a'] } }],
|
||||
['a non-finite time0', { type: 'text-chunks', seq0: 0, time0: Infinity, data: { turn: 1, step: 1, index: 0, dt: [], texts: ['a'] } }],
|
||||
['a fractional time0', { type: 'text-chunks', seq0: 0, time0: 1.5, data: { turn: 1, step: 1, index: 0, dt: [], texts: ['a'] } }],
|
||||
['a data shape mismatch', { type: 'text-chunks', seq0: 0, time0: 1, data: { turn: 1, step: 1, index: 0, dt: [], args: ['a'] } }],
|
||||
['a non-string member', { type: 'text-chunks', seq0: 0, time0: 1, data: { turn: 1, step: 1, index: 0, dt: [], texts: [7] } }],
|
||||
['an empty member list', { type: 'text-chunks', seq0: 0, time0: 1, data: { turn: 1, step: 1, index: 0, dt: [], texts: [] } }],
|
||||
['a dt arity mismatch', { type: 'text-chunks', seq0: 0, time0: 1, data: { turn: 1, step: 1, index: 0, dt: [1, 2], texts: ['a', 'b'] } }],
|
||||
['a non-finite dt gap', { type: 'text-chunks', seq0: 0, time0: 1, data: { turn: 1, step: 1, index: 0, dt: [NaN], texts: ['a', 'b'] } }],
|
||||
['a fractional dt gap', { type: 'text-chunks', seq0: 0, time0: 1, data: { turn: 1, step: 1, index: 0, dt: [0.5], texts: ['a', 'b'] } }],
|
||||
['a member seq leaving safe range', { type: 'text-chunks', seq0: Number.MAX_SAFE_INTEGER, time0: 1, data: { turn: 1, step: 1, index: 0, dt: [0, 0], texts: ['a', 'b', 'c'] } }],
|
||||
['a member time leaving safe range', { type: 'text-chunks', seq0: 0, time0: Number.MAX_SAFE_INTEGER, data: { turn: 1, step: 1, index: 0, dt: [1], texts: ['a', 'b'] } }],
|
||||
['a non-numeric turn', { type: 'text-chunks', seq0: 0, time0: 1, data: { turn: 'x', step: 1, index: 0, dt: [], texts: ['a'] } }],
|
||||
['a tool-call row without id', { type: 'tool-call-chunks', seq0: 0, time0: 1, data: { turn: 1, step: 1, index: 0, dt: [], args: ['a'] } }],
|
||||
['a tool-call row with non-string id', { type: 'tool-call-chunks', seq0: 0, time0: 1, data: { turn: 1, step: 1, index: 0, id: 7, dt: [], args: ['a'] } }],
|
||||
@@ -183,11 +203,19 @@ const boundaryChunkArb: fc.Arbitrary<StreamChunk> = fc.oneof(
|
||||
fc.record({ type: fc.constant<'finish'>('finish'), reason: fc.constant({ kind: 'stop' as const }) }),
|
||||
)
|
||||
|
||||
/** Batches with contiguous seqs, arbitrary gaps in time, mixed chunk kinds and turn/step placement. */
|
||||
/**
|
||||
* Batches with contiguous seqs, arbitrary timestamps, mixed chunk kinds and
|
||||
* turn/step placement. Times draw from the FULL safe-integer range (not just
|
||||
* realistic clocks) so the property exercises the gap-overflow guard: two safe
|
||||
* endpoints can differ by more than a double subtracts exactly.
|
||||
*/
|
||||
const batchArb: fc.Arbitrary<SessionEvent[]> = fc.array(
|
||||
fc.record({
|
||||
chunk: fc.oneof({ weight: 4, arbitrary: deltaChunkArb }, { weight: 1, arbitrary: boundaryChunkArb }),
|
||||
gap: fc.integer({ min: -5, max: 200 }),
|
||||
time: fc.oneof(
|
||||
{ weight: 4, arbitrary: fc.integer({ min: 995, max: 9000 }) },
|
||||
{ weight: 1, arbitrary: fc.integer({ min: Number.MIN_SAFE_INTEGER, max: Number.MAX_SAFE_INTEGER }) },
|
||||
),
|
||||
turn: fc.nat(1),
|
||||
step: fc.nat(1),
|
||||
}),
|
||||
@@ -196,7 +224,7 @@ const batchArb: fc.Arbitrary<SessionEvent[]> = fc.array(
|
||||
// plain objects real log events are (the log is JSON), so equality compares
|
||||
// values, not prototypes.
|
||||
).map(entries => JSON.parse(JSON.stringify(
|
||||
entries.map((entry, k) => chunkEvent(k, 1000 + entry.gap * k, entry.chunk, entry.turn, entry.step)),
|
||||
entries.map((entry, k) => chunkEvent(k, entry.time, entry.chunk, entry.turn, entry.step)),
|
||||
)) as SessionEvent[])
|
||||
|
||||
describe('chunk-row codec properties', () => {
|
||||
|
||||
Reference in New Issue
Block a user