Skip to content

fix(resilience): accept globalConcurrentRequests and budgetMs in the settings schema - #18401

Open
HouMinXi wants to merge 3 commits into
cheaper-inference:release/v3.8.53from
HouMinXi:fix/18390-request-queue-settings-schema
Open

HouMinXi wants to merge 3 commits into
cheaper-inference:release/v3.8.53from
HouMinXi:fix/18390-request-queue-settings-schema

Conversation

@HouMinXi

Copy link
Copy Markdown
Contributor

Summary

Settings > Resilience > Request Queue could not be saved. The card always posts requestQueue.globalConcurrentRequests, and the strict requestQueueSettingsSchema did not list that key, so PATCH /api/resilience answered every save with a validation error (Unrecognized key: globalConcurrentRequests) and nothing in the section persisted (requests per minute, min time between requests, concurrency, queue wait, execution timeout, queue depth).

  • Root cause: feat(resilience): add hierarchical concurrency admission #11493 added the global concurrency gate to the normalizer, the types, the defaults and the dashboard card, but put the schema key on legacyResilienceDefaultsSchema (the legacy defaults block) instead of requestQueueSettingsSchema. The legacy patch mapper never forwarded that key either, so it was accepted there and silently dropped.
  • requestQueueSettingsSchema now accepts globalConcurrentRequests as an integer from 0 to 100000, the same bounds as normalizeRequestQueueSettings. 0 keeps meaning "global gate disabled".
  • waitForCooldownSettingsSchema now accepts budgetMs as an integer from 0 to 300000, the ceiling of normalizeWaitForCooldownSettings. Values below maxRetryWaitSec * 1000 are still raised to that floor by the normalizer, as before.
  • The legacy defaults.globalConcurrentRequests key is now mapped to requestQueue.globalConcurrentRequests instead of being dropped.
  • Unknown keys in both sections are still rejected. The stored settings key (resilienceSettings), the defaults and the GET response shape are unchanged. A saved non-zero globalConcurrentRequests now reaches the global gate in executeProviderRequest, and a legacy client that sends defaults.globalConcurrentRequests now changes it too.

Related Issues

Target Branch

  • Base is the active release/v3.8.53 branch (the highest open release/v*), not main
  • Not a new feature

Validation

  • Change type: other (settings API validation)
  • Focused tests: tests/unit/resilience-request-queue-schema-18390.test.ts (10 pass), plus every unit file that imports updateResilienceSchema or the resilience route (10 files, 78 pass)
  • Prettier and ESLint on changed files, core typecheck, changelog integrity, file size, cycles and boundary ratchets
  • Reconciled with the current active release base (8a8c64ee33)
  • Production code changes include a new automated test in this PR

Tests Added Or Updated

  • tests/unit/resilience-request-queue-schema-18390.test.ts (new):
    • Schema accepts globalConcurrentRequests at 0, 1, 64 and 100000, and the whole Request Queue card payload; rejects -1, 100001, 1.5, a string and a misspelled key.
    • Schema accepts waitForCooldown.budgetMs at 0, 30000 and 300000; rejects -1, 300001 and 1000.5.
    • Round trip through the real GET and PATCH handlers on an isolated data dir: the card payload built from GET saves with status 200 and reads back; the legacy defaults.globalConcurrentRequests key is applied; budgetMs is stored and floored at the single wait cap; an out-of-range value gets a 400.

Coverage Notes

  • Both schema lines and the legacy mapping in src/app/api/resilience/route.ts are covered by the new test. Ten mutants were all killed: dropping either schema key, dropping the legacy mapping, removing the min, max or integer constraint on globalConcurrentRequests, raising its minimum to 1, and removing the max or min of budgetMs or raising its minimum to 1.

Reviewer Notes

  • The dashboard never sends waitForCooldown.budgetMs (the GET response and the card omit it), so that half of the fix only matters for API and CLI callers. It is accepted because the normalizer already reads and stores it, matching comboCooldownWait.budgetMs.
  • The issue reports a 422; this route answers schema failures with 400.
  • The card input for globalConcurrentRequests has no upper bound, so typing a value above 100000 now gets a 400 that names the field.

…settings schema (cheaper-inference#18390)

The strict resilience schemas omitted two keys the normalizers read.
requestQueueSettingsSchema had no globalConcurrentRequests, so every save of
the Request Queue card (which always posts it) was rejected as an
unrecognized key. waitForCooldownSettingsSchema had no budgetMs.

cheaper-inference#11493 added the global gate to the legacy defaults schema instead of the
requestQueue schema, and the legacy patch mapper never forwarded it. Add both
keys with the normalizer bounds and map the legacy key.

Closes cheaper-inference#18390
…per (cheaper-inference#18390)

Mapping globalConcurrentRequests pushed normalizeLegacyPatch to 83
lines and complexity 16, so the complexity ratchet saw route.ts go from
2 to 4 violations. The four requestQueue ternaries now live in
buildLegacyRequestQueuePatch, unchanged, and route.ts is back to the
two pre-existing PATCH violations. No behavior change.
@HouMinXi

Copy link
Copy Markdown
Contributor Author

Pushed one small commit to bring route.ts back under the complexity ratchet. The four request-queue mappings from this PR pushed normalizeLegacyPatch to 83 lines and complexity 16, so I moved them unchanged into a helper called buildLegacyRequestQueuePatch. There is no behavior change, and the file is back to its 2 existing violations in PATCH. The boundary ratchet shows 776 against 774 on the current release tip, but 776 against 776 on this PR's own base, so the PR adds no edges.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(resilience): strict settings schemas reject keys the normalizers read (requestQueue.globalConcurrentRequests, waitForCooldown.budgetMs)

1 participant