feat(libsy-llm-client): strip rejected extra_body keys on retry

[gburachas]

Problem

Per-target extra_body keys are merged into the outbound request body in
crates/libsy-llm-client/src/client.rs at a point that sits outside the retry loop, so the merge
happens once, before any attempt.

When an upstream rejects the request because of one of those keys — a 400/422 naming an unknown or
unexpected parameter — every retry re-sends the same rejected key. The retry cannot succeed, and
there is no way for a caller to say "drop this knob and try again".

The case we hit in practice: reasoning-control knobs (chat_template_kwargs, enable_thinking) that
some OpenAI-compatible servers accept and others reject outright. A call carrying the knob fails
permanently against a rejecting server, even though the identical call without it would succeed.

Proposal

Move the extra_body merge inside the retry loop and allow named keys to be dropped after a
parameter rejection. Roughly 20 lines.

Deliberately narrow:

• Only a 400/422 whose body names the parameter, or an unknown/extra-parameter condition,
  qualifies. A 429 or 5xx must surface as-is — silently stripping a knob and re-firing on a
  rate limit would double the load and mask the real failure.
• Retry exactly once without the knob. No escalation ladder, no second strip.

Scope

crates/libsy-llm-client/ only. Benefits every route that configures extra_body, which is why it
seems worth doing as its own change rather than inside a feature PR.

Notes

Happy to open the PR. Wanted to check the direction first, since it moves an existing statement
across the retry boundary rather than adding alongside it — if you'd prefer a different shape (e.g.
an explicit per-target "droppable keys" list instead of inferring from the reject), that's easy to
adjust.

[gburachas]

Following up on my own proposal: moving the merge inside the retry loop turned out to be unnecessary. There's a smaller change that gets the same behaviour without relocating anything.

merge_extra_body already only inserts where the request omits a key. If it returns the keys it actually injected, a parameter reject can strip exactly those from the already-encoded body and retry — no need to re-merge, and no existing statement moves.

Sketch:

// merge_extra_body now returns Vec<String> of the keys it inserted
let injected_extra_body = merge_extra_body(&mut body, backend.extra_body());
let mut stripped_extra_body = false;

// ...inside the existing retry loop, on failure:
if !stripped_extra_body && !injected_extra_body.is_empty() && failure.is_param_reject() {
    stripped_extra_body = true;
    remove_body_keys(&mut body, &injected_extra_body);
    continue;
}

Three properties that seem worth pinning down, whichever shape you prefer:

1. Only injected keys are stripped, never the caller's own. If the request already set enable_thinking, extra_body never injected it, so it isn't reported and isn't dropped.
2. The strip doesn't consume the retry budget — it fires even at max_retries = 0. A request minus an optional default is a different request, not a repeat of a failed one; the budget exists for transient faults.
3. At most once per call, so a server that rejects with or without the field surfaces its 400 rather than looping.

is_param_reject is narrow by construction: only a 400/422 whose body names a parameter. A 429 or 5xx never qualifies — stripping a field and re-firing on a rate limit would double the load and mask the real failure. Context overflow carries its own error variant, so it can't match either.

One open question for you: the detection is phrase-based (unknown parameter, unexpected keyword, extra_forbidden, plus chat_template_kwargs / enable_thinking explicitly). invalid parameter and unrecognized are broad enough to catch an unrelated 400 that happens to use those words. The cost of a false positive is bounded — one extra request without optional defaults — but if you'd rather have an explicit per-target "droppable keys" list instead of inferring from the body, that's a small change and arguably more predictable.

I have this written and tested (3 new tests; workspace green, clippy and fmt clean). Happy to open the PR whenever you've got a preference on the detection question.