Skip to content

feat(providers): allow per-connection maxWaitMs rate-limit override - #11251

Merged
diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.50from
maxmad64bis:feat/connection-max-wait-ms-override
Aug 23, 2026
Merged

feat(providers): allow per-connection maxWaitMs rate-limit override#11251
diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.50from
maxmad64bis:feat/connection-max-wait-ms-override

Conversation

@maxmad64bis

@maxmad64bis maxmad64bis commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #9985

Summary

  • Lets an operator override the rate-limit queue wait timeout (maxWaitMs) for a single provider connection, the same way rpm/tpm/tpd/minTime/maxConcurrent already work. Today that timeout is one global number (resilienceSettings.requestQueue.maxWaitMs), so a provider that's simply slower than the rest — reasoning models, heavier context — either eats early 504s or forces everyone else to wait longer too. This also folds the previous zai-web-only hardcoded exception into the same per-connection mechanism, so any provider can get that treatment without a code change.
  • Not to be confused with the unrelated providerSpecificData.timeoutMs / FETCH_TIMEOUT_MS (feat(providers): restore per-connection upstream timeout tier #10885) — that bounds how long OmniRoute waits for the upstream HTTP response once a request is dispatched. This PR bounds how long a request may sit in the local rate-limit queue before it is dispatched at all.

Related Issues

Validation

  • Change type: provider
  • Focused tests and category gates from the golden path
  • npm run lint
  • Reconciled with the current active release base; focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR

Tests Added Or Updated

  • tests/unit/provider-rate-limit-overrides-schema.test.ts — new maxWaitMs schema cases (valid, string coercion, ceiling, rejects negative/float, 0 means no override)
  • tests/unit/ratelimit-admission-control-6593.test.ts — new resolveRequestQueueMaxWaitMs cases (override wins over both the plain global default and the zai-web floor; 0 means no override)
  • tests/unit/dashboard/edit-connection-modal-max-wait-ms-override.test.tsx — new component test covering the dashboard field's empty state, persisted state, and the actual submit mapping into rateLimitOverrides.maxWaitMs

Coverage Notes

  • Touches src/ and open-sse/. The new field follows the exact code path already covered for rpm/minTime/maxConcurrent (schema → connectionRateLimitOverrides map → resolver), so this is additive coverage on an already-tested mechanism, not a new untested surface.

Reviewer Notes

  • resolveRpm/resolveMinTime/resolveMaxConcurrent are collapsed into one resolveOverride helper as part of wiring in maxWaitMs — same behavior, less duplication, no call-site changes for the existing three fields.
  • queueTimeoutMs/maxQueueDepth (the direct-connection semaphore settings from feat: expose queueTimeoutMs / maxQueueDepth for direct provider connections #8884) are a different mechanism and intentionally untouched here.
  • i18n: 2 new source strings in en.json, propagated to the other 41 locales as __MISSING__ placeholders (the repo's existing convention for pending translations) rather than running the bulk translate pass — the base branch already carries a large pre-existing translation backlog unrelated to this PR, and pulling that in here would balloon the diff.
  • npm run check:dashboard-typecheck currently fails on this base independent of this PR: EndpointPageClient.tsx has a broken JSX return statement from Beginner UX: guided endpoint connection header #11228 (unrelated file, not touched here). Confirmed identical on origin/release/v3.8.50 before this PR's commit.

@maxmad64bis
maxmad64bis force-pushed the feat/connection-max-wait-ms-override branch from dca645c to 7d7d1f6 Compare August 23, 2026 14:21
@maxmad64bis
maxmad64bis force-pushed the feat/connection-max-wait-ms-override branch from 7d7d1f6 to 24a47ce Compare August 23, 2026 14:22
@maxmad64bis maxmad64bis mentioned this pull request Aug 23, 2026
Closed
26 tasks
@maxmad64bis
maxmad64bis marked this pull request as draft August 23, 2026 15:50
Max
fix(providers): allow maxWaitMs in the rateLimitOverrides persistence…
484ec30 
… allowlist

diegosouzapw#11251 added maxWaitMs to the Zod validation schema and the
EditConnectionModal UI, but not to the separate allowlist in
sanitizeRateLimitOverrides() (src/lib/db/providers/columns.ts) that gates
what actually gets persisted. Saving the field from the dashboard threw
"Refusing to persist rateLimitOverrides with rejected keys: maxWaitMs"
(500) on every attempt — the modal's onSave() error path has no visible
surfacing for this case, so the save silently appeared to do nothing.

Adds maxWaitMs to the allowlist. TDD: tests/unit/columns-validation.test.ts
red before the fix (asserted maxWaitMs rejected), green after.
@maxmad64bis
maxmad64bis marked this pull request as ready for review August 23, 2026 15:59
@diegosouzapw
diegosouzapw merged commit a054ac4 into diegosouzapw:release/v3.8.50 Aug 23, 2026
11 of 16 checks passed
diegosouzapw pushed a commit that referenced this pull request Aug 23, 2026
@backryun
fix(lint): clear the 'No new ESLint warnings' red + last UA pin (#11247/
0bc60c3 
#11249/#11251/#11252/#10952 follow-ups)

The gate (eslint full-tree, baseline 0) failed on the merged head with 26
unsuppressed errors, all introduced by same-day base merges — none by this
PR. Each fixed at the cause (no new suppressions):

1. tests/integration/qdrant-routes.test.ts (#11249/#11213): 20x
   no-explicit-any on route-invocation casts. Fix: typed asNextRequest()
   adapter (Request -> NextRequest) replacing every "as any".

2. tests/unit/effort-tiers-loop-catalog-e2e.test.ts (#11252): unused
   after/beforeEach imports — newly error-level under the no-unused-vars
   ratchet #11247 shipped the same day. Fix: trim the import (the file
   uses test.after/test.beforeEach method forms).

3. EditConnectionModal.tsx (#11251): react-hooks/set-state-in-effect on
   the modal-open setFormData init. The pattern (sync form state with the
   loaded connection when isOpen flips) is the codebase's sanctioned
   exception — same justified eslint-disable-next-line already used in
   FreePoolTab.tsx and BatchConceptCard.tsx; a key-remount of the 30+
   field form would be a behavior-risking restructure for a basereds PR.

4. tests/unit/usage-service-hardening.test.ts (shard 2/4): last stale
   Copilot wire-identity pin — #10952 re-based it on the live-captured CLI
   1.0.81-6 (copilot-developer-cli integration id, API version 2026-08-01).
   Assertions aligned (UA, Editor-Version, Editor-Plugin-Version,
   X-GitHub-Api-Version).

Validation: all four files eslint-clean with the frozen suppressions;
usage-service-hardening 23/23 and effort-tiers 2/2 RED->GREEN
(node --import tsx/esm --test).

Refs #9985
This was referenced Aug 23, 2026
Merged
Merged
xiaoyaner0201 added a commit to xiaoyaner0201/OmniRoute that referenced this pull request Aug 26, 2026
@xiaoyaner0201
fix(costs): disclose flat-rate estimates instead of labelling them as…
a8b23e0 
… billed spend (diegosouzapw#11459)

The costs dashboard opts into flat-rate estimate mode
(`includeFlatRateEstimates=true`, CostOverviewTab.tsx) but every cost surface on
the page kept asserting actual billed money: "Spend Today" / "Spend 7D" /
"Spend 30D" / "Selected Window" / "Avg Cost / Request", the literal `Total Cost,`
CSV summary row, and the month-end projection derived from the same number. A
flat-rate subscription that renders $0 in billed-cost mode therefore rendered a
token-price estimate under money labels with no disclosure.

The analytics API already returns the truthful `includesFlatRateEstimates` flag
(src/app/api/usage/analytics/route.ts:909), but no UI consumed it. This consumes
it instead of relabelling every card, so the page keeps its billed-cost wording
whenever it is showing billed cost:

- a disclosure note next to the cost cards when the flag is true;
- a marker on the month-end forecast;
- a header comment plus a "Total Cost (includes flat-rate estimates)" summary
  row in the CSV export, and an `includesFlatRateEstimates` field in the JSON
  export.

Only an explicit `true` switches the wording — omitted, `false`, malformed and
unknown values all keep the billed-cost presentation, matching the API's own
`=== "true"` parsing, so the $0 default for flat-rate subscriptions is unchanged.
No API contract change: the flag is read, never renamed or re-keyed.

i18n: `costs.flatRateEstimateNotice` and `costs.flatRateEstimateForecast` added
to all 43 locales — real copy in en and vi (vi never ships placeholders per repo
convention), `__MISSING__:<en>` sentinels elsewhere, matching the pattern used by
diegosouzapw#11251. The CSV/JSON markers stay English literals because the export already
emits English headers and runs without an i18n runtime.

Tests: tests/unit/ui/cost-overview-flat-rate-disclosure.test.tsx (new, 6 cases)
asserts disclosure appears on the page, the forecast and the CSV summary row when
the flag is true, and that the billed-cost presentation is byte-identical
(`Total Cost,$30.00`) when the flag is absent or false. Verified RED first:
4 failed / 2 passed before the fix, 6 passed after.
@xiaoyaner0201 xiaoyaner0201 mentioned this pull request Aug 26, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@diegosouzapw diegosouzapw Awaiting requested review from diegosouzapw diegosouzapw is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@maxmad64bis @diegosouzapw @hartmark