mirror of
https://github.com/deepseek-ai/deepseek-harness
synced 2026-08-15 21:04:50 +00:00
Merge pull request #295 from deepseek-harness/codex/simp-prune-llm-contract
refactor: prune unused LLM contract fields
This commit is contained in:
@@ -144,7 +144,7 @@ stream(options: GenerateOptions): AsyncIterable<StreamChunk>
|
||||
|
||||
Types: [GenerateOptions](../core-data-structures/core.md) · [StreamChunk](../core-data-structures/llm-streaming.md)
|
||||
|
||||
Source: [`packages/llm/llm/src/index.ts:96`](../../packages/llm/llm/src/index.ts)
|
||||
Source: [`packages/llm/llm/src/index.ts:94`](../../packages/llm/llm/src/index.ts)
|
||||
|
||||
## `ctx.permission` — `PermissionService`
|
||||
|
||||
|
||||
@@ -20,7 +20,7 @@ Adopt `fast-check` (a root devDependency) with one `tests/properties.spec.ts` pe
|
||||
## Consequences
|
||||
|
||||
- Generator quality is the value lever — the generators bias toward small index pools and short strings so collisions and interleavings are common.
|
||||
- **It already paid off:** the BlockAssembler stream found a real bug — a duplicate `block-end` at the same index overwrote an already-flushed block, so the streamed prefix disagreed with final `blocks()`. Fixed (first close wins, matching the existing straggler rule) with a dedicated regression test.
|
||||
- **It already paid off:** the BlockAssembler stream found a real bug — a duplicate `block-end` at the same index rewrote a completed block. Fixed (first close wins, matching the existing straggler rule) with a dedicated regression test.
|
||||
- A property flake from a timeout is a finding, not something to retry away. The loop properties are deterministic by construction (settle on `agent/status`), so a hang is a real defect.
|
||||
- Property tests supplement, not replace, the example tests that pin specific branches for the 100%-coverage gate.
|
||||
|
||||
|
||||
@@ -28,7 +28,7 @@ Each scenario's `session.jsonl` is harvested from a real run. `assistant/chunk`
|
||||
|
||||
```
|
||||
{ kind: 'chunks', chunks: StreamChunk[] }
|
||||
| { kind: 'throw', chunks: StreamChunk[], message: string, code: string, status?: number }
|
||||
| { kind: 'throw', chunks: StreamChunk[], message: string, code: string }
|
||||
| { kind: 'hang' }
|
||||
```
|
||||
|
||||
|
||||
@@ -6,7 +6,7 @@ Status: implemented
|
||||
|
||||
Model-driving ACP snapshot scenarios ship both `session.jsonl` and `session.golden.jsonl`. For normal recorded scenarios, `session.jsonl` is the replay fixture harvested from a real run, and the replay test normalizes the newly persisted log and compares it to `session.golden.jsonl`. In the current fixtures, the normalized recorded log and normalized golden are identical for the ordinary recorded scenarios.
|
||||
|
||||
Authored override scenarios (`error-finish`, `cancel`) currently use `replay.override.json` to drive model behavior and keep `session.jsonl` as a minimal dummy fixture, while `session.golden.jsonl` holds the expected persisted log. The override file is a JSON array of `ReplayEntry` objects: `{ "kind": "chunks", "chunks": StreamChunk[] }`, `{ "kind": "throw", "chunks": StreamChunk[], "message": string, "code": string, "status"?: number }`, or `{ "kind": "hang" }`. That split is also unnecessary: when an override sidecar exists, `llm-replay` replaces the derived script and does not need `session.jsonl` for model chunks, so `session.jsonl` can still be the expected session-log artifact for the scenario.
|
||||
Authored override scenarios (`error-finish`, `cancel`) currently use `replay.override.json` to drive model behavior and keep `session.jsonl` as a minimal dummy fixture, while `session.golden.jsonl` holds the expected persisted log. The override file is a JSON array of `ReplayEntry` objects: `{ "kind": "chunks", "chunks": StreamChunk[] }`, `{ "kind": "throw", "chunks": StreamChunk[], "message": string, "code": string }`, or `{ "kind": "hang" }`. That split is also unnecessary: when an override sidecar exists, `llm-replay` replaces the derived script and does not need `session.jsonl` for model chunks, so `session.jsonl` can still be the expected session-log artifact for the scenario.
|
||||
|
||||
## Decision
|
||||
|
||||
|
||||
@@ -1,3 +1,3 @@
|
||||
[
|
||||
{ "kind": "throw", "chunks": [], "message": "simulated provider error (HTTP 401)", "code": "AUTH", "status": 401 }
|
||||
{ "kind": "throw", "chunks": [], "message": "simulated provider error (HTTP 401)", "code": "AUTH" }
|
||||
]
|
||||
|
||||
@@ -98,10 +98,10 @@ export class DeepSeekAdapter extends LlmAdapter {
|
||||
const parsed = await response.json() as WireError
|
||||
if (parsed.error?.message) message = parsed.error.message
|
||||
} catch {
|
||||
// Only swallow error-body parsing: status and code are already captured,
|
||||
// so malformed gateway JSON must not mask the actionable HTTP failure.
|
||||
// Only swallow error-body parsing: the stable code and status-line message
|
||||
// are already captured, so malformed gateway JSON must not mask the failure.
|
||||
}
|
||||
throw new LlmError(message, code, response.status)
|
||||
throw new LlmError(message, code)
|
||||
}
|
||||
if (!response.body) {
|
||||
throw new LlmError('DeepSeek API returned no response body', 'EMPTY_RESPONSE')
|
||||
|
||||
@@ -159,7 +159,7 @@ describe('DeepSeekAdapter against a mock server', () => {
|
||||
status,
|
||||
body: JSON.stringify({ error: { message: `failed with ${status}`, type: 't', code: 'c' } }),
|
||||
}
|
||||
const server = await mockServer([behavior, behavior, behavior])
|
||||
const server = await mockServer([behavior, behavior])
|
||||
const ctx = await harness(server.url)
|
||||
await expect(assemble(ctx,{ model: 'deepseek-v4-flash', messages: [] }))
|
||||
.rejects.toThrow(`failed with ${status}`)
|
||||
@@ -167,11 +167,6 @@ describe('DeepSeekAdapter against a mock server', () => {
|
||||
assemble(ctx,{ model: 'deepseek-v4-flash', messages: [] })
|
||||
.catch((error: unknown) => (error as LlmError).code),
|
||||
).resolves.toBe(code)
|
||||
// The numeric HTTP status is carried on the error for explicit handling.
|
||||
await expect(
|
||||
assemble(ctx,{ model: 'deepseek-v4-flash', messages: [] })
|
||||
.catch((error: unknown) => (error as LlmError).status),
|
||||
).resolves.toBe(status)
|
||||
})
|
||||
|
||||
it('keeps the status-line message for JSON error bodies without a message', async () => {
|
||||
|
||||
@@ -76,4 +76,4 @@ Unit tests use pi-ai catalog models redirected to local mock servers and cover p
|
||||
- **Catalog membership is required** — custom model ids that are absent from the installed pi-ai catalog fail with `UNKNOWN_MODEL`, even when a provider profile supplies a custom endpoint.
|
||||
- **`GenerateOptions.stop` is unsupported** — pi-ai's common stream options cannot guarantee stop-sequence behavior across providers, so the adapter rejects the field.
|
||||
- **In-history `system` messages use pi-ai's common context conversion** — provider-specific placement follows pi-ai rather than a harness-owned wire override.
|
||||
- **`LlmError.status` is unavailable for in-stream failures** — pi-ai error events do not expose a stable HTTP status across providers.
|
||||
- **Provider HTTP status is unavailable** — pi-ai error events do not expose a stable HTTP status across providers; failures expose only stable harness error codes.
|
||||
|
||||
@@ -45,7 +45,7 @@ Every product adapter sends application identity on provider HTTP requests. `att
|
||||
- `LlmAdapter` — abstract base class for provider adapters. The only required method is `stream()`.
|
||||
- `BlockAssembler` — incrementally assembles raw chunks into complete content blocks and an assistant message. The agent loop feeds it raw chunks (logging them for replay) while reading the assembled blocks/message for history.
|
||||
- `HarnessError` — base class for the harness error taxonomy: a stable `code` string (distinct from the human `message`) plus `cause` chaining. Lives here, in the leaf package every other imports, so a single base is shared without a new dependency edge. Per-package errors (`LlmError`, `ToolArgsError`, `InvariantError`, …) extend it. `isHarnessError(value)` narrows at seams.
|
||||
- `LlmError` — extends `HarnessError`; `code` string (`NO_ADAPTER`, `DUPLICATE_ADAPTER`, and adapter codes like `AUTH`/`RATE_LIMIT`) plus an optional numeric `status` when the failure came from a non-2xx provider response.
|
||||
- `LlmError` — extends `HarnessError`; its stable `code` string (`NO_ADAPTER`, `DUPLICATE_ADAPTER`, and adapter codes like `AUTH`/`RATE_LIMIT`) is the programmatic failure contract.
|
||||
|
||||
### Real adapters
|
||||
|
||||
|
||||
@@ -39,12 +39,10 @@ export class BlockAssembler {
|
||||
private _replayState: unknown = undefined
|
||||
|
||||
/**
|
||||
* Feed one chunk. Returns the completed block when the chunk closes one
|
||||
* (an explicit `block-end`), otherwise undefined.
|
||||
* Feed one chunk into the assembly state.
|
||||
* @param chunk - the next raw chunk, in stream order.
|
||||
* @returns the authoritative block from the first `block-end` at its index; undefined for every other chunk.
|
||||
*/
|
||||
push(chunk: StreamChunk): ContentBlock | undefined {
|
||||
push(chunk: StreamChunk): void {
|
||||
switch (chunk.type) {
|
||||
case 'block-start': {
|
||||
if (!this.partials.has(chunk.index)) {
|
||||
@@ -78,7 +76,7 @@ export class BlockAssembler {
|
||||
// and the final assembled block in agreement.
|
||||
if (partial.block) return
|
||||
partial.block = chunk.block
|
||||
return chunk.block
|
||||
return
|
||||
}
|
||||
case 'usage': {
|
||||
this._usage = chunk.usage
|
||||
|
||||
@@ -43,12 +43,10 @@ declare module 'cordis' {
|
||||
|
||||
/**
|
||||
* Typed error for LLM-related failures. Extends {@link HarnessError}, so the
|
||||
* `code` string (e.g. `AUTH`, `RATE_LIMIT`, `NO_ADAPTER`) is shared taxonomy;
|
||||
* `status` carries the HTTP status when the error originated from a non-2xx
|
||||
* provider response (absent for protocol/usage errors that have no HTTP status).
|
||||
* `code` string (e.g. `AUTH`, `RATE_LIMIT`, `NO_ADAPTER`) is shared taxonomy.
|
||||
*/
|
||||
export class LlmError extends HarnessError {
|
||||
constructor(message: string, code: string, public status?: number, options?: ErrorOptions) {
|
||||
constructor(message: string, code: string, options?: ErrorOptions) {
|
||||
super(message, code, options)
|
||||
this.name = 'LlmError'
|
||||
}
|
||||
|
||||
@@ -29,12 +29,12 @@ describe('BlockAssembler', () => {
|
||||
expect(assembler.message().role).toBe('assistant')
|
||||
})
|
||||
|
||||
it('returns the completed block from push() on block-end', () => {
|
||||
it('records the completed block from block-end', () => {
|
||||
const assembler = new BlockAssembler()
|
||||
expect(assembler.push({ type: 'block-start', index: 0, blockType: 'text' })).toBeUndefined()
|
||||
expect(assembler.push({ type: 'text-delta', index: 0, text: 'hi' })).toBeUndefined()
|
||||
const block = assembler.push({ type: 'block-end', index: 0, block: { type: 'text', text: 'hi' } })
|
||||
expect(block).toEqual({ type: 'text', text: 'hi' })
|
||||
assembler.push({ type: 'block-start', index: 0, blockType: 'text' })
|
||||
assembler.push({ type: 'text-delta', index: 0, text: 'hi' })
|
||||
assembler.push({ type: 'block-end', index: 0, block: { type: 'text', text: 'hi' } })
|
||||
expect(assembler.blocks()).toEqual([{ type: 'text', text: 'hi' }])
|
||||
})
|
||||
|
||||
it('tolerates deltas without explicit block-start/end', () => {
|
||||
@@ -57,8 +57,8 @@ describe('BlockAssembler', () => {
|
||||
// push a delta first to guarantee the partial exists
|
||||
assembler.push({ type: 'text-delta', index: 0, text: 'hi' })
|
||||
// block-end's ensure() must find the existing partial (the second branch path)
|
||||
const block = assembler.push({ type: 'block-end', index: 0, block: { type: 'text', text: 'hi' } })
|
||||
expect(block).toEqual({ type: 'text', text: 'hi' })
|
||||
assembler.push({ type: 'block-end', index: 0, block: { type: 'text', text: 'hi' } })
|
||||
expect(assembler.blocks()).toEqual([{ type: 'text', text: 'hi' }])
|
||||
})
|
||||
|
||||
it('throws from assemble() when a partial has an unhandled blockType', () => {
|
||||
@@ -128,7 +128,7 @@ describe('assertNever', () => {
|
||||
|
||||
it('BlockAssembler.push rejects chunks outside the closed StreamChunk union', () => {
|
||||
const assembler = new BlockAssembler()
|
||||
expect(() => assembler.push({ type: 'rogue-chunk' } as unknown as StreamChunk))
|
||||
expect(() => { assembler.push({ type: 'rogue-chunk' } as unknown as StreamChunk) })
|
||||
.toThrow('unreachable variant in BlockAssembler.push')
|
||||
})
|
||||
})
|
||||
@@ -140,26 +140,8 @@ describe('BlockAssembler duplicate-close contract', () => {
|
||||
{ type: 'block-end', index: 0, block: { type: 'reasoning', text: 'first' } },
|
||||
{ type: 'block-end', index: 0, block: { type: 'text', text: 'second' } },
|
||||
]
|
||||
const streaming = new BlockAssembler()
|
||||
const closed = []
|
||||
for (const chunk of chunks) {
|
||||
const block = streaming.push(chunk)
|
||||
if (block) closed.push(block)
|
||||
}
|
||||
|
||||
const oneShot = new BlockAssembler()
|
||||
for (const chunk of chunks) oneShot.push(chunk)
|
||||
|
||||
expect(closed).toEqual([{ type: 'reasoning', text: 'first' }])
|
||||
expect(oneShot.blocks()).toEqual([{ type: 'reasoning', text: 'first' }])
|
||||
expect(closed).toEqual(oneShot.blocks())
|
||||
})
|
||||
|
||||
it('push returns undefined for a duplicate block-end (it closed nothing)', () => {
|
||||
const a = new BlockAssembler()
|
||||
expect(a.push({ type: 'block-end', index: 0, block: { type: 'text', text: 'x' } }))
|
||||
.toEqual({ type: 'text', text: 'x' })
|
||||
expect(a.push({ type: 'block-end', index: 0, block: { type: 'text', text: 'y' } }))
|
||||
.toBeUndefined()
|
||||
const assembler = new BlockAssembler()
|
||||
for (const chunk of chunks) assembler.push(chunk)
|
||||
expect(assembler.blocks()).toEqual([{ type: 'reasoning', text: 'first' }])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -255,11 +255,12 @@ describe('LlmService', () => {
|
||||
|
||||
it('LlmError extends the shared HarnessError base', async () => {
|
||||
const { HarnessError, isHarnessError } = await import('@deepseek-ai/dsh-llm')
|
||||
const err = new LlmError('boom', 'AUTH', 401)
|
||||
const cause = new Error('root cause')
|
||||
const err = new LlmError('boom', 'AUTH', { cause })
|
||||
expect(err).toBeInstanceOf(HarnessError)
|
||||
expect(isHarnessError(err)).toBe(true)
|
||||
expect(err.code).toBe('AUTH')
|
||||
expect(err.status).toBe(401)
|
||||
expect(err.cause).toBe(cause)
|
||||
})
|
||||
|
||||
it('HarnessError carries a code, names itself by subclass, and chains cause', async () => {
|
||||
|
||||
@@ -20,7 +20,7 @@ import { LlmAdapter, LlmError, assertNever } from '@deepseek-ai/dsh-llm'
|
||||
*/
|
||||
export type ReplayEntry =
|
||||
| { kind: 'chunks'; chunks: StreamChunk[] }
|
||||
| { kind: 'throw'; chunks: StreamChunk[]; message: string; code: string; status?: number }
|
||||
| { kind: 'throw'; chunks: StreamChunk[]; message: string; code: string }
|
||||
| { kind: 'hang' }
|
||||
|
||||
/** One model exposed by a replay-only provider catalog. */
|
||||
@@ -283,7 +283,7 @@ async function* replayEntry(entry: ReplayEntry, signal: AbortSignal | undefined)
|
||||
if (signal?.aborted) throw new Error('aborted')
|
||||
yield chunk
|
||||
}
|
||||
throw new LlmError(entry.message, entry.code, entry.status)
|
||||
throw new LlmError(entry.message, entry.code)
|
||||
case 'hang':
|
||||
// Replay a stream that stalls until cancelled (mirrors MockAdapter): one
|
||||
// chunk, then wait for abort and surface it as the consumer expects.
|
||||
|
||||
@@ -175,7 +175,7 @@ describe('loadReplayScript', () => {
|
||||
it('uses the sidecar override when present, ignoring the JSONL', () => {
|
||||
writeFileSync(file, sessionJsonl([]), 'utf8')
|
||||
const overrideFile = join(dir, 'replay.override.json')
|
||||
const override: ReplayEntry[] = [{ kind: 'throw', chunks: [], message: '401', code: 'AUTH', status: 401 }]
|
||||
const override: ReplayEntry[] = [{ kind: 'throw', chunks: [], message: '401', code: 'AUTH' }]
|
||||
writeFileSync(overrideFile, JSON.stringify(override), 'utf8')
|
||||
expect(loadReplayScript({ file, overrideFile })).toEqual(override)
|
||||
})
|
||||
@@ -265,12 +265,12 @@ describe('installLlmReplay (through the real LlmService)', () => {
|
||||
expect(await drain(ctx.llm.stream({ provider: 'm', model: 'm', messages: [] }))).toEqual(second)
|
||||
})
|
||||
|
||||
it('replays a sidecar throw-entry as an LlmError with code/status, after its prefix chunks', async () => {
|
||||
it('replays a sidecar throw-entry as an LlmError with its stable code, after its prefix chunks', async () => {
|
||||
writeFileSync(file, sessionJsonl([]), 'utf8')
|
||||
const overrideFile = join(dir, 'replay.override.json')
|
||||
const partial: StreamChunk[] = [{ type: 'block-start', index: 0, blockType: 'text' }]
|
||||
writeFileSync(overrideFile, JSON.stringify([
|
||||
{ kind: 'throw', chunks: partial, message: 'unauthorized', code: 'AUTH', status: 401 },
|
||||
{ kind: 'throw', chunks: partial, message: 'unauthorized', code: 'AUTH' },
|
||||
]), 'utf8')
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(LlmService)
|
||||
@@ -279,7 +279,7 @@ describe('installLlmReplay (through the real LlmService)', () => {
|
||||
const seen: StreamChunk[] = []
|
||||
await expect((async () => {
|
||||
for await (const c of ctx.llm.stream({ provider: 'm', model: 'm', messages: [] })) seen.push(c)
|
||||
})()).rejects.toMatchObject({ message: 'unauthorized', code: 'AUTH', status: 401 })
|
||||
})()).rejects.toMatchObject({ message: 'unauthorized', code: 'AUTH' })
|
||||
expect(seen).toEqual(partial)
|
||||
})
|
||||
|
||||
@@ -384,7 +384,7 @@ describe('installLlmReplay (through the real LlmService)', () => {
|
||||
const overrideFile = join(dir, 'replay.override.json')
|
||||
const partial: StreamChunk[] = [{ type: 'block-start', index: 0, blockType: 'text' }]
|
||||
writeFileSync(overrideFile, JSON.stringify([
|
||||
{ kind: 'throw', chunks: partial, message: 'unauthorized', code: 'AUTH', status: 401 },
|
||||
{ kind: 'throw', chunks: partial, message: 'unauthorized', code: 'AUTH' },
|
||||
]), 'utf8')
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(LlmService)
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
|
||||
The abstract `llm` service: an adapter registry plus a streaming model-call surface, interceptable via the `llm/stream` waterfall.
|
||||
|
||||
[Source](https://github.com/deepseek-harness/deepseek-harness/blob/master/packages/llm/llm/src/index.ts#L96)
|
||||
[Source](https://github.com/deepseek-harness/deepseek-harness/blob/master/packages/llm/llm/src/index.ts#L94)
|
||||
|
||||
### ctx.llm.registerAdapter(providers, adapter)
|
||||
|
||||
@@ -21,7 +21,7 @@ Register an adapter for the given provider routes. Throws `LlmError` with code `
|
||||
|
||||
**Returns** the disposer that unregisters all of them.
|
||||
|
||||
[Source](https://github.com/deepseek-harness/deepseek-harness/blob/master/packages/llm/llm/src/index.ts#L111)
|
||||
[Source](https://github.com/deepseek-harness/deepseek-harness/blob/master/packages/llm/llm/src/index.ts#L109)
|
||||
|
||||
### ctx.llm.listProviders()
|
||||
|
||||
@@ -33,7 +33,7 @@ Describe provider routes with a registered adapter.
|
||||
|
||||
**Returns** detached provider metadata in registration order.
|
||||
|
||||
[Source](https://github.com/deepseek-harness/deepseek-harness/blob/master/packages/llm/llm/src/index.ts#L142)
|
||||
[Source](https://github.com/deepseek-harness/deepseek-harness/blob/master/packages/llm/llm/src/index.ts#L140)
|
||||
|
||||
### ctx.llm.listModels(provider)
|
||||
|
||||
@@ -47,7 +47,7 @@ Discover models advertised by one registered provider. Catalog membership is adv
|
||||
|
||||
**Returns** detached model metadata in adapter-preferred order.
|
||||
|
||||
[Source](https://github.com/deepseek-harness/deepseek-harness/blob/master/packages/llm/llm/src/index.ts#L152)
|
||||
[Source](https://github.com/deepseek-harness/deepseek-harness/blob/master/packages/llm/llm/src/index.ts#L150)
|
||||
|
||||
### ctx.llm.stream(options)
|
||||
|
||||
@@ -61,4 +61,4 @@ Stream one model call as raw chunks (token-level deltas). Throws `LlmError` with
|
||||
|
||||
**Returns** the chunk stream, possibly wrapped by `llm/stream` listeners.
|
||||
|
||||
[Source](https://github.com/deepseek-harness/deepseek-harness/blob/master/packages/llm/llm/src/index.ts#L210)
|
||||
[Source](https://github.com/deepseek-harness/deepseek-harness/blob/master/packages/llm/llm/src/index.ts#L208)
|
||||
|
||||
Reference in New Issue
Block a user