Skip to content

test(e2e): lock in rate-limit failover and saturated-alias behavior#496

Merged
SantiagoDePolonia merged 1 commit into
mainfrom
test/e2e-failover-harness
Jul 6, 2026
Merged

test(e2e): lock in rate-limit failover and saturated-alias behavior#496
SantiagoDePolonia merged 1 commit into
mainfrom
test/e2e-failover-harness

Conversation

@SantiagoDePolonia

@SantiagoDePolonia SantiagoDePolonia commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

What

Closes the last test gap from #482/#492: the two most subtle rate-limit behaviors — saturated primary → skip provider → failover serves and fully saturated alias → honest 429 — were covered by unit tests only, because the e2e fixture had no way to wire failover rules or a model resolver.

Changes

  • e2eServerOptions gains three fields mirroring the app wiring: a shared registry (so collaborators like virtual models and capacity probes see the same catalog), a modelResolver, and a failoverResolver.
  • TestRateLimitSaturatedPrimaryFailsOver_E2E: a model rule saturates gpt-4 after one request; with a failover rule to gpt-3.5-turbo, the second request returns 200 served by the failover target, and the recorded upstream traffic proves the saturated primary was never called again.
  • TestRateLimitFullySaturatedAliasReturns429_E2E: a two-target virtual model wired exactly like production (registry catalog + SetTargetCapacity probe) round-robins while a provider-scoped rule has capacity, then — with every target saturated — returns 429 + Retry-After + rate_limit_exceeded, not an unavailable-model error, with the blocked request never reaching upstream.

User-visible impact

None — test-only, plus fixture plumbing. Full e2e and unit suites pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved rate-limiting behavior when primary models are saturated, ensuring requests can fail over to an available model instead of returning an error.
    • Added clearer handling for fully saturated model aliases so requests now return a proper 429 Too Many Requests response with retry information and a standard error payload.

The two most subtle rate-limit behaviors had unit coverage only, because
the e2e fixture could not wire failover rules or a model resolver. The
fixture now accepts a shared registry, a model resolver, and a failover
resolver, mirroring the app wiring.

Two tests pin the end-to-end contracts:
- A rate-saturated primary route with configured failover rules is served
  by the failover target with no 429, and the saturated primary is never
  called again.
- A virtual model whose targets are all rate-saturated still resolves and
  returns the honest 429 with Retry-After and rate_limit_exceeded — not an
  unavailable-model error — after round-robin alternated targets while
  capacity lasted, and the blocked request never reaches upstream.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f210a338-9a34-477a-b440-1fe9a57844c2

📥 Commits

Reviewing files that changed from the base of the PR and between 383248d and 1914483.

📒 Files selected for processing (2)
  • tests/e2e/failover_test.go
  • tests/e2e/setup_test.go

📝 Walkthrough

Walkthrough

This PR adds a new E2E test file exercising rate-limit-driven failover and virtual-model alias saturation scenarios, including a static failover resolver and request/model inspection helpers. The E2E server test harness is extended with optional fields for injecting a shared registry, model resolver, and failover resolver.

Changes

Rate-limit failover E2E tests

Layer / File(s) Summary
Server harness supports injectable dependencies
tests/e2e/setup_test.go
e2eServerOptions gains registry, modelResolver, and failoverResolver fields; setupE2EServer reuses a provided registry when set and wires ModelResolver/FailoverResolver into server.Config.
Test scaffolding and shared helpers
tests/e2e/failover_test.go
Adds build tag, imports, staticFailoverResolver, sendModelChatRequest, and recordedChatModels helpers used across the new tests.
Primary saturation triggers failover
tests/e2e/failover_test.go
TestRateLimitSaturatedPrimaryFailsOver_E2E verifies that once a model-scoped rate limit saturates the primary, the next request is served by the configured failover target instead of returning 429.
Fully saturated alias returns 429
tests/e2e/failover_test.go
TestRateLimitFullySaturatedAliasReturns429_E2E wires a SQLite-backed virtual-model service and provider-scoped rate limiter, verifies round-robin routing across two targets, then asserts a truthful 429 with Retry-After and OpenAI-style error payload once both targets are saturated, without an extra upstream call.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • ENTERPILOT/GoModel#492: Adds the corresponding capacity-based balancer fix for fully saturated virtual-model targets that this PR's E2E tests validate.

Sequence Diagram(s)

sequenceDiagram
  participant Test
  participant Server
  participant RateLimiter
  participant PrimaryModel
  participant FailoverModel

  Test->>Server: send request 1 (primary model)
  Server->>RateLimiter: check capacity
  RateLimiter-->>Server: allowed
  Server->>PrimaryModel: forward request
  PrimaryModel-->>Test: 200

  Test->>Server: send request 2 (primary model)
  Server->>RateLimiter: check capacity
  RateLimiter-->>Server: saturated
  Server->>FailoverModel: forward request (failover)
  FailoverModel-->>Test: 200
Loading
sequenceDiagram
  participant Test
  participant Server
  participant VirtualModelService
  participant RateLimiter
  participant TargetA
  participant TargetB

  Test->>Server: alias request 1
  Server->>VirtualModelService: resolve target
  VirtualModelService->>RateLimiter: check capacity TargetA
  RateLimiter-->>VirtualModelService: allowed
  VirtualModelService->>TargetA: forward
  TargetA-->>Test: 200

  Test->>Server: alias request 2
  Server->>VirtualModelService: resolve target
  VirtualModelService->>RateLimiter: check capacity TargetB
  RateLimiter-->>VirtualModelService: allowed
  VirtualModelService->>TargetB: forward
  TargetB-->>Test: 200

  Test->>Server: alias request 3
  Server->>VirtualModelService: resolve target
  VirtualModelService->>RateLimiter: check capacity (both saturated)
  RateLimiter-->>VirtualModelService: denied
  VirtualModelService-->>Test: 429 rate_limit_exceeded
Loading

Poem

A rabbit tests the limits deep,
Two targets hop, then fall asleep,
Failover springs to save the day,
While 429 politely says "no way!"
Retry-After ticks in gentle rhyme —
Hop on, dear request, one more time. 🐇⏱️

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main test-only change: locking in rate-limit failover and saturated-alias behavior in e2e.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/e2e-failover-harness

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@greptile-apps

greptile-apps Bot commented Jul 6, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

This test-only PR is safe to merge with minimal risk.

The changes are limited to e2e tests and fixture dependency injection. The fixture wiring matches the production rate-limit and virtual-model setup. No functional or security issues were identified.

No files require special attention.

T-Rex T-Rex Logs

What T-Rex did

  • Reviewed the general-contract-validation-proof that documents a saturated primary failover warning followed by a successful failover and a fully saturated alias returning HTTP 429.
  • Confirmed the proof states the exit status was 0 and the Go tool version used was go1.26.4 linux/amd64.
  • Identified the two artifacts linked to the proof: a log artifact containing the failover sequence and HTTP 429 outcome, and an additional artifact providing execution metadata.

View all artifacts

T-Rex Ran code and verified through T-Rex

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Test as E2E Test
participant Server as setupE2EServer
participant Registry as Shared Registry
participant VM as Model Resolver / Virtual Model
participant RL as Rate Limiter
participant FO as Failover Resolver
participant Upstream as Mock Provider

Test->>Server: Build server with registry, rateLimiter, resolver(s)
Server->>Registry: Create router from shared catalog
Server->>RL: Check route capacity before provider call
alt Saturated primary with failover
    Server->>FO: Resolve alternate selector
    FO-->>Server: test/gpt-3.5-turbo
    Server->>RL: Verify failover target capacity
    Server->>Upstream: Send request to failover target
    Upstream-->>Test: 200 OK
else Fully saturated alias
    Server->>VM: Resolve balanced-chat target
    VM->>RL: Probe target capacity
    VM-->>Server: Fallback target for honest admission
    Server->>RL: Admission fails
    Server-->>Test: 429 rate_limit_exceeded + Retry-After
end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Test as E2E Test
participant Server as setupE2EServer
participant Registry as Shared Registry
participant VM as Model Resolver / Virtual Model
participant RL as Rate Limiter
participant FO as Failover Resolver
participant Upstream as Mock Provider

Test->>Server: Build server with registry, rateLimiter, resolver(s)
Server->>Registry: Create router from shared catalog
Server->>RL: Check route capacity before provider call
alt Saturated primary with failover
    Server->>FO: Resolve alternate selector
    FO-->>Server: test/gpt-3.5-turbo
    Server->>RL: Verify failover target capacity
    Server->>Upstream: Send request to failover target
    Upstream-->>Test: 200 OK
else Fully saturated alias
    Server->>VM: Resolve balanced-chat target
    VM->>RL: Probe target capacity
    VM-->>Server: Fallback target for honest admission
    Server->>RL: Admission fails
    Server-->>Test: 429 rate_limit_exceeded + Retry-After
end
Loading

Reviews (1): Last reviewed commit: "test(e2e): lock in rate-limit failover a..." | Re-trigger Greptile

@SantiagoDePolonia SantiagoDePolonia merged commit 3c81092 into main Jul 6, 2026
20 checks passed
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.

2 participants