tests: add weightless VK allow-list and case-insensitive alias fallback e2e scenarios - #4253
Merged
Pratham-Mishra04 merged 1 commit intoJun 11, 2026
Conversation
This was referenced Jun 10, 2026
|
|
Collaborator
Author
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds support for emitting weightless virtual keys by omitting ChangesE2E Routing Tests
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant BifrostRouter
participant RoutingEngine
participant Provider
participant LogStore
TestRunner->>BifrostRouter: send route request (model or alias)
BifrostRouter->>RoutingEngine: evaluate VK provider_configs and aliases
RoutingEngine->>RoutingEngine: apply allow-list pruning (weightless configs -> no weighted entries)
RoutingEngine->>Provider: select provider/key (no load-balancing when "No weighted configs")
RoutingEngine->>LogStore: emit routing_engine_logs (allow-list & LB-skip markers)
BifrostRouter->>TestRunner: return routed provider/key and alias resolution info
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This was referenced Jun 10, 2026
Pratham-Mishra04
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/api/collections/bifrost-routing-wiring.postman_collection.json`:
- Around line 2249-2287: The prerequest delay is missing for the request
"rt-weightless-vk-allowlist-via-logs-08-assert-trail" (08. log trail shows
allow-list filtering and LB skipped), causing the first poll attempt in the test
script to possibly read incomplete routing_engine_logs; add a prerequest script
that computes pollKey as '__poll_' + pm.info.requestName, reads and sets
__cur_poll_key and __cur_poll_attempt from the collection variable for that
pollKey, and when attempt === 0 introduces a short sleep (e.g. ~1s) before the
first request so the polling logic in the test (which relies on __cur_poll_key
and __cur_poll_attempt) has the same initial-delay behavior as steps 06/07.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 08aca855-4287-4d4b-8db3-993ad9425695
📒 Files selected for processing (2)
tests/e2e/api/collections/bifrost-routing-wiring.postman_collection.jsontests/e2e/api/runners/build-routing-wiring.mjs
Comment on lines
+2249
to
+2287
| "id": "rt-weightless-vk-allowlist-via-logs-08-assert-trail", | ||
| "name": "08. log trail shows allow-list filtering and LB skipped [weightless-vk-allowlist-via-logs]", | ||
| "event": [ | ||
| { | ||
| "listen": "test", | ||
| "script": { | ||
| "type": "text/javascript", | ||
| "exec": [ | ||
| "var maxAttempts = 8;", | ||
| "var pollKey = pm.collectionVariables.get('__cur_poll_key');", | ||
| "var attempt = parseInt(pm.collectionVariables.get('__cur_poll_attempt') || '0', 10);", | ||
| "var cleanupReq = \"cleanup: delete vk [weightless-vk-allowlist-via-logs]\";", | ||
| "function assertNow() {", | ||
| " if (pm.response.code !== 200) { throw new Error('log detail status ' + pm.response.code); }", | ||
| " var row = pm.response.json() || {};", | ||
| " var trail = row.routing_engine_logs || '';", | ||
| " var expected = [\"not in allowed models list\",\"No weighted configs\",\"skipping load balancing\"];", | ||
| " expected.forEach(function (s) {", | ||
| " if (String(trail).indexOf(s) < 0) { throw new Error('routing_engine_logs missing ' + JSON.stringify(s) + '; got ' + JSON.stringify(trail)); }", | ||
| " });", | ||
| "}", | ||
| "var ok = true, errMsg = '';", | ||
| "try { assertNow(); } catch (e) { ok = false; errMsg = e.message; }", | ||
| "if (ok) {", | ||
| " pm.collectionVariables.set(pollKey, '0');", | ||
| " pm.test(\"08. log trail shows allow-list filtering and LB skipped [weightless-vk-allowlist-via-logs]\", function () { pm.expect(true, 'assertion satisfied').to.be.true; });", | ||
| "} else if (attempt < maxAttempts) {", | ||
| " pm.collectionVariables.set(pollKey, String(attempt + 1));", | ||
| " var sleepMs = Math.min(250 * Math.pow(2, attempt), 4000);", | ||
| " var start = Date.now(); while (Date.now() - start < sleepMs) {}", | ||
| " pm.execution.setNextRequest(pm.info.requestName);", | ||
| "} else {", | ||
| " pm.collectionVariables.set(pollKey, '0');", | ||
| " pm.test(\"08. log trail shows allow-list filtering and LB skipped [weightless-vk-allowlist-via-logs]\", function () { throw new Error(errMsg); });", | ||
| " pm.execution.setNextRequest(cleanupReq);", | ||
| "}" | ||
| ] | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Step 08 missing initial delay in prerequest script.
Steps 06 and 07 include a prerequest script with an initial delay (3s and 2s respectively) before the first polling attempt. Step 08 (log trail assertion) lacks this prerequest delay. If the routing_engine_logs field is populated asynchronously after the log row is created, step 08 may attempt to assert on incomplete logs on the first attempt.
Consider adding a prerequest script similar to step 07:
{
"listen": "prerequest",
"script": {
"type": "text/javascript",
"exec": [
"var pollKey = '__poll_' + pm.info.requestName;",
"var attempt = parseInt(pm.collectionVariables.get(pollKey) || '0', 10);",
"pm.collectionVariables.set('__cur_poll_key', pollKey);",
"pm.collectionVariables.set('__cur_poll_attempt', String(attempt));",
"if (attempt === 0) { var __ws = Date.now(); while (Date.now() - __ws < 1000) {} }"
]
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/e2e/api/collections/bifrost-routing-wiring.postman_collection.json`
around lines 2249 - 2287, The prerequest delay is missing for the request
"rt-weightless-vk-allowlist-via-logs-08-assert-trail" (08. log trail shows
allow-list filtering and LB skipped), causing the first poll attempt in the test
script to possibly read incomplete routing_engine_logs; add a prerequest script
that computes pollKey as '__poll_' + pm.info.requestName, reads and sets
__cur_poll_key and __cur_poll_attempt from the collection variable for that
pollKey, and when attempt === 0 introduces a short sleep (e.g. ~1s) before the
first request so the polling logic in the test (which relies on __cur_poll_key
and __cur_poll_attempt) has the same initial-delay behavior as steps 06/07.
Contributor
Confidence Score: 5/5Changes are confined to E2E test definitions and the collection builder; no production code paths are touched. The new scenarios are well-structured with full setup/teardown and polling retry logic. The No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "tests: cover weightless-VK allow-list an..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+578
to
+584
| title: "A weightless VK is an allow-list (no LB), confirmed by the routing log trail", | ||
| description: "Two providers on a VK with NO weights: A's key serves gpt-4o-mini, B's serves only gpt-4o. Routing the bare gpt-4o-mini, governance filters by capability (excludes B) and — having no weighted configs — skips load balancing, routing to A. The routing_engine_logs record the allow-list decisions.", | ||
| steps: [ | ||
| { type: "addProvider", ref: "self", providerType: "openai", keys: [key({ id: "ka", models: [MODEL_B] })] }, | ||
| { type: "addProvider", ref: "b", providerType: "openai", keys: [key({ id: "kb", models: [MODEL_A] })] }, | ||
| { type: "createVK", providerConfigs: [vkProvider({ providerRef: "self", weight: null, allowedModels: ["*"] }), vkProvider({ providerRef: "b", weight: null, allowedModels: ["*"] })] }, | ||
| { type: "route", model: MODEL_B, bareModel: true, expectStatus: 200, expectProviderOneOf: ["self"], waitSeconds: 3, label: "bare model routes to the only capable provider (allow-list, no LB)" }, |
Contributor
There was a problem hiding this comment.
The inline description says "governance filters by capability (excludes B)," but both VK provider configs use allowed_models: ["*"], so VK governance passes both providers through. Provider B is excluded at the per-key models list level (key-catalog layer) because key kb only declares ["gpt-4o"]. The log message "not in allowed models list" that the trail assertion expects refers to that key-level check, not VK governance. The description is misleading to anyone who needs to diagnose a failure or understand the routing layers being exercised.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Pratham-Mishra04
force-pushed
the
06-10-tests_cover_blacklist_multi-key_keyless_details_base_catalog_cases
branch
from
June 11, 2026 06:32
e726dc8 to
b7fd0f8
Compare
Pratham-Mishra04
force-pushed
the
06-10-tests_cover_weightless-vk_allow-list_and_case-insensitive_alias_fallback
branch
from
June 11, 2026 06:32
8f3421e to
0e4e39e
Compare
18 tasks
Collaborator
Author
Merge activity
|
Pratham-Mishra04
changed the base branch from
06-10-tests_cover_blacklist_multi-key_keyless_details_base_catalog_cases
to
graphite-base/4253
June 11, 2026 07:26
Pratham-Mishra04
force-pushed
the
06-10-tests_cover_weightless-vk_allow-list_and_case-insensitive_alias_fallback
branch
from
June 11, 2026 07:29
0e4e39e to
51f152c
Compare
Pratham-Mishra04
deleted the
06-10-tests_cover_weightless-vk_allow-list_and_case-insensitive_alias_fallback
branch
June 11, 2026 07:31
akshaydeo
pushed a commit
that referenced
this pull request
Jun 12, 2026
…ck e2e scenarios (#4253) ## Summary This PR adds two new E2E routing scenarios and fixes the field ordering of `weight` in virtual key provider config payloads across all existing test cases. It also updates the VK builder to support omitting `weight` entirely, enabling "weightless" (allow-list-only) virtual key configurations that skip load balancing. ## Changes - **Weightless VK allow-list scenario**: Adds a new E2E test (`weightless-vk-allowlist-via-logs`) that creates a VK with two providers and no weights. Provider A serves `gpt-4o-mini`, Provider B serves only `gpt-4o`. Routing `gpt-4o-mini` confirms governance filters out Provider B by capability and, finding no weighted configs, skips load balancing entirely and routes to Provider A. The `routing_engine_logs` are asserted to contain `"not in allowed models list"`, `"No weighted configs"`, and `"skipping load balancing"`. - **Case-insensitive alias fallback scenario**: Adds a new E2E test (`alias-case-insensitive-fallback`) that registers a mixed-case alias (`CatWiring-CI-{{run_id}}`) on a key and routes the lowercased form. The test asserts that the router resolves the alias via a case-insensitive fallback and that `routing_info.resolved_key_alias.model_id` equals `gpt-4o-mini`. - **`weight: null` support in VK builder**: The `createVK` step in `build-routing-wiring.mjs` now omits the `weight` field entirely when `pc.weight === null`, rather than defaulting to `1`. This is what enables the weightless VK scenario above. - **Field ordering normalization**: Moved `weight` to the end of the provider config object in all existing VK creation payloads throughout the Postman collection, making the ordering consistent with the new builder output. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Run the Postman collection or the Newman-based E2E runner against a live Bifrost instance: ```sh # Run the full routing-wiring collection via Newman newman run tests/e2e/api/collections/bifrost-routing-wiring.postman_collection.json \ --environment <your-env-file> \ --folder "A weightless VK is an allow-list (no LB), confirmed by the routing log trail" \ --folder "A request resolves to an alias whose name differs only in case" ``` To regenerate the Postman collection from the scenario definitions: ```sh node tests/e2e/api/runners/build-routing-wiring.mjs ``` Verify the generated collection matches the committed one. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations None. Changes are limited to E2E test definitions and the test collection builder. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added scenarios validating "weightless" virtual-key behavior that enforces allow-list filtering and skips load balancing, with assertions from routing logs. * Added scenario verifying case-insensitive alias resolution. * Adjusted test request payloads to reorder fields for consistency and clearer validation across routing/governance scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds two new E2E routing scenarios and fixes the field ordering of
weightin virtual key provider config payloads across all existing test cases. It also updates the VK builder to support omittingweightentirely, enabling "weightless" (allow-list-only) virtual key configurations that skip load balancing.Changes
Weightless VK allow-list scenario: Adds a new E2E test (
weightless-vk-allowlist-via-logs) that creates a VK with two providers and no weights. Provider A servesgpt-4o-mini, Provider B serves onlygpt-4o. Routinggpt-4o-miniconfirms governance filters out Provider B by capability and, finding no weighted configs, skips load balancing entirely and routes to Provider A. Therouting_engine_logsare asserted to contain"not in allowed models list","No weighted configs", and"skipping load balancing".Case-insensitive alias fallback scenario: Adds a new E2E test (
alias-case-insensitive-fallback) that registers a mixed-case alias (CatWiring-CI-{{run_id}}) on a key and routes the lowercased form. The test asserts that the router resolves the alias via a case-insensitive fallback and thatrouting_info.resolved_key_alias.model_idequalsgpt-4o-mini.weight: nullsupport in VK builder: ThecreateVKstep inbuild-routing-wiring.mjsnow omits theweightfield entirely whenpc.weight === null, rather than defaulting to1. This is what enables the weightless VK scenario above.Field ordering normalization: Moved
weightto the end of the provider config object in all existing VK creation payloads throughout the Postman collection, making the ordering consistent with the new builder output.Type of change
Affected areas
How to test
Run the Postman collection or the Newman-based E2E runner against a live Bifrost instance:
To regenerate the Postman collection from the scenario definitions:
Verify the generated collection matches the committed one.
Breaking changes
Related issues
Security considerations
None. Changes are limited to E2E test definitions and the test collection builder.
Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit