fix(ci): exclude non-text-input models from review sidecar catalog

CHANGELOG.md

@@ -5,6 +5,11 @@ this file. The format follows Keep a Changelog, and versioned releases follow
 Semantic Versioning where the repository publishes a release.
 
 ## [Unreleased]
+- Exclude models with declared non-text input requirements from the Contextual
+  Orchestrator review catalog before cost, privacy, and account selection.
+  Plain-text Chat Completions, Responses, and structured-output review paths
+  remain eligible, while the existing multi-hour per-model review budgets are
+  unchanged (#1415).
 - Fail closed when the first top-level Noema JSON candidate is malformed,
   preventing a later approval object from overriding malformed preface data;
   multiple-object output remains supported when its first object is valid.

scripts/ci/contextual_orchestrator_review_launcher.py

@@ -105,6 +105,29 @@ def _has_text_output(model: object) -> bool:
     return not modalities or "text" in {str(modality).casefold() for modality in modalities}
 
 
+def _is_text_review_candidate(
+    model: object,
+    *,
+    is_general_chat_agent_model_id: Any,
+    requires_non_text_input: Any,
+) -> bool:
+    """Return whether discovery evidence permits plain-text review traffic.
+
+    Unknown input modality remains eligible because absence of catalog evidence
+    is not evidence of a non-text requirement. Any declared non-text input is
+    excluded conservatively before cost, privacy, or account selection can
+    admit the route. Chat and structured-output capability metadata otherwise
+    remain untouched for the gateway's Responses and Chat Completions paths.
+    """
+    model_id = getattr(model, "model_id", "")
+    input_modalities = getattr(model, "input_modalities", ()) or ()
+    return bool(
+        is_general_chat_agent_model_id(model_id)
+        and _has_text_output(model)
+        and not requires_non_text_input(input_modalities)
+    )
+
+
 _DISCOVERY_DIAGNOSTICS_COMPLETE_SENTINEL = "discovery_diagnostics_complete"
 
 
@@ -796,7 +819,10 @@ def main(argv: list[str] | None = None) -> int:
     args = parser.parse_args(argv)
 
     from contextual_orchestrator.credentials import get_credential
-    from contextual_orchestrator.chat_capability import is_general_chat_agent_model_id
+    from contextual_orchestrator.chat_capability import (
+        is_general_chat_agent_model_id,
+        requires_non_text_input,
+    )
     from contextual_orchestrator.model_discovery import discover_all_models, free_discovered_models
     from contextual_orchestrator.orchestrator import ModelClient, TaskOrchestrator, load_agents
     from contextual_orchestrator.review_gateway import (
@@ -834,8 +860,11 @@ def main(argv: list[str] | None = None) -> int:
     free_route_identities = frozenset(_route_identity(model) for model in free_models)
     selected_models = []
     for model in routable_discovered:
-        model_id = getattr(model, "model_id", "")
-        if not is_general_chat_agent_model_id(model_id) or not _has_text_output(model):
+        if not _is_text_review_candidate(
+            model,
+            is_general_chat_agent_model_id=is_general_chat_agent_model_id,
+            requires_non_text_input=requires_non_text_input,
+        ):
             continue
         if args.pool == "free" and _route_identity(model) not in free_route_identities:
             continue

tests/test_contextual_orchestrator_review_sidecar_contract.py

@@ -279,16 +279,19 @@ def test_launcher_registers_secrets_into_the_kv_once() -> None:
 def test_launcher_uses_orchestrator_discovery_and_governed_pools() -> None:
     """Discovery, price evidence, and serving come from the vendored library."""
     text = _read(LAUNCHER)
-    assert "from contextual_orchestrator.chat_capability import is_general_chat_agent_model_id" in text
+    assert "from contextual_orchestrator.chat_capability import (" in text
+    assert "is_general_chat_agent_model_id," in text
+    assert "requires_non_text_input," in text
     assert "from contextual_orchestrator.model_discovery import discover_all_models, free_discovered_models" in text
     assert "routable_discovered = _routable_discovered_models(discovered)" in text
     assert "free_discovered_models(routable_discovered)" in text
     assert 'getattr(model, "evidence_only", False)' in text
     assert 'getattr(model, "output_modalities", None)' in text
     assert 'isinstance(modalities, str)' in text
     assert '"text" in {str(modality).casefold() for modality in modalities}' in text
-    assert "not _has_text_output(model)" in text
-    assert 'model_id = getattr(model, "model_id", "")' in text
+    assert "_is_text_review_candidate(" in text
+    assert 'input_modalities = getattr(model, "input_modalities", ()) or ()' in text
+    assert "not requires_non_text_input(input_modalities)" in text
 
     launcher = runpy.run_path(str(LAUNCHER))
     has_text_output = launcher["_has_text_output"]
@@ -323,6 +326,61 @@ def test_launcher_uses_orchestrator_discovery_and_governed_pools() -> None:
     assert "from scripts.ci import zdr_policy" in text
 
 
+def test_launcher_admits_only_plain_text_review_candidates() -> None:
+    """Review discovery keeps text protocols while excluding multimodal rows."""
+    launcher = runpy.run_path(str(LAUNCHER))
+    is_candidate = launcher["_is_text_review_candidate"]
+
+    def general_chat(model_id: str) -> bool:
+        return model_id.startswith("chat-")
+
+    def needs_non_text(modalities: object) -> bool:
+        return any(str(value).casefold() != "text" for value in modalities)
+
+    compatible = SimpleNamespace(
+        model_id="chat-structured",
+        input_modalities=("text",),
+        output_modalities=("text",),
+        capabilities=("chat", "response_format"),
+    )
+    unknown_input = SimpleNamespace(
+        model_id="chat-undocumented",
+        input_modalities=(),
+        output_modalities=("text",),
+    )
+    multimodal = SimpleNamespace(
+        model_id="chat-multimodal",
+        input_modalities=("text", "image"),
+        output_modalities=("text",),
+    )
+    media_output = SimpleNamespace(
+        model_id="chat-media-output",
+        input_modalities=("text",),
+        output_modalities=("image",),
+    )
+
+    assert is_candidate(
+        compatible,
+        is_general_chat_agent_model_id=general_chat,
+        requires_non_text_input=needs_non_text,
+    )
+    assert is_candidate(
+        unknown_input,
+        is_general_chat_agent_model_id=general_chat,
+        requires_non_text_input=needs_non_text,
+    )
+    assert not is_candidate(
+        multimodal,
+        is_general_chat_agent_model_id=general_chat,
+        requires_non_text_input=needs_non_text,
+    )
+    assert not is_candidate(
+        media_output,
+        is_general_chat_agent_model_id=general_chat,
+        requires_non_text_input=needs_non_text,
+    )
+
+
 def test_launcher_wraps_catalog_for_vendored_load_agents() -> None:
     """Persist the catalog envelope expected by the pinned orchestrator loader."""
     text = _read(LAUNCHER)

tests/test_pr_review_fix_scheduler.py

@@ -1142,6 +1142,16 @@ def test_fix_inspect_skip_wait_and_error_paths(monkeypatch):
     """Inspect and queue logic report skip, wait, dispatch-limit, and errors."""
     args = fix.parse_args(["--repo", "owner/repo", "--base-branch", "main"])
     assert fix.inspect_pr("owner/repo", make_pr(isDraft=True), args) == ("skip", ("draft PR",))
+    assert fix.inspect_pr(
+        "owner/repo",
+        _approved_dirty_pr(isDraft=True),
+        args,
+    ) == ("skip", ("draft PR",))
+    monkeypatch.setattr(fix, "needs_conflict_resolution", lambda pr, **kwargs: (False, ()))
+    assert fix.inspect_pr("owner/repo", _approved_dirty_pr(), args) == (
+        "skip",
+        ("merge conflict is not authorized for repair",),
+    )
     assert fix.inspect_pr("owner/repo", make_pr(baseRefName="develop"), args)[1][0].startswith("base branch")
     wildcard_args = fix.parse_args(["--repo", "owner/repo", "--base-branch", "*"])
     monkeypatch.setattr(fix, "needs_autofix", lambda pr: (False, ()))

tests/test_pr_review_fix_scheduler_rest_workflow_identity.py

@@ -10,6 +10,45 @@
 from scripts.ci import pr_review_merge_scheduler as merge
 
 
+def test_workflow_name_rest_fallback_paginates_and_skips_incomplete_rows(
+    monkeypatch: Any,
+) -> None:
+    """Workflow identity pagination ignores rows without a complete binding."""
+    calls: list[str] = []
+
+    def fake_api(path: str) -> Any:
+        calls.append(path)
+        if path.endswith("&page=1"):
+            return {
+                "workflow_runs": [
+                    {"check_suite_id": 7, "name": "Required OpenCode Review"},
+                    *({"check_suite_id": None, "name": ""} for _ in range(99)),
+                ]
+            }
+        return {"workflow_runs": []}
+
+    monkeypatch.setattr(merge, "gh_api_json", fake_api)
+
+    assert merge.fetch_workflow_names_by_check_suite_rest(
+        "owner/repo", "a" * 40
+    ) == {7: "Required OpenCode Review"}
+    assert len(calls) == 2
+    assert "page=2" in calls[-1]
+
+
+def test_workflow_name_rest_fallback_propagates_unexpected_errors(
+    monkeypatch: Any,
+) -> None:
+    """Only permission denial degrades to unknown workflow identity."""
+    def fail(_path: str) -> Any:
+        raise RuntimeError("unexpected transport failure")
+
+    monkeypatch.setattr(merge, "gh_api_json", fail)
+
+    with pytest.raises(RuntimeError, match="unexpected transport failure"):
+        merge.fetch_workflow_names_by_check_suite_rest("owner/repo", "b" * 40)
+
+
 def test_rest_fallback_preserves_renamed_opencode_workflow_identity(
     monkeypatch: Any,
 ) -> None: