fix(routing): retain displaced primary in colliding fallback chains

docs/docs/usage-guide/changing_a_model.md

@@ -1006,8 +1006,9 @@ configured output limit.
 
 ## Routing small pull requests to a cheaper model
 
-`config.model` handles every pull request, however small. With model routing enabled, a pull request under a
-configured size goes to a cheaper model instead, and `config.fallback_models` still apply after it:
+Without routing, regular-model calls use `config.model`, however small the pull request. With model routing
+enabled, a pull request under a configured size goes to a cheaper model instead.
+Configured fallbacks follow the selected primary if it fails:
 
 ```toml
 [model_routing]
@@ -1028,6 +1029,25 @@ the call. A pull request that fits no rule uses `config.model`. Size is measured
 (`max_hunks`) and changed files (`max_files`) left after the `[ignore]` rules. Every git provider reports both,
 and neither depends on a model's tokenizer, so a threshold means the same thing whichever model it selects.
 
+The retry chain starts as `[config.model] + config.fallback_models`, with a deployment paired to each model.
+A matched rule replaces the first pair. If the routed `(model, deployment)` pair also appears among the
+configured fallbacks, the effective chain removes duplicate pairs while preserving their order and adds
+the displaced configured primary pair at the end, unless it is already present. This avoids retrying the
+same endpoint while retaining the configured primary in this collision case. With
+`config.model = "gpt-5.6"`, no deployment configured, and the second rule above:
+
+| `config.fallback_models` | retry order if earlier attempts fail |
+| --- | --- |
+| `["gpt-5.6-terra"]` (shipped default) | `gpt-5.6-terra`, `gpt-5.6` |
+| `["gpt-5.6-terra", "other-model"]` | `gpt-5.6-terra`, `other-model`, `gpt-5.6` |
+
+The first successful attempt returns immediately; later entries are attempted only after fallback-eligible
+failures. An empty fallback list adds no backup, and a noncolliding route still replaces the primary without
+adding it back. The same model with distinct deployment IDs is treated as distinct endpoints. `/improve`
+uses this effective chain to recover failed chunks without rerunning successful chunks. Its defensive
+uniqueness checks remain: repeated complete pairs in unchanged, noncolliding or unrouted chains can still
+disable chunk recovery; those chains are not deduplicated by this routing-specific repair.
+
 Routing applies only to calls that ask for the regular model: `/review`, `/improve`, `/generate_labels` and
 `/add_docs`. Tools that already use `config.model_weak` (`/describe`, `/ask`, `/update_changelog`) are left
 alone, and a dedicated `config.model_reasoning` is still used for self-reflection. With `config.output_run_details`

docs/docs/usage-guide/configuration_reference.md

@@ -480,7 +480,7 @@ _This section only documents commented-out examples; see the [TOML source](https
 | Key | Default | Description |
 | --- | --- | --- |
 | `enable` | false | Rules are checked in order; the first one whose limits the pull request fits selects the primary model for the call, and config.fallback_models still apply after it. A pull request that fits no rule uses config.model. Size is measured after the [ignore] rules by diff hunks (max_hunks) and changed files (max_files), which every git provider reports and which do not depend on any model's tokenizer. Only calls that ask for the regular model are routed (/review, /improve, /generate_labels, /add_docs); tools that use model_weak are left alone. With Azure (openai.deployment_id set) a rule also needs its own deployment_id, or it is skipped. |
-| `rules` | [] | e.g. [{ max_hunks = 3, model = "gpt-5.6-luna" }, { max_hunks = 15, max_files = 6, model = "gpt-5.6-terra" }] |
+| `rules` | [] | e.g. [{ max_hunks = 3, model = "gpt-5.6-luna" }, { max_hunks = 15, max_files = 6, model = "gpt-5.6-terra" }]. A matched rule replaces the primary (model, deployment) pair. If it matches a configured fallback pair, deduplicate the effective chain and append the displaced primary unless already present. Otherwise the configured fallbacks follow unchanged. Later pairs are attempted only after fallback-eligible failures; distinct deployments remain separate. /improve uses this effective chain for chunk recovery. |
 
 
 ## `[otel]` {#otel}

pr_agent/algo/pr_processing.py

@@ -831,13 +831,19 @@ async def retry_with_fallback_models(f: Callable, model_type: ModelType = ModelT
                                      git_provider: GitProvider | None = None):
     all_models = _get_all_models(model_type)
     all_deployments = _get_all_deployments(all_models)
-    routed = route_primary_model(model_type, git_provider)
-    if routed:
-        # A cheaper primary for a small pull request; config.fallback_models still follow it.
-        all_models[0], all_deployments[0] = routed
     # Ignore surplus deployment entries when fewer fallback models are configured; this matches
     # the existing retry loop, which stops as soon as the first successful model returns.
-    effective_chain = tuple(zip(all_models, all_deployments[:len(all_models)], strict=True))
+    configured_chain = tuple(zip(all_models, all_deployments[:len(all_models)], strict=True))
+    effective_chain = configured_chain
+    routed = route_primary_model(model_type, git_provider)
+    if routed:
+        effective_chain = (routed, *configured_chain[1:])
+        # Routing treats empty deployment IDs as absent; compare equivalent endpoint pairs.
+        normalized_chain = tuple((model, deployment or None) for model, deployment in configured_chain)
+        if routed in normalized_chain[1:]:
+            # Retain the displaced primary, appending it only when it is not already present.
+            # Keep other fallbacks in order and distinguish deployments of the same model.
+            effective_chain = tuple(dict.fromkeys((routed, *normalized_chain[1:], normalized_chain[0])))
     original_deployment_id = get_settings().get("openai.deployment_id", None)
     context_token = _effective_fallback_chain.set(effective_chain)
     attempt_errors = []
@@ -859,9 +865,9 @@ async def retry_with_fallback_models(f: Callable, model_type: ModelType = ModelT
                     f"Failed to generate prediction with {model}",
                     artifact={"error": e},
                 )
-                if i == len(all_models) - 1:  # If it's the last iteration
+                if i == len(effective_chain) - 1:  # If it's the last iteration
                     raise Exception(
-                        f"Failed to generate prediction with any model of {all_models}. "
+                        f"Failed to generate prediction with any model of {[model for model, _ in effective_chain]}. "
                         f"Attempts: {'; '.join(attempt_errors)}"
                     ) from e
             else:

pr_agent/settings/configuration.toml

@@ -540,7 +540,7 @@ max_tokens = 0 # hard cap on completion tokens for the request; 0 = unset
 # are routed (/review, /improve, /generate_labels, /add_docs); tools that use model_weak are left alone.
 # With Azure (openai.deployment_id set) a rule also needs its own deployment_id, or it is skipped.
 enable = false
-rules = [] # e.g. [{ max_hunks = 3, model = "gpt-5.6-luna" }, { max_hunks = 15, max_files = 6, model = "gpt-5.6-terra" }]
+rules = [] # e.g. [{ max_hunks = 3, model = "gpt-5.6-luna" }, { max_hunks = 15, max_files = 6, model = "gpt-5.6-terra" }]. A matched rule replaces the primary (model, deployment) pair. If it matches a configured fallback pair, deduplicate the effective chain and append the displaced primary unless already present. Otherwise the configured fallbacks follow unchanged. Later pairs are attempted only after fallback-eligible failures; distinct deployments remain separate. /improve uses this effective chain for chunk recovery.
 
 [otel]
 # Keys map to settings.get("OTEL.*") via section-key dot notation.

tests/unittest/test_model_routing.py

@@ -7,12 +7,16 @@
 import pytest
 
 from pr_agent.algo.model_routing import count_hunks, route_primary_model
-from pr_agent.algo.pr_processing import FallbackEligibleError, retry_with_fallback_models
+from pr_agent.algo.pr_processing import (
+    FallbackEligibleError,
+    get_effective_fallback_chain,
+    retry_with_fallback_models,
+)
 from pr_agent.algo.run_details import get_run_details, init_run_details
 from pr_agent.algo.types import FilePatchInfo
 from pr_agent.algo.utils import ModelType
 from pr_agent.config_loader import get_settings
-from tests.unittest._settings_helpers import restore_settings, snapshot_settings
+from tests.unittest._settings_helpers import SENTINEL, restore_settings, snapshot_settings
 
 _TRACKED_KEYS = (
     "config.model",
@@ -107,6 +111,147 @@ def test_configured_fallbacks_follow_the_routed_primary(self, settings):
         calls = _models_tried(_pr(num_files=1, hunks_per_file=1), fail=("tiny-model",))
         assert calls == ["tiny-model", "fallback-1"]
 
+    def test_routed_model_repeating_in_fallbacks_keeps_distinct_fallbacks_first(self, settings):
+        settings.set("config.fallback_models", ["tiny-model", "fallback-1"])
+        calls = _models_tried(_pr(num_files=1, hunks_per_file=1), fail=("tiny-model",))
+        assert calls == ["tiny-model", "fallback-1"]
+
+    def test_routed_collision_recovers_with_the_displaced_primary(self, settings):
+        settings.set("config.fallback_models", ["tiny-model"])
+        init_run_details()
+        calls = _models_tried(_pr(num_files=1, hunks_per_file=1), fail=("tiny-model",))
+        assert calls == ["tiny-model", "primary-model"]
+        assert get_run_details().model_used == "primary-model"
+        assert get_run_details().fallback_used is True
+
+    def test_successful_routed_collision_does_not_attempt_fallbacks(self, settings):
+        settings.set("config.fallback_models", ["tiny-model"])
+        init_run_details()
+        assert _models_tried(_pr(num_files=1, hunks_per_file=1)) == ["tiny-model"]
+        assert get_run_details().model_used == "tiny-model"
+        assert get_run_details().fallback_used is False
+
+    @pytest.mark.parametrize("deployment", [SENTINEL, None, ""])
+    @pytest.mark.parametrize("fallback_deployments", [SENTINEL, None, [], "", [None], [""], " "])
+    def test_empty_deployments_match_routed_endpoints(self, settings, deployment, fallback_deployments):
+        restore_settings({"openai.deployment_id": deployment,
+                          "openai.fallback_deployments": fallback_deployments})
+        settings.set("config.fallback_models", ["tiny-model"])
+        original_deployment = settings.get("openai.deployment_id")
+        previous_chain = get_effective_fallback_chain()
+        expected = (("tiny-model", None), ("primary-model", None))
+        observed = []
+
+        async def fake_f(model):
+            observed.append((model, settings.get("openai.deployment_id")))
+            assert get_effective_fallback_chain() == expected
+            raise FallbackEligibleError("endpoint failed")
+
+        with pytest.raises(Exception, match="Failed to generate prediction with any model"):
+            asyncio.run(retry_with_fallback_models(fake_f, git_provider=_pr(1, 1)))
+
+        assert observed == list(expected)
+        assert settings.get("openai.deployment_id") == original_deployment
+        assert get_effective_fallback_chain() == previous_chain
+
+    @pytest.mark.parametrize("outcome", ["routed-success", "primary-success", "exhausted", "fatal"])
+    @pytest.mark.parametrize("fallback_deployments", [["", None, ""], ",,"])
+    def test_mixed_empty_deployments_preserve_primary_and_restore_context(self, settings, outcome,
+                                                                        fallback_deployments):
+        settings.set("openai.deployment_id", "")
+        settings.set("config.fallback_models", ["tiny-model", "primary-model", "tiny-model"])
+        settings.set("openai.fallback_deployments", fallback_deployments)
+        expected = (("tiny-model", None), ("primary-model", None))
+        previous_chain = get_effective_fallback_chain()
+        observed = []
+
+        async def fake_f(model):
+            observed.append((model, settings.get("openai.deployment_id")))
+            assert get_effective_fallback_chain() == expected
+            if outcome == "fatal":
+                raise ValueError("not eligible for fallback")
+            if outcome == "exhausted" or (outcome == "primary-success" and model == "tiny-model"):
+                raise FallbackEligibleError("endpoint failed")
+            return model
+
+        if outcome in ("fatal", "exhausted"):
+            error = ValueError if outcome == "fatal" else Exception
+            with pytest.raises(error):
+                asyncio.run(retry_with_fallback_models(fake_f, git_provider=_pr(1, 1)))
+        else:
+            result = asyncio.run(retry_with_fallback_models(fake_f, git_provider=_pr(1, 1)))
+            assert result == ("tiny-model" if outcome == "routed-success" else "primary-model")
+
+        assert observed == list(expected[:1] if outcome in ("fatal", "routed-success") else expected)
+        assert settings.get("openai.deployment_id") == ""
+        assert get_effective_fallback_chain() == previous_chain
+
+    @pytest.mark.parametrize("routed", [True, False])
+    @pytest.mark.parametrize("deployment", [None, "", " "])
+    @pytest.mark.parametrize("fallbacks", [[], ["fallback-1"]])
+    def test_noncolliding_chains_keep_raw_deployments(self, settings, routed, deployment, fallbacks):
+        settings.set("openai.deployment_id", "")
+        settings.set("config.fallback_models", fallbacks)
+        settings.set("openai.fallback_deployments", [deployment])
+        settings.set("model_routing.enable", routed)
+        expected = (("tiny-model", None) if routed else ("primary-model", ""),)
+        if fallbacks:
+            expected += (("fallback-1", deployment),)
+        observed = []
+
+        async def fake_f(model):
+            observed.append((model, settings.get("openai.deployment_id")))
+            assert get_effective_fallback_chain() == expected
+            raise FallbackEligibleError("endpoint failed")
+
+        with pytest.raises(Exception, match="Failed to generate prediction with any model"):
+            asyncio.run(retry_with_fallback_models(fake_f, git_provider=_pr(1, 1)))
+
+        assert observed == list(expected)
+        assert settings.get("openai.deployment_id") == ""
+
+    @pytest.mark.parametrize("fallbacks, routed, expected", [
+        ([("tiny-model", "tiny-deployment")], ("tiny-model", "tiny-deployment"),
+         [("tiny-model", "tiny-deployment"), ("primary-model", "primary-deployment")]),
+        ([("fallback-1", "other"), ("tiny-model", "tiny-deployment"), ("fallback-1", "other")],
+         ("tiny-model", "tiny-deployment"),
+         [("tiny-model", "tiny-deployment"), ("fallback-1", "other"), ("primary-model", "primary-deployment")]),
+        ([("tiny-model", "tiny-deployment"), ("primary-model", "primary-deployment"), ("fallback-1", "other")],
+         ("tiny-model", "tiny-deployment"),
+         [("tiny-model", "tiny-deployment"), ("primary-model", "primary-deployment"), ("fallback-1", "other")]),
+        ([("primary-model", "primary-deployment")], ("primary-model", "primary-deployment"),
+         [("primary-model", "primary-deployment")]),
+        ([("tiny-model", "other-deployment")], ("tiny-model", "tiny-deployment"),
+         [("tiny-model", "tiny-deployment"), ("tiny-model", "other-deployment")]),
+        ([], ("tiny-model", "tiny-deployment"), [("tiny-model", "tiny-deployment")]),
+        ([("fallback-1", "other"), ("fallback-1", "other")], ("tiny-model", "tiny-deployment"),
+         [("tiny-model", "tiny-deployment"), ("fallback-1", "other"), ("fallback-1", "other")]),
+    ])
+    def test_routed_chain_exhaustion_preserves_endpoint_order_and_restores_context(self, settings,
+                                                                                  fallbacks, routed, expected):
+        settings.set("openai.deployment_id", "primary-deployment")
+        settings.set("config.fallback_models", [model for model, _ in fallbacks])
+        # Surplus deployments are ignored, not attached to the restored primary.
+        settings.set("openai.fallback_deployments", [deployment for _, deployment in fallbacks] + ["surplus"])
+        settings.set("model_routing.rules", [
+            {"max_hunks": 3, "model": routed[0], "deployment_id": routed[1]},
+        ])
+        observed = []
+        previous_chain = get_effective_fallback_chain()
+
+        async def fake_f(model):
+            observed.append((model, get_settings().get("openai.deployment_id")))
+            assert get_effective_fallback_chain() == tuple(expected)
+            raise FallbackEligibleError("endpoint failed")
+
+        with pytest.raises(Exception, match="Failed to generate prediction with any model") as error:
+            asyncio.run(retry_with_fallback_models(fake_f, git_provider=_pr(num_files=1, hunks_per_file=1)))
+
+        assert observed == expected
+        assert f"any model of {[model for model, _ in expected]}" in str(error.value)
+        assert get_settings().get("openai.deployment_id") == "primary-deployment"
+        assert get_effective_fallback_chain() == previous_chain
+
     def test_weak_tier_is_not_routed(self, settings):
         provider = _pr(num_files=1, hunks_per_file=1)
         assert _models_tried(provider, model_type=ModelType.WEAK) == ["weak-model"]

tests/unittest/test_suggestion_chunk_recovery.py

@@ -503,6 +503,16 @@ async def test_recovered_run_details_report_primary_model_and_sticky_fallback(co
     assert details.num_ai_calls == 0
 
 
+@pytest.mark.parametrize("fallback_deployment", ["primary", "secondary"])
+def test_recovery_chain_distinguishes_deployments_for_the_same_model(configured, monkeypatch, fallback_deployment):
+    chain = [("gpt-4o", "primary"), ("gpt-4o", fallback_deployment)]
+    monkeypatch.setattr(module, "get_effective_fallback_chain", lambda: chain)
+    tool = PRCodeSuggestions.__new__(PRCodeSuggestions)
+
+    expected = None if fallback_deployment == "primary" else (chain, 1)
+    assert tool._recovery_chain("gpt-4o", get_settings()) == expected
+
+
 async def test_ambiguous_model_deployment_chain_keeps_partial_result(configured, monkeypatch):
     get_settings().set("config.fallback_models", ["gpt-4o", "gpt-4.1"])
     get_settings().set("openai.fallback_deployments", ["primary", "last"])
@@ -574,6 +584,32 @@ async def test_routed_primary_recovers_failed_slots_with_configured_fallback(con
     tool.git_provider.get_diff_files.assert_called_once_with()
 
 
+@pytest.mark.parametrize("parallel", [True, False])
+@pytest.mark.parametrize("deployment, fallback_deployments", [(None, []), ("", []), (None, [""]), ("", [None])])
+async def test_routed_collision_recovers_failed_chunks_with_displaced_primary(configured, monkeypatch, parallel,
+                                                                            deployment, fallback_deployments):
+    get_settings().set("pr_code_suggestions.parallel_calls", parallel)
+    get_settings().set("config.model", "gpt-5.6")
+    get_settings().set("config.fallback_models", ["gpt-5.6-terra"])
+    get_settings().set("openai.deployment_id", deployment)
+    get_settings().set("openai.fallback_deployments", fallback_deployments)
+    get_settings().set("model_routing.enable", True)
+    get_settings().set("model_routing.rules", [{"model": "gpt-5.6-terra", "max_files": 10}])
+    tool, calls = make_tool(monkeypatch, {("gpt-5.6-terra", "b"): RuntimeError("failure")})
+    tool.git_provider.get_diff_files.return_value = [FilePatchInfo("old()\n", "old()\n", "", c + ".py") for c in "abc"]
+
+    result = await retry_with_fallback_models(tool.prepare_prediction_main, git_provider=tool.git_provider)
+
+    assert [s["relevant_file"] for s in result["code_suggestions"]] == ["a.py", "b.py", "c.py"]
+    assert [(model, chunk, dep) for model, chunk, dep, _ in calls] == [
+        ("gpt-5.6-terra", "a", None), ("gpt-5.6-terra", "b", None), ("gpt-5.6-terra", "c", None),
+        ("gpt-5.6", "b", None),
+    ]
+    assert tool.failed_chunk_count == 0
+    assert get_settings().get("openai.deployment_id") == deployment
+    tool.git_provider.get_diff_files.assert_called_once_with()
+
+
 async def test_routing_is_not_repeated_during_recovery(configured, monkeypatch):
     get_settings().set("config.fallback_models", ["gpt-4.1"])
     get_settings().set("openai.fallback_deployments", ["last"])