fix(combos): align strategy contracts

[dhaern]

Summary

• Align combo routing strategy contracts across runtime, validation schemas, MCP tools, UI defaults, and shared types.
• Implement the previously advertised fill-first and p2c combo strategies with conservative runtime behavior.
• Prevent combo strategy tests from writing to the live data directory and fix the post-Gemini CLI duplicate import in chat-pipeline.test.ts.

Details

Strategy contract alignment

• Centralizes combo strategy values in ROUTING_STRATEGY_VALUES and auto-routing values in AUTO_ROUTING_STRATEGY_VALUES.
• Separates account-level fallback strategies into ACCOUNT_FALLBACK_STRATEGY_VALUES so combo-only strategies are not accepted as global account fallback settings.
• Updates combo schemas, legacy combo schemas, settings schemas, MCP listCombos output, and MCP setRoutingStrategy input to use the shared contracts.
• Keeps combo defaults and global account fallback settings separate in the Settings UI to avoid partial saves when a combo-only strategy is selected.

Runtime behavior

• Makes fill-first explicit as priority-order routing instead of an implicit fallthrough.
• Adds a minimal p2c implementation that samples two targets and orders the better candidate first using available metrics and circuit-breaker state.
• Makes context-optimized preserve the original order when no context capability data is known, instead of pretending to optimize unknown models.
• Normalizes legacy combo strategy names at safe boundaries:
  • usage → least-used
  • context → context-optimized
  • unknown runtime/import values → priority

Test and hygiene fixes

• Isolates combo-strategies.test.ts with a temporary DATA_DIR, preventing tests from creating UUID-named combos in a real user database.
• Rewrites that test to use current strategies and assert real routing behavior instead of swallowing exceptions.
• Adds JSON import coverage for legacy strategy normalization.
• Removes stale autoCombo barrel exports that pointed to non-existent functions.
• Removes the duplicate setCliCompatProviders import that broke chat-pipeline.test.ts after the Gemini CLI merge.

Validation

• node --import tsx/esm --test tests/integration/chat-pipeline.test.ts tests/integration/combo-routing-e2e.test.ts tests/unit/combo-routes-composite-tiers.test.ts tests/unit/autocombo-unification.test.ts tests/unit/combo-routing-engine.test.ts tests/unit/combo-strategies.test.ts tests/unit/json-migration-combos.test.ts tests/unit/settings-schema-routing-strategies.test.ts
• npm run typecheck:core
• npx eslint 'src/app/(dashboard)/dashboard/settings/components/ComboDefaultsTab.tsx' 'open-sse/services/combo.ts' 'src/lib/db/jsonMigration.ts' 'src/shared/constants/routingStrategies.ts'
• git diff --check

Known limitations / follow-ups

• p2c is intentionally a minimal power-of-two ordering strategy, not a full health-aware router. It currently scores by model-level metrics, not per target/connection.
• autoCombo persistence/adaptation remains a separate follow-up; this PR does not turn auto into a fully persistent adaptive routing system.
• combo.ts remains monolithic. Splitting strategies into separate modules would be useful, but is intentionally left out to keep this PR focused.
• context-optimized still depends on available capability metadata. When no context data is known, it now preserves order rather than guessing.
• Existing UUID-named combos created by the old test are not deleted by this PR; users can remove any already-created junk combos manually from the UI.

[gemini-code-assist]

Code Review

This pull request centralizes routing strategy definitions and validation logic across the MCP server, services, and UI. It introduces a normalization utility for strategy names and implements two new strategies: 'fill-first' and 'p2c' (Power-of-Two-Choices). Review feedback highlights a bug in context limit retrieval, a logic error in the P2C target selection that could inadvertently remove duplicate targets, and a potential breaking change regarding allowed fallback strategies in global settings.

[gemini-code-assist]

The call to getModelContextLimit here passes the full model string (target.modelStr), but the usage in sortModelsByContextSize (line 637) indicates that the function expects separate provider and model arguments. This inconsistency will likely cause hasKnownContext to always be false if the function doesn't handle full strings, effectively disabling the context-optimized strategy's reordering logic.

Suggested change

	const hasKnownContext = targets.some((target) => getModelContextLimit(target.modelStr) != null);
	const hasKnownContext = targets.some((target) => {
	const parsed = parseModel(target.modelStr);
	return getModelContextLimit(parsed.provider || parsed.providerAlias || "unknown", parsed.model || target.modelStr) != null;
	});

[dhaern]

@gemini-code-assist fixed in 5c26398. context-optimized now uses the same parsed provider/model lookup as sortModelsByContextSize, so the known-context guard no longer disables reordering for normal provider/model strings.

[gemini-code-assist]

Thank you for the update. Since the context-optimized strategy now correctly utilizes the shared normalizeRoutingStrategy and the parsed provider/model lookup, the logic is consistent with sortModelsByContextSize and the guard is no longer necessary. I'll mark this thread as resolved.

[gemini-code-assist]

Using filter on executionKey to remove the selected target assumes that executionKey is unique within the targets array. If a combo is configured with duplicate step IDs (which is permitted by the schema), this logic will inadvertently drop all instances of the selected target except for the one placed at the front. It is safer to use the index to remove only the specific sampled instance.

Suggested change

	const selected = getP2CTargetScore(second, metrics) > getP2CTargetScore(first, metrics) ? second : first;
	return [selected, ...targets.filter((target) => target.executionKey !== selected.executionKey)];
	const selectedIndex = getP2CTargetScore(second, metrics) > getP2CTargetScore(first, metrics) ? secondIndex : firstIndex;
	return [targets[selectedIndex], ...targets.filter((_, i) => i !== selectedIndex)];

[dhaern]

@gemini-code-assist fixed in 5c26398. p2c now keeps the selected sampled target by index and removes only that sampled index from the tail, so duplicate execution keys or duplicate targets are preserved correctly.

[gemini-code-assist]

Thank you for the update, @dhaern. Using the index to preserve the specific sampled instance is indeed the correct approach to handle potential duplicate execution keys. The fix in 5c26398 looks robust.

[gemini-code-assist]

Excluding priority and weighted from ACCOUNT_FALLBACK_STRATEGY_VALUES while keeping them in ROUTING_STRATEGY_VALUES creates an inconsistency in the contract. Since fill-first is behaviorally identical to priority in this context, having both in the UI but only one allowed in global settings may be confusing. Additionally, this change might break existing global settings if they were previously set to priority or weighted and are subsequently validated against the updated schema.

[dhaern]

@gemini-code-assist fixed in 5c26398. I kept priority and weighted accepted for the global fallback setting to preserve compatibility with existing settings, while still keeping truly combo-only values like auto, lkgp, and context-optimized out of account fallback validation.

[gemini-code-assist]

Thank you for the clarification, @dhaern. That approach makes sense to maintain backward compatibility for global settings while restricting the newer, combo-specific strategies. The implementation in src/shared/constants/routingStrategies.ts and the corresponding validation updates look correct for this requirement.

[dhaern]

@gemini-code-assist I pushed 5c26398 with the review fixes and revalidated the combo-focused suite (131/131), typecheck, eslint, and diff check.

Regarding #1891: this PR is complementary to that provider breaker profile fix. #1891 passes the runtime provider profile into recordProviderFailure; this PR aligns combo strategy contracts and runtime ordering. The only overlapping test cleanup is the stale chat-pipeline.test.ts import, and I did not force extra churn around it because #1891 is still open and the import is still used on this base. If #1891 lands first, this PR can be trivially rebased and that duplicate diff will disappear naturally.

[gemini-code-assist]

Thanks for the update and for revalidating the test suite. Your approach to handling the potential dependency on #1891 by keeping the import in chat-pipeline.test.ts is sensible and avoids unnecessary churn. The alignment of strategy contracts and the addition of fill-first and p2c look solid. I have no further concerns.

[diegosouzapw]

Thanks @dhaern for this comprehensive combo strategy audit! 🎉 The centralized strategy constants, fill-first/p2c implementations, and test isolation fixes are exactly what we needed. Merged into release/v3.7.9 — it will be part of the next release. Outstanding work!