feat(core): route 4 remaining provider sites through llm_resolution chain (#861) (#874)

* feat(core): route 4 remaining provider sites through llm_resolution chain (#861)

Follow-up to #768 (PR #860) which introduced codeframe/core/llm_resolution.py
as the single source of truth for the provider chain
(CODEFRAME_LLM_PROVIDER -> .codeframe/config.yaml -> anthropic).
Four code paths still constructed providers directly, bypassing the chain.

Sites migrated:
- codeframe/core/conductor.py:SupervisorResolver.llm  -- was get_provider()
  (bare -> anthropic). Concrete failure fixed: cf work batch run --strategy auto
  --llm-provider ollama now uses ollama for blocker auto-resolution instead
  of demanding an Anthropic key downstream of a passing pre-flight.
- codeframe/core/dependency_analyzer.py:analyze_dependencies -- the private
  _get_default_provider() helper (hardcoded AnthropicProvider + ValueError on
  missing key) is dropped; the call site now inlines
  create_provider(resolve_llm_settings(workspace.repo_path)) when provider=None.
- codeframe/core/prd_discovery.py:PrdDiscoverySession.__post_init__ -- api_key
  remains as a backward-compat field (constructs AnthropicProvider when set,
  preserving every existing caller). When api_key is unset the chain is used.
  The missing-key check is generalized via LLMSettings.required_key_env so
  the existing NoApiKeyError contract is preserved AND extended to any keyed
  provider (not just Anthropic).
- codeframe/core/adapters/streaming_chat.py:StreamingChatAdapter -- the
  no-provider fallback (used only by tests/external callers; production
  session_chat_ws.py already passes provider= explicitly) now uses the chain.
  The vestigial api_key parameter is kept for backward compat.

Regression coverage: tests/core/test_provider_resolution_chain.py adds one
test class per site (9 tests total) mirroring the reference pattern in
tests/ui/test_discovery_generate_tasks.py -- set CODEFRAME_LLM_PROVIDER=ollama,
delete ANTHROPIC_API_KEY, mock create_provider, assert the chain is invoked
and the resulting provider is the one used.

All existing tests in the 4 affected areas pass unchanged (67+17=84 tests).

* fix(streaming-chat): preserve api_key= legacy contract + tighten regression tests

Addresses cross-family review (opencode/GLM) on PR for #861:

Major 1 + Major 2: streaming_chat docstring lied about api_key behavior, and
the api_key= legacy contract was asymmetric with PrdDiscoverySession (which
preserves it). Restore the legacy branch: when api_key is set AND provider
is None, construct AnthropicProvider(api_key=...) — mirroring prd_discovery.
Update docstring to describe the new contract accurately.

Suggestion 1: test_explicit_provider_skips_chain for dependency_analyzer
previously passed task_ids=[] which hit the early-return at line ~63 before
the provider was ever consulted. Seed a task and assert explicit_provider.complete
was actually called, so the test fails if explicit-provider handling breaks.

Nitpick 2: lock in model_flag= propagation in streaming_chat (the only site
that threads model into resolve_llm_settings) via an assertion on settings.model.

New test_legacy_api_key_skips_chain locks in the Major 2 fix. Total tests:
10 (was 9).

* chore: gitignore .opencode-tmp scratch dir (review artifacts)

* fix(tests): patch llm_resolution.create_provider in prd_generate CLI tests

The 8 CLI tests in tests/cli/test_prd_generate.py patched
codeframe.core.prd_discovery.AnthropicProvider to substitute a mock LLM.
After #861 migrated PrdDiscoverySession to use the shared llm_resolution
chain, the construction path moved to create_provider -> get_provider ->
codeframe.adapters.llm.anthropic.AnthropicProvider, which the existing
patch target didn't intercept. CI surfaced this as 401 'invalid x-api-key'
errors reaching the real Anthropic API.

Fix: retarget the patch to codeframe.core.llm_resolution.create_provider
across all 8 affected test methods. The mock setup pattern is unchanged
(mock.return_value = mock_llm_provider); only the patch site moved.

This is a contract change documented in issue #861 (PrdDiscoverySession
now resolves via the shared chain) — test patches are updated to reflect
the new contract, per the anti-mutation rule's allowance for documented
contract changes.

* docs(prd-discovery): mark api_key as optional in class docstring

Per claude[bot] review on PR #874: the PrdDiscoverySession class docstring
still claimed api_key is 'required' even though #861 made it optional (when
unset, the provider is resolved via the llm_resolution chain). Aligns with
the streaming_chat docstring fix from the same migration.

Author: frankbria

.gitignore

@@ -98,3 +98,4 @@ tests/e2e/.e2e-state.db*
 test_audit_report.md
 tests/integration/.env.integration
 web-ui/test-results/
+.opencode-tmp/

codeframe/core/adapters/streaming_chat.py

@@ -169,24 +169,40 @@ def __init__(
             db_repo: ``InteractiveSessionRepository`` instance.
             workspace_path: Absolute path used to scope file-system tool calls.
             model: Model identifier passed to the provider's ``async_stream()``.
-            api_key: API key used when constructing the default
-                ``AnthropicProvider`` (when ``provider`` is ``None``).
-            provider: LLM provider to use for streaming.  When ``None``,
-                an ``AnthropicProvider`` is constructed automatically using
-                ``api_key`` or the ``ANTHROPIC_API_KEY`` environment variable.
+                Also threaded into ``resolve_llm_settings`` as ``model_flag``
+                when the provider must be resolved (#861).
+            api_key: Legacy construct — when set AND ``provider`` is ``None``,
+                an ``AnthropicProvider`` is constructed with this key
+                (matches the pre-#861 contract). Ignored if ``provider`` is
+                given. Prefer passing ``provider`` directly.
+            provider: LLM provider to use for streaming.  When ``None`` and
+                ``api_key`` is also ``None``, the provider is resolved via the
+                shared ``llm_resolution`` chain (#861):
+                ``CODEFRAME_LLM_PROVIDER`` → ``.codeframe/config.yaml`` →
+                anthropic.
 
         Raises:
-            ValueError: If no provider is given and no Anthropic API key is
-                available.
+            ValueError: If no provider is given, no ``api_key`` is given, and
+                the chain resolves to a provider whose required API key is
+                missing from the environment.
         """
         if provider is None:
-            # Backward-compatibility fallback: callers that haven't been
-            # updated to pass an explicit provider (e.g. tests using the old
-            # api_key= constructor argument) still get an AnthropicProvider.
-            # New callers should construct the provider themselves and pass it
-            # in — see session_chat_ws.py for the recommended pattern.
-            from codeframe.adapters.llm.anthropic import AnthropicProvider
-            provider = AnthropicProvider(api_key=api_key)
+            if api_key is not None:
+                # Legacy path: explicit api_key always means Anthropic.
+                # Preserves the pre-#861 contract (mirrors PrdDiscoverySession).
+                from codeframe.adapters.llm.anthropic import AnthropicProvider
+
+                provider = AnthropicProvider(api_key=api_key)
+            else:
+                # New: resolve via the shared chain
+                # (CODEFRAME_LLM_PROVIDER → .codeframe/config.yaml → anthropic,
+                # per #861). New callers should construct the provider
+                # themselves and pass it in — see session_chat_ws.py for the
+                # recommended pattern.
+                from codeframe.core.llm_resolution import create_provider, resolve_llm_settings
+
+                settings = resolve_llm_settings(workspace_path, model_flag=model)
+                provider = create_provider(settings)
 
         self._session_id = session_id
         self._db_repo = db_repo

codeframe/core/conductor.py

@@ -119,10 +119,15 @@ def __init__(self, workspace: Workspace):
 
     @property
     def llm(self):
-        """Lazy-load LLM provider."""
+        """Lazy-load LLM provider.
+
+        Resolves via the shared ``llm_resolution`` chain (#861):
+        ``CODEFRAME_LLM_PROVIDER`` → ``.codeframe/config.yaml`` → anthropic.
+        """
         if self._llm is None:
-            from codeframe.adapters.llm import get_provider
-            self._llm = get_provider()
+            from codeframe.core.llm_resolution import create_provider, resolve_llm_settings
+
+            self._llm = create_provider(resolve_llm_settings(self.workspace.repo_path))
         return self._llm
 
     def try_resolve_blocked_task(self, task_id: str) -> bool:

codeframe/core/dependency_analyzer.py

@@ -11,7 +11,6 @@
 """
 
 import json
-import os
 import re
 from typing import Optional
 
@@ -81,7 +80,11 @@ def analyze_dependencies(
 
     # Get or create LLM provider
     if provider is None:
-        provider = _get_default_provider()
+        # Resolve via the shared chain (#861):
+        # CODEFRAME_LLM_PROVIDER → .codeframe/config.yaml → anthropic.
+        from codeframe.core.llm_resolution import create_provider, resolve_llm_settings
+
+        provider = create_provider(resolve_llm_settings(workspace.repo_path))
 
     # Call LLM
     response = provider.complete(
@@ -211,19 +214,3 @@ def apply_inferred_dependencies(
         if deps:
             task_module.update_depends_on(workspace, task_id, deps)
 
-
-def _get_default_provider():
-    """Get the default Anthropic LLM provider.
-
-    Returns:
-        AnthropicProvider instance
-
-    Raises:
-        ValueError: If ANTHROPIC_API_KEY not set
-    """
-    api_key = os.getenv("ANTHROPIC_API_KEY")
-    if not api_key:
-        raise ValueError("ANTHROPIC_API_KEY environment variable not set")
-
-    from codeframe.adapters.llm.anthropic import AnthropicProvider
-    return AnthropicProvider(api_key=api_key)

codeframe/core/prd_discovery.py

@@ -222,7 +222,10 @@ class PrdDiscoverySession:
 
     Attributes:
         workspace: The workspace this session belongs to
-        api_key: API key for LLM provider (required)
+        api_key: Optional; when set, constructs an ``AnthropicProvider``
+            with this key (legacy contract). When unset, the provider is
+            resolved via the ``llm_resolution`` chain (#861):
+            ``CODEFRAME_LLM_PROVIDER`` → ``.codeframe/config.yaml`` → anthropic.
         session_id: Unique session identifier
         state: Current session state
     """
@@ -239,15 +242,33 @@ class PrdDiscoverySession:
     _is_complete: bool = field(default=False, init=False)
 
     def __post_init__(self):
-        """Initialize the LLM provider."""
-        key = self.api_key or os.getenv("ANTHROPIC_API_KEY")
-        if not key:
+        """Initialize the LLM provider.
+
+        Resolution order (#861):
+        1. ``api_key`` set → ``AnthropicProvider(api_key=...)`` (legacy contract;
+           preserves existing callers passing an explicit key).
+        2. Otherwise → shared ``llm_resolution`` chain
+           (``CODEFRAME_LLM_PROVIDER`` → ``.codeframe/config.yaml`` → anthropic).
+           When the resolved provider requires an API key (see
+           ``LLMSettings.required_key_env``) and it is missing from the
+           environment, ``NoApiKeyError`` is raised — generalizes the previous
+           Anthropic-only check to all keyed providers.
+        """
+        if self.api_key:
+            self._llm_provider = AnthropicProvider(api_key=self.api_key)
+            return
+
+        from codeframe.core.llm_resolution import create_provider, resolve_llm_settings
+
+        settings = resolve_llm_settings(self.workspace.repo_path)
+        required_env = settings.required_key_env
+        if required_env and not os.getenv(required_env, ""):
             raise NoApiKeyError(
-                "ANTHROPIC_API_KEY is required for AI-driven discovery. "
+                f"{required_env} is required for AI-driven discovery "
+                f"(resolved provider: {settings.provider_type}). "
                 "Set the environment variable or pass api_key parameter."
             )
-
-        self._llm_provider = AnthropicProvider(api_key=key)
+        self._llm_provider = create_provider(settings)
 
     @property
     def answered_count(self) -> int:

tests/cli/test_prd_generate.py

@@ -128,7 +128,7 @@ def test_no_api_key_shows_error(self, workspace_dir: Path, monkeypatch):
         assert result.exit_code == 1
         assert "ANTHROPIC_API_KEY" in result.output
 
-    @patch("codeframe.core.prd_discovery.AnthropicProvider")
+    @patch("codeframe.core.llm_resolution.create_provider")
     def test_command_starts_discovery(
         self, mock_provider_class, workspace_dir: Path, monkeypatch, mock_llm_provider
     ):
@@ -153,7 +153,7 @@ def test_command_starts_discovery(
         # Should start discovery
         assert "AI-driven" in result.output or "discovery" in result.output.lower()
 
-    @patch("codeframe.core.prd_discovery.AnthropicProvider")
+    @patch("codeframe.core.llm_resolution.create_provider")
     def test_existing_prd_prompts_confirmation(
         self, mock_provider_class, workspace_dir: Path, monkeypatch, mock_llm_provider
     ):
@@ -179,7 +179,7 @@ def test_existing_prd_prompts_confirmation(
 
         assert "Cancelled" in result.output or "existing" in result.output.lower()
 
-    @patch("codeframe.core.prd_discovery.AnthropicProvider")
+    @patch("codeframe.core.llm_resolution.create_provider")
     def test_help_command_shows_commands(
         self, mock_provider_class, workspace_dir: Path, monkeypatch, mock_llm_provider
     ):
@@ -203,7 +203,7 @@ def test_help_command_shows_commands(
         assert "/pause" in result.output
         assert "/quit" in result.output
 
-    @patch("codeframe.core.prd_discovery.AnthropicProvider")
+    @patch("codeframe.core.llm_resolution.create_provider")
     def test_pause_saves_progress(
         self, mock_provider_class, workspace_dir: Path, monkeypatch, mock_llm_provider
     ):
@@ -231,7 +231,7 @@ def test_pause_saves_progress(
 class TestPrdGenerateResume:
     """Tests for resuming paused discovery sessions."""
 
-    @patch("codeframe.core.prd_discovery.AnthropicProvider")
+    @patch("codeframe.core.llm_resolution.create_provider")
     def test_resume_with_invalid_blocker(
         self, mock_provider_class, workspace_dir: Path, monkeypatch, mock_llm_provider
     ):
@@ -252,7 +252,7 @@ def test_resume_with_invalid_blocker(
 class TestPrdGenerateComplete:
     """Tests for completing discovery and generating PRD."""
 
-    @patch("codeframe.core.prd_discovery.AnthropicProvider")
+    @patch("codeframe.core.llm_resolution.create_provider")
     def test_complete_discovery_generates_prd(
         self, mock_provider_class, workspace_dir: Path, monkeypatch
     ):
@@ -330,7 +330,7 @@ def complete_side_effect(messages, **kwargs):
         prd_record = prd.get_latest(workspace)
         assert prd_record is not None
 
-    @patch("codeframe.core.prd_discovery.AnthropicProvider")
+    @patch("codeframe.core.llm_resolution.create_provider")
     def test_generated_prd_shows_preview(
         self, mock_provider_class, workspace_dir: Path, monkeypatch
     ):
@@ -402,7 +402,7 @@ def complete_side_effect(messages, **kwargs):
 class TestPrdGenerateValidation:
     """Tests for AI validation during discovery."""
 
-    @patch("codeframe.core.prd_discovery.AnthropicProvider")
+    @patch("codeframe.core.llm_resolution.create_provider")
     def test_ai_rejects_vague_answer(
         self, mock_provider_class, workspace_dir: Path, monkeypatch
     ):