-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(combos): fail over provider-specific context caps #3461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1547,6 +1547,27 @@ describe("server combo failover 030 activation matrix", () => { | |
| expect(await exhausted.text()).not.toContain("sk-a-should-redact"); | ||
| }); | ||
|
|
||
| test("provider-specific prompt-too-long 400 hops to a larger-context combo target", async () => { | ||
| let backupHits = 0; | ||
| const capped = serve(() => Response.json({ error: { | ||
| message: "Prompt 346030 > 262144 maximum context length", | ||
| type: "invalid_request_prompt_too_long", | ||
| code: "5059", | ||
| raw_status_code: 400, | ||
| } }, { status: 400 })); | ||
|
Comment on lines
+1552
to
+1557
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Assert that the capped target was attempted. The test asserts one backup request, but it does not assert that the capped provider received a request. A routing regression that skips the capped target could still pass this test. Count requests to the capped server and assert exactly one capped request and one backup request. Suggested test change+ let cappedHits = 0;
- const capped = serve(() => Response.json({ error: {
+ const capped = serve(() => {
+ cappedHits += 1;
+ return Response.json({ error: {
message: "Prompt 346030 > 262144 maximum context length",
type: "invalid_request_prompt_too_long",
code: "5059",
raw_status_code: 400,
- } }, { status: 400 }));
+ } }, { status: 400 });
+ });
...
+ expect(cappedHits).toBe(1);
expect(backupHits).toBe(1);Also applies to: 1566-1567 🤖 Prompt for AI Agents |
||
| const backup = serve(() => { | ||
| backupHits += 1; | ||
| return chatSuccess("larger context backup", "m2"); | ||
| }); | ||
| const response = await post(comboConfig({ | ||
| a: provider("openai-chat", baseUrl(capped), "key-a"), | ||
| b: provider("openai-chat", baseUrl(backup), "key-b"), | ||
| })); | ||
| expect(response.status).toBe(200); | ||
| expect(backupHits).toBe(1); | ||
| expect(await response.text()).toContain("larger context backup"); | ||
| }); | ||
|
|
||
| test("429 Retry-After 120 keeps A cooling at 60 seconds and restores it at 120", async () => { | ||
| const t0 = Date.parse("2026-07-18T00:00:00.000Z"); | ||
| let now = t0; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Test the two overflow predicates independently.
The
"hop"case contains bothinvalid_request_prompt_too_longand code5059. It can pass even if the5059plusPrompt N > M maximum context lengthbranch is broken. Add one marker-only case and one5059plus message-shape case without the marker. Keep the existing generic5059case as the terminal case.Suggested test additions
📝 Committable suggestion
🤖 Prompt for AI Agents