fix(retry): degrade optional params on [1210] invalid input 400s (#190)

[Fahad090NP]

Fixes #190

Summary

ox-alpha-free rejects requests with HTTP 400:

Upstream request failed: [1210] Invalid API parameter, please check the documentation.invalid input

The gateway gives no hint which parameter is invalid, and the reporter's payload was 1.98 MB — well-formed otherwise. Nothing in our request builder is obviously wrong for this model (it's a fallback-family model, so no thinking fields are even sent), which means one of the optional parameters we always include isn't accepted by this upstream.

Fix

New patchInvalidInput classifier in src/retry.ts: when a 400 body matches [1210] … invalid input / invalid API parameter, the retry degrades the request by stripping optional parameters one per attempt, least-likely-to-matter first:

1. stream_options (usage accounting hint — pure nicety)
2. temperature (fall back to the model's own default)
3. image parts (some models reject data-URL images despite metadata claiming vision support; text parts are preserved)

Each patch only fires when the parameter is actually present, so once everything optional is gone the sequence stops and real failures surface normally — no infinite retries, and no masking of genuine errors after degradation is exhausted.

This mirrors how #171 (completion-cap) and #109 (context overflow) self-heal: use the engine's existing 400-retry to adapt to upstream constraints we can't know ahead of time.

Verification

• npm run lint all 7 checks green.
• 5 new unit tests in src/test/retry.test.ts: each degradation step, image-part stripping while preserving text, exhaustion → undefined, and no-fire for unrelated 400s.

Fi Amanillah

[ltmoerdani]

Nice one @Fahad090NP, the classifier and tests look clean. one thing before I merge though.

the ladder doesn't actually run end-to-end right now. analyzeHttp400ForRetry returns one patch per call, but src/transports/engine.ts only calls it once and retries once. so if dropping stream_options still gets a 400, we stop there. temperature and the image strip are never reached in a single request, the unit tests pass because they call the analyzer directly with pre-degraded bodies, not through the engine.

the JSDoc says "one per engine retry attempt", which is why I'm asking for the fix rather than a doc edit. could you wrap the analyze → patch → retry in engine.ts in a loop? something like:

let response = await fetchWithTransientRetry(payload);
let consumedErrorBody: string | undefined;
for (let patchAttempt = 0; patchAttempt < 3 && response.status === 400; patchAttempt++) {
  const errorDetail = consumedErrorBody ?? (await response.text());
  const parsedBody = JSON.parse(rawPayload) as Record<string, unknown>;
  const patch = analyzeHttp400ForRetry(errorDetail, parsedBody);
  if (!patch) break;
  options.output?.appendLine(`[retry] HTTP 400 recoverable: ${patch.reason}. Retrying with patched body…`);
  payload = JSON.stringify(patch.body);
  rawPayload = payload; // keep them in sync for the next iteration
  response = await fetchWithTransientRetry(payload);
  consumedErrorBody = response.status === 400 ? await response.text() : undefined;
}

(key detail: re-analyze has to run against the patched body, so rawPayload needs to be updated inside the loop too.)

a couple of notes while you're in there:

• the [http-error-body] log line from the current single-attempt version should stay, just move it inside the loop or before it.
• no need to add engine-level tests if it's a pain, the existing analyzer tests plus a compile pass are fine. if you do want one, a loop test with a mock fetch that 400s until images are stripped would be the one worth having.

happy to merge once the loop is in. thanks for picking this up.

[Fahad090NP]

Loop is in. engine.ts now wraps the analyze → patch → retry in a for loop up to MAX_400_PATCH_ATTEMPTS = 3:

• each iteration re-analyzes the latest 400 body against the latest patched payload — rawPayload is reassigned inside the loop as you flagged, so the next ladder step (temperature → images) is visible to the analyzer
• consumedErrorBody carries between iterations: the retry's 400 body is consumed once and reused for the next analysis / final error message, no double-read
• [http-error-body] log moved inside the loop
• exits when status ≠ 400 or the analyzer returns no patch

Also rebased onto current main (picked up #188's merge). Skipped the engine-level mock-fetch test per your note — the analyzer tests cover the ladder steps. Full gate green (all 7 checks). Fi Amanillah

[ltmoerdani]

Nice @Fahad090NP, the loop looks right, rawPayload sync, consumedErrorBody handling, break on no patch, all clean. CI green too.

merging now.