fix(review): registry fail-loud, error-bypass parse, detail symmetry

- init: sentinel pre-check makes registerAll retry fail loud instead of
  piling partial state; honest no-rollback comment (throw is fatal).
- wire: isErrorEnvelope string path now parses data: JSON with shared
  object rules + anchored event check, so assistant text mentioning
  error literals no longer bypasses translation.
- chat-responses: image detail reuses normalizeImageDetail
  (original->high, symmetric with responses->chat); unnamed-custom
  first-wins collapse documented (Go parity, unreferenceable).
- docs: single-terminal usage assumption + thinking case-insensitivity
  notes (both zero live impact: collector coalesces; 'none' adds no
  prompt line).
- tests: 5 new cases (fail-loud, 12-pair, string FP, detail, Disabled).
- Verification: typecheck clean, 22 suites / 347 tests green.

Author: samson910022

src/converters/chat-messages/request.ts

@@ -74,6 +74,12 @@ export function resolveMessagesReasoningLevel(
  * Thinking -> reasoning_effort, extending anthropicThinkingToReasoningEffort
  * with adaptive/auto variants (Go claude_openai_request.go:57-98 core):
  * adaptive/auto + output_config.effort|effort passthrough, none/disabled -> none.
+ * NOTE vs legacy: type matching is case-insensitive here (legacy was
+ * exact-match). The only newly-accepted shape is mixed-case Disabled (which
+ * yields 'none', and 'none' adds no prompt line per buildSystemPrompt), plus
+ * mixed-case Adaptive/Auto (collapsed to the legacy fallback by
+ * resolveMessagesReasoningLevel on the live leg). Lowercase spec input —
+ * the only shape clients send — behaves exactly as legacy.
  */
 function messagesThinkingToReasoningEffort(thinking: unknown): string | null {
     if (!thinking || typeof thinking !== 'object' || Array.isArray(thinking)) return null;

src/converters/chat-messages/response.ts

@@ -117,6 +117,9 @@ export function convertMessagesResponseToChatNonStream(
  * Stateful chat-chunk -> Anthropic-SSE translator (one instance per stream).
  * Emits message_start / content_block_start(content text=0, tool_use=1..) /
  * content_block_delta / content_block_stop / message_delta / message_stop.
+ * Single-terminal assumption: usage is read from the finish_reason chunk
+ * only; a trailing usage-only chunk after completion is ignored (the gateway
+ * collector coalesces finish + usage into one terminal chunk).
  * TODO(P4): thinking-delta events + signature passthrough + multi-choice.
  */
 export function createChatToMessagesStreamTranslator(model: string, messageId?: string) {

src/converters/chat-responses/request.ts

@@ -85,7 +85,10 @@ function responsesToolToChatTool(tool: unknown): Record<string, unknown> | null
         // `custom` may be a name string or a descriptor object. Unlike
         // function tools (dropped when unnamed — validators reject empty
         // names), custom tools always fall back to 'custom_tool' because the
-        // shape itself implies a callable.
+        // shape itself implies a callable. NOTE: several unnamed customs
+        // therefore share that one name and collapse via first-wins dedup
+        // in pushTool — accepted: an unnamed declaration cannot be referenced
+        // by any call, so nothing routable is lost.
         const custom = t['custom'];
         const customName = typeof custom === 'object' && custom !== null ? str(asRecord(custom)['name']) : str(custom);
         const name = str(t['name']) || customName || 'custom_tool';
@@ -431,11 +434,13 @@ export function convertChatRequestToResponses(model: string, body: unknown, stre
                 const part = asRecord(p);
                 if (str(part['type']) === 'image_url') {
                     const iu = asRecord(part['image_url']);
-                    // Preserve detail (Go request.go:207-209); responses side
-                    // carries it on the input_image item itself.
+                    // Preserve detail via the shared normalizer (Go
+                    // request.go:207-209); responses side carries it on the
+                    // input_image item itself. original->high, like the
+                    // responses->chat direction.
                     const imageItem: Record<string, unknown> = { type: 'input_image', image_url: str(iu['url']) };
-                    const detail = str(iu['detail']).toLowerCase().trim();
-                    if (detail === 'auto' || detail === 'low' || detail === 'high') imageItem['detail'] = detail;
+                    const detail = normalizeImageDetail(iu['detail']);
+                    if (detail) imageItem['detail'] = detail;
                     parts.push(imageItem);
                 } else {
                     parts.push({ type: 'input_text', text: str(part['text']) });

src/converters/chat-responses/response.ts

@@ -154,6 +154,9 @@ export interface ResponsesStreamEvent {
  * per-tool-call output_item.added/args-delta/done, message text delta, and
  * response.completed deferred until the terminal chunk carrying finish_reason
  * (and late usage when present).
+ * Single-terminal assumption: usage is read from the finish_reason chunk
+ * only; a trailing usage-only chunk after completion is ignored (the gateway
+ * collector coalesces finish + usage into one terminal chunk).
  */
 export function createChatToResponsesStreamTranslator(model: string, responseId?: string) {
     const id = responseId || newResponseId();

src/converters/init.ts

@@ -15,6 +15,7 @@
 
 import type { TranslatorRegistry } from './registry.js';
 import { defaultTranslatorRegistry } from './registry.js';
+import { FormatOpenAI, FormatOpenAIResponse } from './formats.js';
 import { registerChatResponsesPair } from './chat-responses/init.js';
 import { registerChatMessagesPair } from './chat-messages/init.js';
 import { registerResponsesMessagesPair } from './responses-messages/init.js';
@@ -25,10 +26,20 @@ const initializedRegistries = new WeakSet<TranslatorRegistry>();
 
 export function registerAllTranslatorPairs(registry: TranslatorRegistry = defaultTranslatorRegistry()): void {
     if (initializedRegistries.has(registry)) return;
-    // NOTE: mark initialized only AFTER all four succeed — a throwing
-    // register must not poison the cache into a half-wired (6/12) no-op.
+    // Fail fast (before mutating) when a retry lands on an already-wired
+    // registry: per-pair register() throws on duplicates midway, which would
+    // otherwise add more partial state on top of a previous partial failure.
+    // NOTE: the WeakSet mark below happens only AFTER all four succeed, so a
+    // throwing register never poisons the cache into a half-wired no-op — but
+    // the registry map itself may still hold that run's predecessors (each
+    // single register() call is atomic, the batch has no rollback). Callers
+    // must treat a throw as fatal (boot fails loud) and must not keep serving
+    // from the half-wired registry.
     // Per-pair register* on a shared registry is not supported; always use
     // registerAll (fresh registries in tests may use per-pair directly).
+    if (registry.hasRequestTransformer(FormatOpenAIResponse, FormatOpenAI)) {
+        throw new Error('[translators] duplicate request transformer openai-response->openai');
+    }
     registerChatResponsesPair(registry);
     registerChatMessagesPair(registry);
     registerResponsesMessagesPair(registry);

src/converters/wire.ts

@@ -41,10 +41,42 @@ export function ensureTranslatorsRegistered(registry: TranslatorRegistry = defau
  */
 export function isErrorEnvelope(payload: unknown): boolean {
     if (typeof payload === 'string') {
-        return payload.includes('event: error') || payload.includes('"type":"error"') || payload.includes('"type": "error"') || payload.includes('response.failed');
+        // SSE `event:` fields are line-anchored: only a real error event
+        // matches (assistant text merely mentioning errors must not bypass).
+        if (/^event:\s*error(\s|$)/m.test(payload)) return true;
+        // Inspect JSON data payloads with the same object rules below, so a
+        // valid delta whose text contains error literals still translates.
+        const tryRecord = (text: string): boolean => {
+            try {
+                const parsed: unknown = JSON.parse(text);
+                return !!parsed && typeof parsed === 'object' && !Array.isArray(parsed) && isErrorRecord(asRecord(parsed));
+            } catch {
+                return false;
+            }
+        };
+        for (const line of payload.split('\n')) {
+            const trimmed = line.trim();
+            if (!trimmed.startsWith('data:')) continue;
+            const jsonText = trimmed.slice(5).trim();
+            if (!jsonText || jsonText === '[DONE]') continue;
+            if (tryRecord(jsonText)) return true;
+        }
+        // Bare JSON string without an SSE envelope.
+        const trimmed = payload.trim();
+        if (trimmed.startsWith('{') && tryRecord(trimmed)) return true;
+        return false;
     }
     if (!payload || typeof payload !== 'object' || Array.isArray(payload)) return false;
-    const r = asRecord(payload);
+    return isErrorRecord(asRecord(payload));
+}
+
+/**
+ * Object-shape error check shared by the object and string (parsed SSE)
+ * paths: wrapped {error}, error/response.failed type or event, and the bare
+ * TransformedUpstreamErrorBody ({message, type} with none of the
+ * valid-payload markers choices/output/content/data).
+ */
+function isErrorRecord(r: Record<string, unknown>): boolean {
     // Wrapped envelopes: {error: {...}} / {error: 'msg'}. Falsy sentinels
     // (''/0/false) are not error envelopes.
     const wrapped = r['error'];

tests/translator-phase-fixes.test.js

@@ -1,5 +1,8 @@
 import { TranslatorRegistry } from '../src/converters/registry.js';
 import { TranslatorPipeline } from '../src/converters/pipeline.js';
+import { registerAllTranslatorPairs } from '../src/converters/init.js';
+import { FormatOpenAI, FormatOpenAIResponse } from '../src/converters/formats.js';
+import { isErrorEnvelope } from '../src/converters/wire.js';
 import { makeId, normalizeArgs, num, asArray, str } from '../src/converters/json.js';
 import {
     usageToChat,
@@ -11,7 +14,8 @@ import {
     convertChatResponseToResponsesNonStream,
     convertResponsesResponseToChatNonStream,
 } from '../src/converters/chat-responses/response.js';
-import { convertChatRequestToMessages } from '../src/converters/chat-messages/request.js';
+import { convertChatRequestToMessages, convertMessagesRequestToChat } from '../src/converters/chat-messages/request.js';
+import { convertChatRequestToResponses } from '../src/converters/chat-responses/request.js';
 import {
     convertMessagesRequestToResponses,
     convertResponsesRequestToMessages,
@@ -166,3 +170,49 @@ describe('Phase4 text-core downgrade pinned (P0)', () => {
         expect(deltas.some((e) => e.type === 'response.output_text.delta')).toBe(true);
     });
 });
+
+describe('PR review follow-ups', () => {
+    test('registerAll retry on a wired registry fails loud (no silent half-wire)', () => {
+        const r = new TranslatorRegistry();
+        r.register(FormatOpenAIResponse, FormatOpenAI, (model, body) => ({ ...body, model }), {
+            nonStream: (m, _o, _t, b) => b,
+        });
+        expect(() => registerAllTranslatorPairs(r)).toThrow(/duplicate/);
+    });
+
+    test('registerAll on a fresh registry wires all 12 pairs', () => {
+        const r = new TranslatorRegistry();
+        expect(() => registerAllTranslatorPairs(r)).not.toThrow();
+        expect(r.size()).toEqual({ requests: 12, responses: 12 });
+    });
+
+    test('isErrorEnvelope string path: error markers match, assistant text does not', () => {
+        expect(isErrorEnvelope('event: error\ndata: {"type":"error"}')).toBe(true);
+        expect(isErrorEnvelope('data: {"type":"response.failed"}')).toBe(true);
+        expect(isErrorEnvelope('{"type":"error","error":{"message":"x"}}')).toBe(true);
+        expect(isErrorEnvelope('data: {"type":"response.output_text.delta"}')).toBe(false);
+        // Valid delta whose text mentions error literals must still translate.
+        expect(isErrorEnvelope('data: {"choices":[{"delta":{"content":"she explained \\"type\\":\\"error\\" handling"}}]}')).toBe(false);
+        expect(isErrorEnvelope('data: [DONE]')).toBe(false);
+    });
+
+    test('chat->responses image detail original normalizes to high (symmetric)', () => {
+        const out = convertChatRequestToResponses('m', {
+            messages: [{ role: 'user', content: [{ type: 'image_url', image_url: { url: 'data:x', detail: 'original' } }] }],
+        }, false);
+        expect(out.input[0].content).toEqual([{ type: 'input_image', image_url: 'data:x', detail: 'high' }]);
+        const missing = convertChatRequestToResponses('m', {
+            messages: [{ role: 'user', content: [{ type: 'image_url', image_url: { url: 'data:x' } }] }],
+        }, false);
+        expect(missing.input[0].content).toEqual([{ type: 'input_image', image_url: 'data:x' }]);
+    });
+
+    test('thinking type matching is case-insensitive (Disabled -> none)', () => {
+        const out = convertMessagesRequestToChat('m', {
+            messages: [{ role: 'user', content: [{ type: 'text', text: 'hi' }] }],
+            thinking: { type: 'Disabled' },
+        }, false);
+        // 'none' adds no prompt line downstream; legacy exact-match gave null.
+        expect(out.reasoning_effort).toBe('none');
+    });
+});