Compare commits

..

2 Commits

Author SHA1 Message Date
33d7007af5 Merge pull request #128: learn sampling-param rejection
Some checks failed
CI/CD / typecheck (push) Successful in 1m20s
CI/CD / lint (push) Successful in 2m32s
CI/CD / test (push) Successful in 1m26s
CI/CD / smoke (push) Failing after 2m3s
CI/CD / build (push) Successful in 4m34s
CI/CD / publish (push) Has been skipped
2026-08-25 23:12:08 +00:00
bc7eb5a0ad fix(mcplocal): learn which models reject sampling params, don't hardcode them
Some checks failed
CI/CD / typecheck (pull_request) Successful in 1m22s
CI/CD / lint (pull_request) Successful in 2m34s
CI/CD / test (pull_request) Successful in 1m28s
CI/CD / build (pull_request) Successful in 2m27s
CI/CD / smoke (pull_request) Failing after 3m6s
CI/CD / publish (pull_request) Has been skipped
Auto-following to the newest Opus moved the failure rather than removing it.
The deploy's own smoke output showed it: 404 "model not found" became

  HTTP 400: `temperature` is deprecated for this model.

Anthropic removed temperature/top_p/top_k on the current generation (Opus 5,
Sonnet 5, Opus 4.7/4.8, Fable 5), and the adapter sends temperature: 0
unconditionally. So the gate's prompt-selection was still degrading on every
call -- just with a different status code.

A list of which models accept sampling would rot exactly the way the pinned
model ids did, which is the whole thing this branch is trying to stop. So the
provider learns it instead: the first 400 naming a sampling parameter drops it
and retries, and remembers the model so every later call omits it up front.
One wasted call, once, rather than a hardcoded table to maintain.

The match is deliberately narrow -- a 400 must actually name temperature/top_p/
top_k. An unrelated 400 (missing max_tokens, bad schema) propagates untouched;
there is a test for that, because a broad match here would silently swallow
real request errors and retry them pointlessly.

Verified live: claude-opus-latest -> claude-opus-5, first attempt 400 on
temperature, retry succeeds past it. The retry then hit HTTP 429 -- the
personal OAuth token's rate limit, which is the pre-existing credential-tiering
issue, not this path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GqMidYEGUJG5fxeoTELBu2
2026-08-26 00:11:50 +01:00
2 changed files with 101 additions and 3 deletions

View File

@@ -30,6 +30,15 @@ function familyOf(model: string): string | null {
return null;
}
/**
* A 400 naming a sampling parameter, e.g.
* "`temperature` is deprecated for this model."
*/
function isSamplingRejection(err: unknown): boolean {
const msg = err instanceof Error ? err.message : String(err);
return msg.includes('HTTP 400') && /`?(temperature|top_p|top_k)`?/.test(msg);
}
/** Resolutions are cached this long before the models endpoint is consulted again. */
const MODEL_CACHE_TTL_MS = Number(process.env['MCPCTL_ANTHROPIC_MODEL_TTL_MS']) || 12 * 60 * 60 * 1000;
@@ -40,6 +49,17 @@ export class AnthropicProvider implements LlmProvider {
readonly name = 'anthropic';
/** Shared across instances: the model list is account-wide, not per-provider. */
private static readonly modelCache = new Map<string, { id: string; expiresAt: number }>();
/**
* Models that rejected a sampling parameter, learned at runtime.
*
* Anthropic removed `temperature`/`top_p`/`top_k` on the current generation
* (Opus 5, Sonnet 5, Opus 4.7/4.8, Fable 5) — sending one is a hard 400. A
* hardcoded list of which models accept it would rot exactly the way the
* pinned model ids did, so learn it from the API instead: the first call
* retries without the parameter and remembers, and every later call omits it
* up front.
*/
private static readonly rejectsSampling = new Set<string>();
private apiKey: string;
private defaultModel: string;
@@ -66,7 +86,9 @@ export class AnthropicProvider implements LlmProvider {
if (systemMessages.length > 0) {
body.system = systemMessages.map((m) => m.content).join('\n');
}
if (options.temperature !== undefined) body.temperature = options.temperature;
if (options.temperature !== undefined && !AnthropicProvider.rejectsSampling.has(model)) {
body.temperature = options.temperature;
}
if (options.tools && options.tools.length > 0) {
body.tools = options.tools.map((t) => ({
@@ -76,8 +98,19 @@ export class AnthropicProvider implements LlmProvider {
}));
}
const response = await this.request(body, options.signal);
return parseAnthropicResponse(response);
try {
return parseAnthropicResponse(await this.request(body, options.signal));
} catch (err) {
// Learn-and-retry rather than fail: a model that has dropped sampling
// support should cost one wasted call, once, not every call forever.
if (body.temperature !== undefined && isSamplingRejection(err)) {
AnthropicProvider.rejectsSampling.add(model);
delete body.temperature;
process.stderr.write(`[anthropic] ${model} rejects sampling params — retrying without\n`);
return parseAnthropicResponse(await this.request(body, options.signal));
}
throw err;
}
}
/**

View File

@@ -81,3 +81,68 @@ describe('Anthropic model resolution', () => {
await expect(providerWith(MODELS).listModels()).resolves.toContain('claude-opus-5');
});
});
describe('sampling-parameter rejection', () => {
afterEach(() => {
(AnthropicProvider as unknown as { rejectsSampling: Set<string> }).rejectsSampling.clear();
});
/** Rejects `temperature` exactly as the current Anthropic models do. */
function samplingStrictProvider(): { provider: AnthropicProvider; bodies: Array<Record<string, unknown>> } {
const provider = new AnthropicProvider({ apiKey: 'sk-ant-api-test' });
const bodies: Array<Record<string, unknown>> = [];
(provider as unknown as { request: (b: unknown) => Promise<unknown> }).request = async (b) => {
const body = b as Record<string, unknown>;
bodies.push({ ...body });
if (body.temperature !== undefined) {
throw new Error(
'Anthropic HTTP 400: {"type":"error","error":{"type":"invalid_request_error",'
+ '"message":"`temperature` is deprecated for this model."}}',
);
}
return { content: [{ type: 'text', text: 'ok' }], stop_reason: 'end_turn' };
};
return { provider, bodies };
}
it('retries without temperature when the model rejects it', async () => {
vi.spyOn(process.stderr, 'write').mockImplementation(() => true);
const { provider, bodies } = samplingStrictProvider();
const result = await provider.complete({
model: 'claude-opus-5',
messages: [{ role: 'user', content: 'hi' }],
temperature: 0,
});
expect(result.content).toBe('ok');
expect(bodies).toHaveLength(2);
expect(bodies[0]!.temperature).toBe(0);
expect(bodies[1]!.temperature).toBeUndefined();
});
it('remembers, so the wasted call happens once and not forever', async () => {
vi.spyOn(process.stderr, 'write').mockImplementation(() => true);
const { provider, bodies } = samplingStrictProvider();
const req = { model: 'claude-opus-5', messages: [{ role: 'user' as const, content: 'hi' }], temperature: 0 };
await provider.complete(req);
await provider.complete(req);
await provider.complete(req);
// 2 for the first call (reject + retry), then 1 each — not 2 each.
expect(bodies).toHaveLength(4);
});
it('does not swallow unrelated 400s', async () => {
const provider = new AnthropicProvider({ apiKey: 'sk-ant-api-test' });
(provider as unknown as { request: () => Promise<unknown> }).request = async () => {
throw new Error('Anthropic HTTP 400: {"error":{"message":"max_tokens is required"}}');
};
await expect(provider.complete({
model: 'claude-opus-5',
messages: [{ role: 'user', content: 'hi' }],
temperature: 0,
})).rejects.toThrow(/max_tokens/);
});
});