fix(api-proxy): stop alias fallback picking arbitrary models - #6996
Conversation
A nested alias scoped to other providers (e.g. "haiku" on an OpenAI-only proxy) has a legitimately empty candidate set, but instead of contributing nothing it triggered middle-power fallback, which synthesized a pick from the entire live-discovered catalog. That guess then out-ranked the genuine siblings the same fan-out had already found. - Track fallback-derived candidates separately from genuine pattern matches and only use them when no sibling pattern matched anything. - Skip fallback for nested aliases that do not name the current provider. Scoped to nested references only; a directly requested model retains the existing graceful-degradation behaviour. - Soft price-filter the middle-power pool via an injected predicate, gated on the AI-credits guard being active. Falls back to the unfiltered pool when filtering would empty it. Refs #6993 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes alias fallback so synthesized models do not outrank genuine provider matches.
Changes:
- Separates synthesized and genuine alias candidates.
- Skips provider-mismatched nested fallbacks.
- Soft-filters fallback candidates by pricing availability.
- Adds regression tests.
Show a summary per file
| File | Description |
|---|---|
containers/api-proxy/model-resolver.js |
Prioritizes genuine alias matches. |
containers/api-proxy/model-fallback.js |
Adds optional pricing filtering. |
containers/api-proxy/model-config.js |
Connects pricing checks to fallback configuration. |
containers/api-proxy/model-resolver.test.js |
Adds fan-out and pricing regressions. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Balanced
Addresses PR review feedback. The fallback price filter used checkUnknownModelRejection, which is not a predicate: for every unpriceable candidate it reached resolveLowerPriorityPricing, emitting unknown_model_ai_credits_pricing and recording the model in warnedUnknownModels. Verified this emitted one warning per probed candidate (50/50 in a reproduction) and then suppressed the warning when such a model was genuinely requested later. Adds a quiet option to the pricing resolution path and exports a pure isModelPriceable(), used for speculative candidate filtering. The real request path keeps warning exactly as before. Also splits the mis-named fan-out test: the original asserted the all-mismatched null case under a name describing synthesized activation, leaving the synthesized-only branch uncovered. Now covers all three cases — all-mismatched returns null, synthesized-only activates fallback, and a genuine sibling match beats a synthesized one. Refs #6993 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>
|
✅ Copilot review passed with no inline comments. @davidslater Add the |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Build Test Failed Build Test Suite - See logs for details |
|
✅ Smoke Claude passed |
|
🛡️ Smoke Copilot Network Isolation reports failed while checking network isolation. Investigate the egress model. |
|
❌ Security Guard failed. Please review the logs for details. |
|
❌ Smoke Copilot BYOK AOAI (Entra) reports failed. AOAI BYOK (Entra) mode investigation needed... |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
❌ Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed... |
|
🔌 Smoke Services — All services reachable! ✅ |
|
📰 VERDICT: Smoke Docker Sbx has concluded. All systems operational. This is a developing story. 🎤 |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
✅ Smoke Gemini completed. All facets verified. 💎 |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
✅ Contribution Check completed successfully! PR #6996 follows the contribution guidelines: the change is in the correct container/source paths, includes substantial regression tests, and the PR description is clear with a related issue reference. No contribution-guideline issues found. |
|
Chroot tests failed Smoke Chroot failed - See logs for details. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
✅ Build Test Suite completed successfully! |
|
✅ Smoke Gemini completed. All facets verified. 💎 |
|
EGRESS_RESULT allow=pass deny=pass ✅ Allowed domain (github.com) reachable: Overall: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
|
Smoke Test: Copilot Engine (
Overall: PASS
|
Smoke Test: Claude Engine Validation
Overall result: PASS
|
|
Smoke Test: Copilot BYOK (Direct) Mode Recent merged PRs (MCP check):
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) via api-proxy → api.githubcopilot.com Overall: PASS cc
|
|
Smoke Test Results: Services Connectivity
Overall: FAIL —
|
|
Smoke test results: MCP ✅, Connectivity ❌, Writing ✅, Bash ✅. Overall: FAIL.
|
🏗️ Build Test Suite Results
Overall: 7/8 ecosystems passed — FAIL Error DetailsJava (gson, caffeine) — The Maven proxy in
|
📡 OTel Tracing Smoke Test Results
All scenarios pass — no regression detected in OTel tracing integration.
|
Chroot Version Comparison
Overall: FAILED (
|
Smoke Test: Docker Sbx
Overall: PASS cc
|
|
Smoke test results:
Overall status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Fixes #6993
Problem
Alias resolution could resolve to an internal provider staging model that nobody configured. In gh-aw-threat-detection run 31034552109, the
detectionalias resolved tocrest-alpha-0418-block-cy4.5.The chain is gh-aw's built-in table, not user config:
On an OpenAI-only proxy,
haikuandgemini-flash-liteare scoped to other providers and legitimately match nothing. Rather than contributing no candidates, each triggered middle-power fallback, which could not infer a family prefix, fell back to the entire ~220-model live catalog, tiered it (onlygpt-5/gpt-4/gpt-3.5get real tiers, so ~200 staging entries tie at tier 1), and took the alphabetical median.compareByVersionthen ranked that synthesized pick ahead of the two legitimate candidates the same fan-out had already found.Measured against a representative catalog, 52 of the 59 built-in aliases resolved to a staging model — including every meta-alias (
agent,auto,large,small,summarization) and the engine aliases.detectionis simply the one that ran.Changes
Synthesized candidates no longer out-rank genuine matches.
_resolveAliasPatternsnow tracks candidates from a nested alias's fallback separately, and only uses them when no sibling pattern matched anything. The returnedfallback.activatedflag reflects whether the winner was synthesized.Provider-mismatched nested aliases are skipped. The fallback gate checked
patterns.some(p => p.includes('/'))— "some pattern names a provider" — instead of "some pattern names the current provider". Scoped deliberately to nested references: a directly requested model keeps today's graceful-degradation behaviour, which is intentional and covered by an existing test (should fall back when provider patterns do not match current provider).Soft price filter on the fallback pool.
selectMiddlePowerFallbackaccepts an optionalisModelPriceablepredicate, injected frommodel-config.jsand active only when the AI-credits guard is (a credit cap set, no configured default pricing). Applied only to this synthesized-selection path — explicitly requested and pattern-matched models are untouched. If filtering would empty the pool it uses the unfiltered pool, and a throwing predicate is treated as priceable, so it can never turn a success into a failure.Verification
Against the real gh-aw alias table and a synthetic 208-model catalog:
detectionnow resolves togpt-5-mini, as intended.npm run buildclean; verified no circular dependency from the newmodel-config.js→ai-credits-guardimportDeliberately not included
Ordered-list semantics. gh-aw documents patterns as "tried in sequence until one resolves", which AWF does not implement. This looked like the highest-coverage fix, but it regresses
sonnet-6xfromclaude-sonnet-5toclaude-sonnet-4.5— so the built-in table is not consistently written assuming priority order. That is a cross-repo contract ambiguity needing gh-aw's input.Whether
middle_powershould be on by default. It currently fabricates a model whenever an alias misses, and nobody opted in.Whether "median capability" suits a
mini/smallalias. Price-filtering can yieldgpt-4-turbo($10/$30 per 1M) where the alias asked forgpt-5-nano($0.05/$0.40) — trading a hard failure for a silent ~100× cost increase. Acheapest_pricedstrategy may be more appropriate.Known residual
Six aliases can still reach a staging model:
any,agent,copilot,claude,codex,gemini. All bottom out inany->[copilot/*, anthropic/*, openai/*, ...], whoseopenai/*wildcard genuinely matches everything. That iscompareByVersionranking rather than fallback, so it is out of scope here and warrants a separate issue.Caveat
The 220-model catalog used for measurement is synthetic, matching the shape reported from the run; the affected set is a property of the alias table rather than the catalog, but exact picks vary. I could not read the run's artifacts directly, so the assumption that the failure surfaced via
checkUnknownModelRejectionis unverified — the run'smodels.jsonartifact would confirm.