diff --git a/src/conclave/modes.py b/src/conclave/modes.py index 9f0541b..f8b0d75 100644 --- a/src/conclave/modes.py +++ b/src/conclave/modes.py @@ -161,12 +161,26 @@ async def run_adversarial( ) -> CouncilResult: """Run a propose -> refute -> verdict pass and return a :class:`CouncilResult`. + The proposer is a single point of failure, so the run is layered to survive a + bad one: + + 1. **Proposer fallback.** Members are tried as proposer in council order, + starting with the requested one. A member that returns an unusable answer + (``ModelAnswer.error`` set / no text) is recorded and the next member is + tried, until one produces a usable proposal. + 2. **Graceful degrade.** If no member can propose, the run does *not* abort + with "no verdict". It degrades to a plain synthesize over the surviving + members and surfaces an actionable warning on ``CouncilResult.synthesis_error`` + (mirrored to the adversarial verdict so consumers reading either field see + why the adversarial flow was skipped). + Args: council: The :class:`Council` providing fan-out, config, and judge. prompt: The user prompt. proposer: Friendly name of the proposing member. Defaults to the first requested council member. If the named proposer has no key, the run - falls back to the first available member. + falls back to the first available member; if its answer is unusable, + the run falls back to the next available member as proposer. Returns: A :class:`CouncilResult` whose ``adversarial`` field carries the proposal, @@ -182,33 +196,48 @@ async def run_adversarial( return result requested_proposer = proposer or council.requested_models[0] - p_name, p_model_id = _pick_proposer(members, requested_proposer) - if p_name != requested_proposer: + order = _proposer_order(members, requested_proposer) + + # Step 1: find a proposer that produces a usable answer. Each attempt is a + # single-member fan-out reusing the same partial-failure primitive. Failed + # attempts are recorded so the judge/degrade path can explain what was tried. + base = [{"role": "user", "content": prompt}] + proposal: ModelAnswer | None = None + p_name = order[0][0] + failed_proposers: list[ModelAnswer] = [] + for cand_name, cand_model_id in order: + attempt = (await council.fan_out([(cand_name, cand_model_id)], lambda _n, _m: base))[0] + result.answers.append(attempt) + if attempt.ok: + proposal = attempt + p_name = cand_name + break + failed_proposers.append(attempt) logger.warning( - "proposer '%s' unavailable; falling back to '%s'", - requested_proposer, - p_name, + "proposer '%s' produced no usable answer (%s); trying next member", + cand_name, + attempt.error, ) - # Step 1: the proposal (single-member fan-out reuses the same primitive). - base = [{"role": "user", "content": prompt}] - proposal = (await council.fan_out([(p_name, p_model_id)], lambda _n, _m: base))[0] + # Step 2: no member could propose -> degrade to synthesize over the survivors + # instead of aborting the whole run with "no verdict produced". + if proposal is None: + await _degrade_to_synthesize(council, result, failed_proposers) + return result adv = AdversarialResult(proposer=p_name, proposal=proposal) - result.answers.append(proposal) - # Step 2: critics refute the proposal. If the proposal itself failed, there is - # nothing to refute -- record the failure and skip to the (empty) judge. - critics = [(n, m) for (n, m) in members if n != p_name] - if proposal.ok and critics: + # Step 3: critics refute the proposal. Every other available member critiques; + # any failed-proposer attempts are excluded so a member is never both. + tried = {a.name for a in failed_proposers} | {p_name} + critics = [(n, m) for (n, m) in members if n not in tried] + if critics: adv.critiques = await council.fan_out( critics, _critic_messages_for(prompt, proposal.answer or "") ) result.answers.extend(adv.critiques) - elif not proposal.ok: - logger.warning("proposal failed (%s); critics skipped", proposal.error) - # Step 3: the judge weighs proposal vs critiques and issues a verdict. + # Step 4: the judge weighs proposal vs critiques and issues a verdict. await _adversarial_judge(council, prompt, adv) result.adversarial = adv result.synthesis = adv.verdict @@ -218,6 +247,48 @@ async def run_adversarial( return result +async def _degrade_to_synthesize( + council: Council, + result: CouncilResult, + failed_proposers: list[ModelAnswer], +) -> None: + """Fall back to plain synthesize when no member can produce a proposal. + + Every requested member was tried as proposer and none returned a usable + answer, so there is nothing to refute. Rather than emit "no verdict produced", + we run the standard synthesizer over whatever members *did* answer (here: + none succeeded, by definition of reaching this path) and always surface an + actionable warning on ``result.synthesis_error`` explaining the degrade. The + warning is mirrored into an :class:`AdversarialResult` so consumers reading + ``result.adversarial.verdict_error`` (e.g. the CLI) see it too. + """ + names = ", ".join(a.name for a in failed_proposers) or "(none)" + warning = ( + "adversarial degraded to synthesize: no council member produced a usable " + f"proposal (tried: {names}). Showing a consolidated answer over the " + "surviving members instead of a propose/refute/verdict result." + ) + logger.warning(warning) + + # Reuse the single synthesizer path. successful_answers is empty here (all + # proposer attempts failed), so _synthesize records its own no-answers error; + # we override synthesis_error afterwards so the degrade reason is what surfaces. + await council._synthesize(result) + result.synthesis_error = warning + result.synthesizer = council.synthesizer + result.synthesizer_model_id = council.config.resolve_model_id(council.synthesizer) + + # Mirror the degrade into the adversarial structure so the field-specific CLI + # renderer and library consumers reading result.adversarial both see it. The + # first failed attempt stands in as the (failed) proposal for shape parity. + if failed_proposers: + adv = AdversarialResult(proposer=failed_proposers[0].name, proposal=failed_proposers[0]) + adv.verdict_error = warning + adv.judge = council.synthesizer + adv.judge_model_id = council.config.resolve_model_id(council.synthesizer) + result.adversarial = adv + + def _critic_messages_for(prompt: str, proposal_text: str): """Build the per-critic message factory for the refutation step.""" @@ -230,12 +301,19 @@ def critic_messages(_name: str, _model_id: str) -> list[dict[str, str]]: return critic_messages -def _pick_proposer(members: list[tuple[str, str]], requested: str) -> tuple[str, str]: - """Return the requested proposer member, or the first available as fallback.""" - for member in members: - if member[0] == requested: - return member - return members[0] +def _proposer_order(members: list[tuple[str, str]], requested: str) -> list[tuple[str, str]]: + """Return members ordered for proposer selection: requested first, then rest. + + The requested proposer is tried first when it is available; every other + available member follows in council order so a failed proposer can fall back + to the next candidate. A requested name with no key is simply absent from + ``members`` (filtered upstream), so the council order leads. + """ + requested_member = next((m for m in members if m[0] == requested), None) + if requested_member is None: + return list(members) + rest = [m for m in members if m[0] != requested] + return [requested_member, *rest] async def _adversarial_judge(council: Council, prompt: str, adv: AdversarialResult) -> None: diff --git a/tests/test_adapters.py b/tests/test_adapters.py index d393e38..6a27391 100644 --- a/tests/test_adapters.py +++ b/tests/test_adapters.py @@ -270,6 +270,28 @@ def test_gemini_parse_malformed_raises(): adapter.parse_response(200, {"candidates": []}) +@pytest.mark.parametrize( + "payload", + [ + # candidate present but content.parts key missing (issue #9 KeyError shape). + {"candidates": [{"content": {}}]}, + # blocked/safety candidate with no content object at all. + {"candidates": [{"finishReason": "SAFETY"}]}, + ], +) +def test_gemini_parse_missing_content_parts_raises_provider_error(payload): + """A candidate missing content.parts is a typed ProviderError, never a KeyError. + + Issue #9: a malformed/blocked Gemini response (missing + ``candidates[0].content.parts``) must surface as a redact-safe ProviderError + so ``call_model`` can turn it into ``ModelAnswer.error`` rather than aborting + the run with a raw ``KeyError``. + """ + adapter = GeminiAdapter() + with pytest.raises(ProviderError, match="missing candidates"): + adapter.parse_response(200, payload) + + def test_gemini_parse_error_status_raises(): adapter = GeminiAdapter() payload = {"error": {"status": "PERMISSION_DENIED", "message": "no access"}} diff --git a/tests/test_modes.py b/tests/test_modes.py index 9b8a68c..ab88dae 100644 --- a/tests/test_modes.py +++ b/tests/test_modes.py @@ -355,11 +355,18 @@ def handler(model, messages, **kwargs): assert adv.verdict == "VERDICT DESPITE FAILURE" -async def test_adversarial_proposal_failure_skips_critics(monkeypatch, patch_call_model): - """If the proposal itself fails, critics are skipped and no verdict is made.""" +async def test_adversarial_all_proposers_fail_degrades_to_synthesize(monkeypatch, patch_call_model): + """If every member fails as proposer, the run degrades to synthesize (issue #9). + + No member can produce a usable proposal, so there is nothing to refute. The + run must NOT abort with "no verdict produced"; it degrades and surfaces an + actionable warning on ``result.synthesis_error`` (mirrored to the adversarial + ``verdict_error``). Critics never run because no proposal exists. + """ _all_keys(monkeypatch) critic_calls: list[str] = [] + proposer_attempts: list[str] = [] def handler(model, messages, **kwargs): system = _system_text(messages) @@ -368,7 +375,8 @@ def handler(model, messages, **kwargs): if "critic on an adversarial review" in system: # pragma: no cover critic_calls.append(model) return make_response("crit") - # Proposal (grok) fails. + # Every proposal attempt fails (grok first, then gemini fallback). + proposer_attempts.append(model) raise RuntimeError("proposer down") patch_call_model(handler) @@ -376,13 +384,97 @@ def handler(model, messages, **kwargs): council = Council(models=["grok", "gemini"], synthesizer="claude", config=_config()) result = await council.adversarial("q") - adv = result.adversarial - assert not adv.proposal.ok - assert adv.critiques == [] + # Both members were tried as proposer in council order before degrading. + assert proposer_attempts == ["xai/grok-4.3", "gemini/gemini-2.5-pro"] + # No critics ran -- there was no usable proposal to refute. assert critic_calls == [] + # The actionable degrade warning is surfaced on the CouncilResult. + assert result.synthesis_error is not None + assert "degraded to synthesize" in result.synthesis_error + assert "grok, gemini" in result.synthesis_error + # Mirrored into the adversarial structure for field-specific consumers. + adv = result.adversarial + assert adv is not None assert adv.verdict is None - assert "proposal from 'grok' failed" in adv.verdict_error - assert result.synthesis is None + assert adv.verdict_error == result.synthesis_error + assert not adv.proposal.ok + + +async def test_adversarial_failed_proposer_falls_back_to_next_member(monkeypatch, patch_call_model): + """A chosen proposer returning an unusable answer falls back to the next member. + + The run completes with a real verdict (issue #9). The failed first proposer + is recorded in ``answers`` but never doubles as a critic; the surviving + members critique the *successful* fallback proposal. + """ + _all_keys(monkeypatch) + + def handler(model, messages, **kwargs): + system = _system_text(messages) + if "judge of an adversarial review" in system: + return make_response("VERDICT AFTER FALLBACK") + if "critic on an adversarial review" in system: + return make_response(f"critique from {model}") + # Proposal step: the first proposer (grok) fails; gemini succeeds. + if model == "xai/grok-4.3": + raise RuntimeError("grok malformed proposal") + return make_response(f"proposal from {model}") + + patch_call_model(handler) + + council = Council( + models=["grok", "gemini", "perplexity"], + synthesizer="claude", + config=_config(), + ) + result = await council.adversarial("q") # requested proposer = grok + + adv = result.adversarial + assert adv is not None + # Fallback proposer is gemini (next available after the failed grok). + assert adv.proposer == "gemini" + assert adv.proposal.ok + assert "proposal from gemini/gemini-2.5-pro" in adv.proposal.answer + # The failed grok attempt is NOT a critic; perplexity is the sole critic. + assert {c.name for c in adv.critiques} == {"perplexity"} + # The run completed with a real verdict despite the proposer failure. + assert adv.verdict == "VERDICT AFTER FALLBACK" + assert result.synthesis == "VERDICT AFTER FALLBACK" + assert result.synthesis_error is None + # answers carries the failed proposal attempt + the successful proposal + critique. + failed = [a for a in result.answers if not a.ok] + assert {a.name for a in failed} == {"grok"} + + +async def test_adversarial_explicit_failed_proposer_falls_back(monkeypatch, patch_call_model): + """An explicit --proposer that fails still falls back to the next member.""" + _all_keys(monkeypatch) + + def handler(model, messages, **kwargs): + system = _system_text(messages) + if "judge of an adversarial review" in system: + return make_response("V") + if "critic on an adversarial review" in system: + return make_response("crit") + if model == "perplexity/sonar-pro": + raise RuntimeError("perplexity proposal blocked") + return make_response(f"prop {model}") + + patch_call_model(handler) + + council = Council( + models=["grok", "gemini", "perplexity"], + synthesizer="claude", + config=_config(), + ) + # Explicitly request perplexity as proposer; it fails -> first available (grok). + result = await council.adversarial("q", proposer="perplexity") + + adv = result.adversarial + assert adv is not None + assert adv.proposer == "grok" + assert adv.proposal.ok + assert adv.verdict == "V" async def test_adversarial_proposer_no_key_falls_back(monkeypatch, patch_call_model, clear_keys): diff --git a/tests/test_providers.py b/tests/test_providers.py index 826193c..d652da8 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -136,6 +136,33 @@ async def boom(url, headers, json_body, timeout): assert "timed out" in answer.error +async def test_call_model_gemini_missing_parts_becomes_error(monkeypatch): + """A malformed Gemini body (missing candidates[0].content.parts) -> ModelAnswer.error. + + Issue #9 end-to-end: the adapter's ProviderError must flow through call_model + as ``.error`` (never a raised KeyError), so one bad proposer cannot abort the + adversarial run. Exercises the real GeminiAdapter + the real call_model path + with only the transport patched. + """ + monkeypatch.setenv("GEMINI_API_KEY", "AIza-dummy") + monkeypatch.setenv("CONCLAVE_CONFIG", "/nonexistent/conclave.yml") + + async def malformed_gemini(url, headers, json_body, timeout): + # 200 OK but the candidate carries no content.parts (blocked/empty shape). + return 200, {"candidates": [{"finishReason": "SAFETY"}]} + + monkeypatch.setattr("conclave.transport.post_json", malformed_gemini) + + answer = await call_model( + "gemini", + "gemini/gemini-2.5-pro", + [{"role": "user", "content": "hi"}], + ) + assert not answer.ok + assert answer.answer is None + assert "missing candidates" in answer.error + + async def test_call_model_missing_key_is_error(monkeypatch): """No key in env -> a clean ModelAnswer.error naming the env var, never raises.""" monkeypatch.delenv("OPENAI_API_KEY", raising=False)