mirror of
https://github.com/dtzp555-max/olp.git
synced 2026-07-21 21:15:10 +00:00
fix(ir): D12 — remove invented top-level error field on OpenAI response shapes
cold-audit catch from 2026-05-23
Cold-audit Finding 3 (P2 ALIGNMENT.md Rule 2(b) violation). The IR→OpenAI
translator was inventing a top-level `error: {message, type}` field on
chat.completion / chat.completion.chunk objects. OpenAI's spec defines
no such field — errors surface via HTTP 4xx/5xx with `{error: {...}}`
body, not as in-band SSE annotations. The pre-D12 code comments
acknowledged Rule 2(b) for `finish_reason` enum values then proceeded
to invent the top-level error field anyway.
Changes (3 files, +25 / -50 net):
1. lib/ir/ir-to-openai.mjs — both invention sites removed:
- irChunkToOpenAISSE: removed `if (irChunk.type === 'error')` branch
that emitted `{...spec fields, error: {message, type: 'provider_error'}}`
- irResponseToOpenAINonStream: removed `else if (chunk.type === 'error')`
branch + dead `errorChunk` local var + the `if (errorChunk)` block
that appended top-level `response.error = {...}`
- Added explanatory comments at both sites citing ALIGNMENT.md Rule 2(b)
+ OpenAI spec URLs so future readers see the rationale
2. server.mjs:581 — burst-replay loop break condition simplified:
- Pre: `if (irChunk.type === 'stop' || irChunk.type === 'error') break;`
- Post: `if (irChunk.type === 'stop') break;`
- Cache writes (server.mjs:456, :470) only append to streamedChunks
after the error-chunk guard fires, so cached chunks structurally
cannot contain type === 'error'. The removed clause was dead.
3. test-features.mjs — one test rewritten (test count unchanged 288 → 288):
- Old test asserted `payload.error.type === 'provider_error'` exists
(i.e., it tested the violation as if it were correct)
- New test asserts `payload.error === undefined` (the correct Rule 2(b)
invariant) and that object stays `chat.completion.chunk` with
finish_reason in the OpenAI enum
IR contract decision (verified by both implementer and reviewer):
- IR keeps `'error'` in IRResponseChunk typedef — providers legitimately
emit error chunks (e.g., anthropic.mjs)
- server.mjs intercepts error chunks BEFORE translation:
· collectAllChunks() throws ProviderError on type === 'error'
· Real-streaming branch throws ProviderError (no first chunk) or
truncates res.end() (after first chunk per ADR 0004 first-chunk rule)
- The translator is downstream of both guards; error chunks structurally
cannot reach it in normal operation
- Therefore the invention sites were dead-code paths handling an
impossible case — safe to remove
- Pass-through behavior on the impossible path: produces an empty SSE
delta `{choices: [{index: 0, delta: {}, finish_reason: null}]}`
(verified manually by reviewer); the error message text does NOT
leak to the wire
288/288 tests pass on Node 20.20.2.
Authority:
- ALIGNMENT.md Rule 2(b) — no invented OpenAI-spec fields
- OpenAI Chat Completions object spec
https://platform.openai.com/docs/api-reference/chat/object
https://platform.openai.com/docs/api-reference/chat/streaming
- ADR 0004 § Fallback safety — first-chunk rule (preserves the
truncation semantics for post-first-chunk errors)
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE. Verified error chunks never reach translator via
three independent grep passes (collectAllChunks guard, real-streaming
guard, no third caller exists), verified cache cannot contain error
chunks (streamedChunks.push is post-guard), verified Rule 2(b) compliance
by direct read of ALIGNMENT.md line 29 + OpenAI spec citations.
Three non-blocking suggestions (defensive log on impossible path;
defensive guard at server.mjs:587; comment placement nit) explicitly
"Not for D12" per the reviewer — preserved as separate cleanup
opportunities if ever needed.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
+10
-8
@@ -357,17 +357,19 @@ describe('irChunkToOpenAISSE format', () => {
|
||||
assert.deepEqual(payload.choices[0].delta, {});
|
||||
});
|
||||
|
||||
it('formats an error chunk with finish_reason within the OpenAI enum', () => {
|
||||
// ALIGNMENT.md Rule 2 (b): finish_reason must stay within the OpenAI spec
|
||||
// enum (stop|length|tool_calls|content_filter|function_call|null).
|
||||
// Provider errors surface via the top-level `error` object, not via an
|
||||
// invented finish_reason value.
|
||||
it('does not invent a top-level error field on error chunks (ALIGNMENT.md Rule 2(b))', () => {
|
||||
// ALIGNMENT.md Rule 2 (b): OLP must not introduce OpenAI-spec fields that
|
||||
// OpenAI's /v1/chat/completions specification does not document.
|
||||
// OpenAI chat.completion.chunk objects have no top-level `error` field.
|
||||
// Provider errors surface via HTTP 4xx/5xx, not as in-band SSE fields.
|
||||
// Error chunks should not reach the translator in normal operation —
|
||||
// server.mjs converts them to thrown ProviderError before translation.
|
||||
// If one somehow does reach here, no `error` field must be invented.
|
||||
const sse = irChunkToOpenAISSE({ type: 'error', error: 'spawn failed' }, ID, MODEL);
|
||||
const payload = JSON.parse(sse.slice(6).trim());
|
||||
assert.ok(payload.error);
|
||||
assert.equal(payload.error.type, 'provider_error');
|
||||
assert.equal(payload.error, undefined, 'translator must not invent top-level error field');
|
||||
assert.equal(payload.object, 'chat.completion.chunk');
|
||||
assert.ok(['stop', 'length', 'tool_calls', 'content_filter', 'function_call', null].includes(payload.choices[0].finish_reason));
|
||||
assert.equal(payload.choices[0].finish_reason, 'stop');
|
||||
});
|
||||
|
||||
it('SSE_DONE is the [DONE] terminator', () => {
|
||||
|
||||
Reference in New Issue
Block a user