Skip to content

fix: DDG circuit breaker (#6999) + null content validation (#7000) - #7001

Merged
diegosouzapw merged 10 commits into
cheaper-inference:release/v3.8.49from
rafaumeu:fix/6999-7000-ddg-circuit-breaker-null-content
Jul 18, 2026
Merged

diegosouzapw merged 10 commits into
cheaper-inference:release/v3.8.49from
rafaumeu:fix/6999-7000-ddg-circuit-breaker-null-content

Conversation

@rafaumeu

Copy link
Copy Markdown
Contributor

Summary

Two bug fixes for issues discovered during provider testing.

#6999 — Circuit breaker for DuckDuckGo executor

Problem: When DuckDuckGo's upstream is rate-limiting (429) or down (5xx), the executor keeps sending every new request to the upstream, wasting time on timeouts before the combo engine can fail over.

Fix: Lightweight in-memory circuit breaker:

  • After 5 consecutive failures (429, 5xx, or network errors), the breaker opens for 30 seconds
  • During the open window, every request fast-fails with 503 immediately
  • The combo engine picks up the 503 and failovers to the next provider instantly
  • After cooldown, the breaker half-opens naturally — the next request is a real probe
  • A single success resets the failure counter to 0

No external dependencies, no config changes. State is module-level (not per-instance) because the upstream availability is global, not per-session.

#7000 — Null/empty content false positive in validateResponseQuality

Problem: validateResponseQuality() in combo/validateQuality.ts checked content !== null && content !== undefined && content !== "" but:

  1. When content is an array (OpenAI multimodal format), an empty array [] passed as valid
  2. When content was a whitespace-only string (e.g. " "), it passed as valid
  3. Both cases resulted in false positives — empty responses were treated as successful

Fix:

  • Arrays now require at least 1 non-empty part (text with trimmed length > 0, or image/audio/file type)
  • Strings must have trimmed.length > 0
  • null/undefined/empty string still correctly rejected

Testing

  • Existing DDG search tests pass (tests/unit/search-handler-duckduckgo.test.ts — 2/2 ok)
  • TypeScript compilation passes (no new errors introduced)
  • All quality gates pass: prettier, eslint, t11:any-budget, tracked-artifacts

Closes #6999, closes #7000

rafaumeu added 3 commits July 12, 2026 15:57
…pencode-go

Previously only deepseek-v4-pro had effort-tier aliases on the opencode-go
provider. GLM-5.2 and MiMo-V2.5 only had base model ids, making it impossible
to pin reasoning effort per combo target.

Changes:
- Generalize parseDeepSeekEffortLevel → parseEffortLevel with EFFORT_TIERS table
- deepseek-v4-pro: low/medium/high/max (unchanged)
- glm-5.2: high/max only (OpenAI transport; low/medium not supported)
- mimo-v2.5: high/max only (same reasoning)
- Register alias model ids in opencode-go registry
- Mark base models supportsReasoning: true
- 9 unit tests covering registry + executor + backward compat

Closes cheaper-inference#6922
@rafaumeu
rafaumeu requested a review from diegosouzapw as a code owner July 12, 2026 23:14
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks for the fix — the circuit breaker and array-content validation logic both look correct and merge cleanly against the current release branch (verified with a 3-way merge simulation, the already-merged #6996/#6427 fixes are preserved).

Two things before this is mergeable:

  1. Your branch stacks on top of feat(6922): effort-tier aliases for glm-5.2 & mimo-v2.5 on opencode-go #6987 (I can see the feat(providers): register reasoning effort-tier aliases for glm-5.2 & mimo-v2.5 on opencode-go (per-target combo effort) #6922 effort-tier-aliases commit in the branch history) — could you rebase once feat(6922): effort-tier aliases for glm-5.2 & mimo-v2.5 on opencode-go #6987 lands, or call out the dependency explicitly? Right now the PR body only describes feat(resilience): Circuit breaker for no-auth providers — stop advertising dead models #6999/fix(providers): oc/deepseek-v4-flash-free returns HTTP 200 with null content — false positive success #7000 but the diff also carries feat(providers): register reasoning effort-tier aliases for glm-5.2 & mimo-v2.5 on opencode-go (per-target combo effort) #6922's changes.
  2. Neither feat(resilience): Circuit breaker for no-auth providers — stop advertising dead models #6999 (circuit breaker) nor fix(providers): oc/deepseek-v4-flash-free returns HTTP 200 with null content — false positive success #7000 (array/whitespace content check) has a dedicated test — the existing DDG tests and the one new test file (which is actually for feat(providers): register reasoning effort-tier aliases for glm-5.2 & mimo-v2.5 on opencode-go (per-target combo effort) #6922) don't exercise either new code path. Per our contribution guidelines, production fixes need a regression test that proves the bug before/after. Could you add one for each?

@diegosouzapw
diegosouzapw changed the base branch from main to release/v3.8.49 July 15, 2026 08:17
rafaumeu added 3 commits July 15, 2026 08:29
…evel function

- Export parseEffortLevel from opencode.ts so tests can import it
- Replace grep-on-source-file assertions with real function calls
- 13 tests: 4 deepseek tiers + 2 glm-5.2 tiers + 2 mimo-v2.5 tiers
  + 5 negative cases (unknown model, unsupported tiers, empty, base-only)
- Remove dependency on readFileSync / string matching
…dation (cheaper-inference#7000)

cheaper-inference#6999: Add lightweight circuit breaker to DuckDuckGo executor.
After 5 consecutive failures (429, 5xx, network errors), the breaker
opens for 30s — during that window every request fast-fails with 503
so the combo engine can immediately fail over to the next provider
instead of waiting for timeouts. Half-open probing happens naturally
once the cooldown expires. A single success resets the counter.

cheaper-inference#7000: Fix false positive in validateResponseQuality where multimodal
content arrays (empty []) and whitespace-only strings passed as valid.
Now properly validates: arrays must have >=1 non-empty part; strings
must have non-zero trimmed length.
…#6999) and null content validation (cheaper-inference#7000)

- Circuit breaker: verifies 400 for empty messages is unaffected by CB state,
  and that CB starts closed (no 503 on first request)
- Null content (cheaper-inference#7000): verifies validateResponseQuality correctly flags
  null content, empty array content [] as invalid, and array with text as valid
@rafaumeu
rafaumeu force-pushed the fix/6999-7000-ddg-circuit-breaker-null-content branch from c525d0e to eadf6bd Compare July 15, 2026 11:39
rafaumeu and others added 4 commits July 15, 2026 12:24
…ull-content

Clean merge, no conflicts (verified with git merge-tree before merging).

Stacking disclosure: this branch was cut on top of
feat/6922-opencode-go-effort-aliases-glm-mimo (cheaper-inference#6987) — its 4 commits
(b42531d..4a4d37b, the effort-tier-aliases feature + its own test
rewrite) are part of this branch's history and are NOT re-described here
since they belong to cheaper-inference#6987. cheaper-inference#6987 is a separate, still-open PR; once it
merges to release, the merge-train/squash will dedupe the shared commits
so this branch does not double-apply that diff. Until then, this PR's
declared diff stats include cheaper-inference#6987's changes even though its own commits
(the 3e0f28e/eadf6bdf2/a4a0c5641 range) only ever touched DDG circuit
breaker + null-content-validation files.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…machine directly

The existing "circuit breaker fast-fails with 503 after consecutive
failures" test never actually drives 5 consecutive failures — it makes a
single real network call and only asserts the response isn't 503, which
passes whether or not the breaker logic works at all (confirmed by
disabling the open-threshold check entirely: that test stayed green).

Exports cbIsOpen/cbRecordFailure/cbRecordSuccess/CB_THRESHOLD/
CB_COOLDOWN_MS (previously module-private) plus two test-only helpers
(__setDdgCircuitBreakerStateForTests/__getDdgCircuitBreakerStateForTests,
following the __xxxForTests convention already used in
src/shared/utils/circuitBreaker.ts) so tests can drive the module-level
singleton directly instead of needing a full network mock through
warmSession/seedChallengeChain/acquireAuthHeaders, and without waiting
CB_COOLDOWN_MS=30s in real time for the half-open case.

New tests cover: starts closed; opens on the CB_THRESHOLD-th consecutive
failure (not before); execute() fast-fails with 503 while open without
reaching the network (verified: disabling the cbIsOpen() gate makes the
same test fall through to a real network call, ~1s slower and red);
still open just before cooldown elapses; self-closes once cooldown has
elapsed (half-open); cbRecordSuccess resets the counter.

Red-first proof (both independently green->red->restored-green):
1. `if (false && failures >= CB_THRESHOLD ...)` — neuters the open
   transition. Result: the new "opens after CB_THRESHOLD..." test fails;
   the pre-existing weak test stays green regardless.
2. `if (false && cbIsOpen())` — neuters the execute() gate. Result: the
   new "execute() fast-fails with 503 while open" test fails (and takes
   ~1s longer, falling through to a real network attempt instead of
   short-circuiting).

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…opy (landed via its own PR)

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw
diegosouzapw merged commit a5e5e88 into cheaper-inference:release/v3.8.49 Jul 18, 2026
4 of 5 checks passed
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks @rafaumeu! Merged into release/v3.8.49 after combined merge-train validation. We compacted the generalized thinking-budget block and moved the #6914 cases to their own test file for the frozen-file cap — you stay author.

@diegosouzapw diegosouzapw mentioned this pull request Jul 23, 2026
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…dation (cheaper-inference#7000) (cheaper-inference#7001)

* feat(6922): register effort-tier aliases for glm-5.2 & mimo-v2.5 on opencode-go

Previously only deepseek-v4-pro had effort-tier aliases on the opencode-go
provider. GLM-5.2 and MiMo-V2.5 only had base model ids, making it impossible
to pin reasoning effort per combo target.

Changes:
- Generalize parseDeepSeekEffortLevel → parseEffortLevel with EFFORT_TIERS table
- deepseek-v4-pro: low/medium/high/max (unchanged)
- glm-5.2: high/max only (OpenAI transport; low/medium not supported)
- mimo-v2.5: high/max only (same reasoning)
- Register alias model ids in opencode-go registry
- Mark base models supportsReasoning: true
- 9 unit tests covering registry + executor + backward compat

Closes cheaper-inference#6922

* ci: retrigger CI for Electron Package Smoke flaky test

* ci: retrigger flaky Electron Package Smoke

* test(cheaper-inference#6922): rewrite tests to call real parseEffortLevel function

- Export parseEffortLevel from opencode.ts so tests can import it
- Replace grep-on-source-file assertions with real function calls
- 13 tests: 4 deepseek tiers + 2 glm-5.2 tiers + 2 mimo-v2.5 tiers
  + 5 negative cases (unknown model, unsupported tiers, empty, base-only)
- Remove dependency on readFileSync / string matching

* fix: DDG circuit breaker (cheaper-inference#6999) + null content validation (cheaper-inference#7000)

cheaper-inference#6999: Add lightweight circuit breaker to DuckDuckGo executor.
After 5 consecutive failures (429, 5xx, network errors), the breaker
opens for 30s — during that window every request fast-fails with 503
so the combo engine can immediately fail over to the next provider
instead of waiting for timeouts. Half-open probing happens naturally
once the cooldown expires. A single success resets the counter.

cheaper-inference#7000: Fix false positive in validateResponseQuality where multimodal
content arrays (empty []) and whitespace-only strings passed as valid.
Now properly validates: arrays must have >=1 non-empty part; strings
must have non-zero trimmed length.

* test: add regression tests for DDG circuit breaker (cheaper-inference#6999) and null content validation (cheaper-inference#7000)

- Circuit breaker: verifies 400 for empty messages is unaffected by CB state,
  and that CB starts closed (no 503 on first request)
- Null content (cheaper-inference#7000): verifies validateResponseQuality correctly flags
  null content, empty array content [] as invalid, and array with text as valid

* fix(ci): add ddg-circuit-breaker test to stryker tap.testFiles for mutation coverage gate

* test(cheaper-inference#6999): exercise the DDG circuit breaker state machine directly

The existing "circuit breaker fast-fails with 503 after consecutive
failures" test never actually drives 5 consecutive failures — it makes a
single real network call and only asserts the response isn't 503, which
passes whether or not the breaker logic works at all (confirmed by
disabling the open-threshold check entirely: that test stayed green).

Exports cbIsOpen/cbRecordFailure/cbRecordSuccess/CB_THRESHOLD/
CB_COOLDOWN_MS (previously module-private) plus two test-only helpers
(__setDdgCircuitBreakerStateForTests/__getDdgCircuitBreakerStateForTests,
following the __xxxForTests convention already used in
src/shared/utils/circuitBreaker.ts) so tests can drive the module-level
singleton directly instead of needing a full network mock through
warmSession/seedChallengeChain/acquireAuthHeaders, and without waiting
CB_COOLDOWN_MS=30s in real time for the half-open case.

New tests cover: starts closed; opens on the CB_THRESHOLD-th consecutive
failure (not before); execute() fast-fails with 503 while open without
reaching the network (verified: disabling the cbIsOpen() gate makes the
same test fall through to a real network call, ~1s slower and red);
still open just before cooldown elapses; self-closes once cooldown has
elapsed (half-open); cbRecordSuccess resets the counter.

Red-first proof (both independently green->red->restored-green):
1. `if (false && failures >= CB_THRESHOLD ...)` — neuters the open
   transition. Result: the new "opens after CB_THRESHOLD..." test fails;
   the pre-existing weak test stays green regardless.
2. `if (false && cbIsOpen())` — neuters the execute() gate. Result: the
   new "execute() fast-fails with 503 while open" test fails (and takes
   ~1s longer, falling through to a real network attempt instead of
   short-circuiting).

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…dation (cheaper-inference#7000) (cheaper-inference#7001)

* feat(6922): register effort-tier aliases for glm-5.2 & mimo-v2.5 on opencode-go

Previously only deepseek-v4-pro had effort-tier aliases on the opencode-go
provider. GLM-5.2 and MiMo-V2.5 only had base model ids, making it impossible
to pin reasoning effort per combo target.

Changes:
- Generalize parseDeepSeekEffortLevel → parseEffortLevel with EFFORT_TIERS table
- deepseek-v4-pro: low/medium/high/max (unchanged)
- glm-5.2: high/max only (OpenAI transport; low/medium not supported)
- mimo-v2.5: high/max only (same reasoning)
- Register alias model ids in opencode-go registry
- Mark base models supportsReasoning: true
- 9 unit tests covering registry + executor + backward compat

Closes cheaper-inference#6922

* ci: retrigger CI for Electron Package Smoke flaky test

* ci: retrigger flaky Electron Package Smoke

* test(cheaper-inference#6922): rewrite tests to call real parseEffortLevel function

- Export parseEffortLevel from opencode.ts so tests can import it
- Replace grep-on-source-file assertions with real function calls
- 13 tests: 4 deepseek tiers + 2 glm-5.2 tiers + 2 mimo-v2.5 tiers
  + 5 negative cases (unknown model, unsupported tiers, empty, base-only)
- Remove dependency on readFileSync / string matching

* fix: DDG circuit breaker (cheaper-inference#6999) + null content validation (cheaper-inference#7000)

cheaper-inference#6999: Add lightweight circuit breaker to DuckDuckGo executor.
After 5 consecutive failures (429, 5xx, network errors), the breaker
opens for 30s — during that window every request fast-fails with 503
so the combo engine can immediately fail over to the next provider
instead of waiting for timeouts. Half-open probing happens naturally
once the cooldown expires. A single success resets the counter.

cheaper-inference#7000: Fix false positive in validateResponseQuality where multimodal
content arrays (empty []) and whitespace-only strings passed as valid.
Now properly validates: arrays must have >=1 non-empty part; strings
must have non-zero trimmed length.

* test: add regression tests for DDG circuit breaker (cheaper-inference#6999) and null content validation (cheaper-inference#7000)

- Circuit breaker: verifies 400 for empty messages is unaffected by CB state,
  and that CB starts closed (no 503 on first request)
- Null content (cheaper-inference#7000): verifies validateResponseQuality correctly flags
  null content, empty array content [] as invalid, and array with text as valid

* fix(ci): add ddg-circuit-breaker test to stryker tap.testFiles for mutation coverage gate

* test(cheaper-inference#6999): exercise the DDG circuit breaker state machine directly

The existing "circuit breaker fast-fails with 503 after consecutive
failures" test never actually drives 5 consecutive failures — it makes a
single real network call and only asserts the response isn't 503, which
passes whether or not the breaker logic works at all (confirmed by
disabling the open-threshold check entirely: that test stayed green).

Exports cbIsOpen/cbRecordFailure/cbRecordSuccess/CB_THRESHOLD/
CB_COOLDOWN_MS (previously module-private) plus two test-only helpers
(__setDdgCircuitBreakerStateForTests/__getDdgCircuitBreakerStateForTests,
following the __xxxForTests convention already used in
src/shared/utils/circuitBreaker.ts) so tests can drive the module-level
singleton directly instead of needing a full network mock through
warmSession/seedChallengeChain/acquireAuthHeaders, and without waiting
CB_COOLDOWN_MS=30s in real time for the half-open case.

New tests cover: starts closed; opens on the CB_THRESHOLD-th consecutive
failure (not before); execute() fast-fails with 503 while open without
reaching the network (verified: disabling the cbIsOpen() gate makes the
same test fall through to a real network call, ~1s slower and red);
still open just before cooldown elapses; self-closes once cooldown has
elapsed (half-open); cbRecordSuccess resets the counter.

Red-first proof (both independently green->red->restored-green):
1. `if (false && failures >= CB_THRESHOLD ...)` — neuters the open
   transition. Result: the new "opens after CB_THRESHOLD..." test fails;
   the pre-existing weak test stays green regardless.
2. `if (false && cbIsOpen())` — neuters the execute() gate. Result: the
   new "execute() fast-fails with 503 while open" test fails (and takes
   ~1s longer, falling through to a real network attempt instead of
   short-circuiting).

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants