fix: address codex review — drop false-positive fallback keywords, tighten stream-error detection

Two valid findings from an opposite-model (Codex) review of this branch:

- provider-request: drop bare `unknown`/`unrecognized` from the structured-output
  rejection keywords. They false-positive on model-name errors ("unknown model",
  "unrecognized model ID" from Ollama/Bedrock), triggering a wasted fallback
  retry. The specific feature tokens + bare `schema` still cover real structured-
  output rejections, so true-positive detection is unaffected.
- streaming: streamErrorMessage's OpenAI-style `{error:{...}}` branch now only
  treats the payload as an error when it carries a message/type, so a stray empty
  `error: {}` (or code-only object) in a normal chunk no longer aborts the stream.
  The Anthropic `{type:'error'}` branch still always signals an error.

Declined: Codex's suggestion to convert streaming's HTTP>=400 to ProviderApiError
+ add fallback — the streaming path always sends `structured:false`, so there is
no structured request to fall back from.

Tests updated/added: model-name 400s do not retry; empty/code-only error objects
do not throw.

Change-Id: I413d633c8ce7036365f97e2d8e1287c111e0902f

src/provider-request.ts

@@ -94,8 +94,12 @@ export async function requestJsonBody(
   return responseJson(resp, label, settings);
 }
 
+// Bare `unknown`/`unrecognized` were intentionally dropped: they false-positive on
+// model-name errors ("unknown model", "unrecognized model ID") and trigger a wasted
+// fallback retry. The specific feature tokens + bare `schema` cover real structured-
+// output rejections (e.g. "Unknown field: responseSchema" still matches `schema`).
 const STRUCTURED_OUTPUT_REJECTION_KEYWORDS =
-  /response_format|json_schema|responseJsonSchema|responseMimeType|tools?|tool_choice|unsupported|unrecognized|unknown|schema/i;
+  /response_format|json_schema|responseJsonSchema|responseMimeType|tools?|tool_choice|unsupported|schema/i;
 const STRUCTURED_OUTPUT_FALLBACK_STATUSES = new Set([400, 404, 422]);
 
 export function shouldRetryWithoutStructuredOutput(error: unknown): boolean {

src/streaming.ts

@@ -42,13 +42,14 @@ export function streamErrorMessage(json: Record<string, unknown>): string | null
     if (typeof err.type === 'string' && err.type) return err.type;
     return null;
   };
+  // Anthropic: an explicit error event is an error even if its details are sparse.
   if (json.type === 'error') {
     return messageFromError(json.error) ?? 'Provider returned a streaming error';
   }
-  if (json.error) {
-    return messageFromError(json.error) ?? 'Provider returned a streaming error';
-  }
-  return null;
+  // OpenAI-compatible: { error: { message, type, code } }. Only treat it as an error
+  // when it actually carries a message/type, so a stray empty `error: {}` (or a
+  // code-only object) in an otherwise-normal chunk does not abort the stream.
+  return messageFromError(json.error);
 }
 
 export type DeltaExtractor = (json: Record<string, unknown>) => string;

tests/direct-providers.test.js

@@ -118,6 +118,16 @@ const { assert, requireBundledModule, cleanup } = require('./direct-test-setup')
       shouldRetryWithoutStructuredOutput(new ProviderApiError('bad request', 400, 'missing required field: model')),
       false,
     );
+    // Model-name errors ("unknown model" / "unrecognized model ID") must NOT trigger a
+    // wasted fallback retry — bare unknown/unrecognized keywords were dropped for this.
+    assert.strictEqual(
+      shouldRetryWithoutStructuredOutput(new ProviderApiError('bad request', 400, 'unknown model: gpt-5')),
+      false,
+    );
+    assert.strictEqual(
+      shouldRetryWithoutStructuredOutput(new ProviderApiError('bad request', 400, 'unrecognized model ID: x')),
+      false,
+    );
     // Legacy path: a plain Error carrying the English template still works (back-compat).
     assert.strictEqual(
       shouldRetryWithoutStructuredOutput(new Error('OpenAI API returned HTTP 400: response_format not supported')),

tests/direct-streaming.test.js

@@ -85,10 +85,10 @@ const { assert, requireBundledModule, cleanup } = require('./direct-test-setup')
     );
     assert.strictEqual(streaming.streamErrorMessage({ type: 'error' }), 'Provider returned a streaming error');
     assert.strictEqual(streaming.streamErrorMessage({ error: { message: 'Quota exceeded' } }), 'Quota exceeded');
-    assert.strictEqual(
-      streaming.streamErrorMessage({ error: { code: 'insufficient_quota' } }),
-      'Provider returned a streaming error',
-    );
+    // OpenAI-style error object without a message/type is NOT treated as an error
+    // (avoids aborting a normal chunk that carries a stray empty/code-only `error`).
+    assert.strictEqual(streaming.streamErrorMessage({ error: { code: 'insufficient_quota' } }), null);
+    assert.strictEqual(streaming.streamErrorMessage({ error: {} }), null);
     assert.strictEqual(streaming.streamErrorMessage({ choices: [{ delta: { content: 'hi' } }] }), null);
     assert.strictEqual(streaming.streamErrorMessage({ type: 'content_block_delta', delta: { text: 'x' } }), null);
     assert.strictEqual(streaming.streamErrorMessage({ error: null }), null);