Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions src/codeforerunner/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ def cmd_generate(args: argparse.Namespace) -> int:
repo_root = Path(args.repo).resolve() if args.repo else Path.cwd()
cfg = load_from_repo(repo_root)

provider_name = args.provider or (cfg.provider if cfg else "anthropic")
explicit_provider = args.provider or (cfg.provider if cfg else None)
provider_name = explicit_provider or "anthropic"
model = args.model or (cfg.model if cfg else None)
Comment on lines +114 to 116
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

explicit_provider incorrectly treats default config as user-explicit provider.

Line 114 currently marks cfg.provider as explicit even when it comes from defaults, so fallback is skipped whenever forerunner.config.yaml exists without a provider: key. That breaks the intended “no explicit provider → auto-fallback to Ollama” flow.

💡 Proposed fix
-    from codeforerunner.config import load_from_repo
+    from codeforerunner.config import CONFIG_FILENAME, load_from_repo
@@
-    explicit_provider = args.provider or (cfg.provider if cfg else None)
-    provider_name = explicit_provider or "anthropic"
+    cfg_path = repo_root / CONFIG_FILENAME
+    config_provider_is_explicit = False
+    if cfg_path.is_file():
+        config_provider_is_explicit = any(
+            line.split("#", 1)[0].strip().startswith("provider:")
+            for line in cfg_path.read_text(encoding="utf-8").splitlines()
+        )
+
+    explicit_provider = args.provider or (
+        cfg.provider if (cfg and config_provider_is_explicit) else None
+    )
+    provider_name = args.provider or (cfg.provider if cfg else "anthropic")

Also add a regression test for: config file exists without provider:, no API key, ollama_available=True ⇒ fallback occurs.

Also applies to: 136-147

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/codeforerunner/cli.py` around lines 114 - 116, The current logic treats
cfg.provider as an explicit user choice even when the config file contains
defaults; fix explicit_provider by checking whether the provider key was
actually present in the loaded config (e.g., use a presence check on the config
object or its raw dict rather than truthiness of cfg.provider) and only treat it
as explicit if the key exists; apply the same presence-based check to the other
provider-selection spots noted around lines 136-147 (where provider/model are
derived), and add a regression test that writes a config file without a provider
key, ensures no API key is set, sets ollama_available=True, runs the CLI
selection path, and asserts the selected provider is "ollama".

provider_cls = _providers.get(provider_name)
provider = provider_cls()
Expand All @@ -132,8 +133,19 @@ def cmd_generate(args: argparse.Namespace) -> int:
env_var = (cfg.api_key_env.get(provider_name) if cfg else None) or provider.default_env_var
api_key = os.environ.get(env_var)
if api_key is None and provider_name != "ollama":
print(f"error: missing API key; set ${env_var}", file=sys.stderr)
return 3
if explicit_provider is None and _providers.ollama_available():
provider_name = "ollama"
provider_cls = _providers.get("ollama")
provider = provider_cls()
if not args.model:
model = provider.default_model
print("info: no API key; falling back to Ollama (local mode)", file=sys.stderr)
else:
msg = f"error: missing API key; set ${env_var}"
if explicit_provider is None:
msg += "\nhint: start Ollama for keyless local generation (https://ollama.com)"
print(msg, file=sys.stderr)
return 3

if getattr(args, "stream", False):
try:
Expand Down
14 changes: 12 additions & 2 deletions src/codeforerunner/doctor.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,13 +225,23 @@ def _check_config_loadable(repo: Path) -> list[Finding]:


def _check_provider_api_key(repo: Path) -> list[Finding]:
from codeforerunner.providers.ollama import is_available as _ollama_available

cfg_path = repo / CONFIG_FILENAME
if not cfg_path.is_file():
if _ollama_available():
return [
Finding(
"ok",
"provider-api-key",
"no config; Ollama running — generate will use local mode automatically",
)
]
return [
Finding(
"ok",
"provider-api-key",
f"no {CONFIG_FILENAME}; provider key not checked",
f"no {CONFIG_FILENAME}; set an API key in config or start Ollama for keyless local generation",
)
]
try:
Expand Down Expand Up @@ -259,7 +269,7 @@ def _check_provider_api_key(repo: Path) -> list[Finding]:
Finding(
"ok",
"provider-api-key",
"ollama needs no API key (OLLAMA_HOST optional)",
"running in local mode (Ollama; no API key needed)",
)
]
env_var = cfg.api_key_env.get(provider) or _DEFAULT_PROVIDER_ENV.get(provider, "")
Expand Down
3 changes: 2 additions & 1 deletion src/codeforerunner/providers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from codeforerunner.providers.anthropic import AnthropicProvider
from codeforerunner.providers.base import CompletionResult, Provider, ProviderError
from codeforerunner.providers.google import GoogleProvider
from codeforerunner.providers.ollama import OllamaProvider
from codeforerunner.providers.ollama import OllamaProvider, is_available as ollama_available
from codeforerunner.providers.openai import OpenAIProvider

__all__ = [
Expand All @@ -18,6 +18,7 @@
"ProviderError",
"REGISTRY",
"get",
"ollama_available",
]

REGISTRY: dict[str, type] = {
Expand Down
10 changes: 10 additions & 0 deletions src/codeforerunner/providers/ollama.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,16 @@
DEFAULT_HOST = "http://localhost:11434"


def is_available(host: str | None = None) -> bool:
"""Return True if an Ollama instance is reachable at the configured host."""
base = (host or os.environ.get("OLLAMA_HOST") or DEFAULT_HOST).rstrip("/")
try:
urllib.request.urlopen(f"{base}/api/tags", timeout=2)
return True
except Exception:
return False


class OllamaProvider:
name = "ollama"
default_env_var = "OLLAMA_HOST"
Expand Down
130 changes: 130 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,3 +307,133 @@ def complete(self, *, prompt, model=None, api_key=None):
assert rc == 0
assert cap.out == "ok\n"
assert calls == [{"model": "claude-opus-4-7", "api_key": "override-secret"}]


# ── Ollama local-mode fallback ─────────────────────────────────────────────────

def test_generate_falls_back_to_ollama_when_no_key_and_ollama_running(
tmp_path, capsys, monkeypatch
):
"""No explicit provider, no API key, Ollama reachable → auto-switch to Ollama."""
_seed_repo_with_config(tmp_path)
(tmp_path / "forerunner.config.yaml").unlink()
calls: list[dict] = []

class FakeOllamaProvider:
default_env_var = "OLLAMA_HOST"
default_model = "llama3"

def complete(self, *, prompt, model=None, api_key=None):
calls.append({"model": model, "api_key": api_key})
return CompletionResult(text="ollama output", model=model or "llama3")

from codeforerunner import providers
from unittest.mock import patch

monkeypatch.setitem(providers.REGISTRY, "ollama", FakeOllamaProvider)
monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False)

with patch("codeforerunner.providers.ollama_available", return_value=True):
rc = main(["--repo", str(tmp_path), "generate", "readme"])

cap = capsys.readouterr()
assert rc == 0
assert "local mode" in cap.err
assert cap.out == "ollama output\n"
assert calls[0]["model"] == "llama3"


def test_generate_no_fallback_when_provider_explicit_and_key_missing(
tmp_path, capsys, monkeypatch
):
"""Explicit --provider means no auto-fallback even if Ollama is running."""
_seed_repo_with_config(tmp_path)
(tmp_path / "forerunner.config.yaml").unlink()

class FakeProvider:
default_env_var = "FAKE_API_KEY"
default_model = "fake-default"

def complete(self, *, prompt, model=None, api_key=None): # pragma: no cover
raise AssertionError("should not be called")

from codeforerunner import providers
from unittest.mock import patch

monkeypatch.setitem(providers.REGISTRY, "fake", FakeProvider)
monkeypatch.delenv("FAKE_API_KEY", raising=False)

with patch("codeforerunner.providers.ollama_available", return_value=True):
rc = main(["--repo", str(tmp_path), "generate", "readme", "--provider", "fake"])

cap = capsys.readouterr()
assert rc == 3
assert "missing API key" in cap.err


def test_generate_no_fallback_when_config_provider_set_and_key_missing(
tmp_path, capsys, monkeypatch
):
"""Provider in config file counts as explicit — no auto-fallback."""
_seed_repo_with_config(tmp_path)
(tmp_path / "forerunner.config.yaml").write_text(
"provider: anthropic\n", encoding="utf-8"
)

from unittest.mock import patch

monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False)

with patch("codeforerunner.providers.ollama_available", return_value=True):
rc = main(["--repo", str(tmp_path), "generate", "readme"])

cap = capsys.readouterr()
assert rc == 3
assert "missing API key" in cap.err


def test_generate_missing_key_includes_ollama_hint_when_ollama_absent(
tmp_path, capsys, monkeypatch
):
"""No explicit provider, no API key, Ollama not running → error + Ollama hint."""
_seed_repo_with_config(tmp_path)
(tmp_path / "forerunner.config.yaml").unlink()

from unittest.mock import patch

monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False)

with patch("codeforerunner.providers.ollama_available", return_value=False):
rc = main(["--repo", str(tmp_path), "generate", "readme"])

cap = capsys.readouterr()
assert rc == 3
assert "missing API key" in cap.err
assert "Ollama" in cap.err


def test_generate_ollama_fallback_uses_explicit_model(tmp_path, capsys, monkeypatch):
"""--model flag is preserved when falling back to Ollama."""
_seed_repo_with_config(tmp_path)
(tmp_path / "forerunner.config.yaml").unlink()
calls: list[dict] = []

class FakeOllamaProvider:
default_env_var = "OLLAMA_HOST"
default_model = "llama3"

def complete(self, *, prompt, model=None, api_key=None):
calls.append({"model": model})
return CompletionResult(text="ok", model=model or "llama3")

from codeforerunner import providers
from unittest.mock import patch

monkeypatch.setitem(providers.REGISTRY, "ollama", FakeOllamaProvider)
monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False)

with patch("codeforerunner.providers.ollama_available", return_value=True):
rc = main(["--repo", str(tmp_path), "generate", "readme", "--model", "llama3.2"])

assert rc == 0
assert calls[0]["model"] == "llama3.2"
39 changes: 39 additions & 0 deletions tests/test_doctor.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,42 @@ def test_doctor_fix_does_not_overwrite_existing_config(tmp_path: Path, capsys):
capsys.readouterr()

assert cfg_path.read_text(encoding="utf-8") == "# my custom config\n"


# ── local-mode surfacing ───────────────────────────────────────────────────────

def test_provider_api_key_local_mode_when_ollama_running_no_config(tmp_path: Path):
from unittest.mock import patch
repo = _copy_repo_layout(tmp_path)
# no forerunner.config.yaml
with patch("codeforerunner.providers.ollama.is_available", return_value=True):
findings = run(repo)
matches = [f for f in findings if f.check == "provider-api-key"]
assert len(matches) == 1
assert matches[0].severity == "ok"
assert "local mode" in matches[0].message


def test_provider_api_key_hint_when_ollama_absent_no_config(tmp_path: Path):
from unittest.mock import patch
repo = _copy_repo_layout(tmp_path)
# no forerunner.config.yaml
with patch("codeforerunner.providers.ollama.is_available", return_value=False):
findings = run(repo)
matches = [f for f in findings if f.check == "provider-api-key"]
assert len(matches) == 1
assert matches[0].severity == "ok"
assert "Ollama" in matches[0].message


def test_provider_api_key_ollama_config_shows_local_mode(tmp_path: Path, monkeypatch):
repo = _copy_repo_layout(tmp_path)
(repo / "forerunner.config.yaml").write_text(
"provider: ollama\nmodel: llama3\n", encoding="utf-8"
)
monkeypatch.delenv("OLLAMA_HOST", raising=False)
findings = run(repo)
matches = [f for f in findings if f.check == "provider-api-key"]
assert len(matches) == 1
assert matches[0].severity == "ok"
assert "local mode" in matches[0].message
49 changes: 49 additions & 0 deletions tests/test_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -479,3 +479,52 @@ def test_ollama_stream_http_error_raises():
with patch("urllib.request.urlopen", side_effect=_stream_http_error(500)):
with pytest.raises(ProviderError, match="HTTP 500"):
list(OllamaProvider().stream(prompt="hi"))


# ── OllamaProvider.is_available ───────────────────────────────────────────────

def test_ollama_is_available_returns_true_when_reachable(monkeypatch):
monkeypatch.delenv("OLLAMA_HOST", raising=False)
with patch("urllib.request.urlopen", return_value=MagicMock()):
from codeforerunner.providers.ollama import is_available
assert is_available() is True


def test_ollama_is_available_returns_false_on_connection_error(monkeypatch):
import urllib.error
monkeypatch.delenv("OLLAMA_HOST", raising=False)
with patch("urllib.request.urlopen", side_effect=OSError("connection refused")):
from codeforerunner.providers.ollama import is_available
assert is_available() is False


def test_ollama_is_available_uses_env_host(monkeypatch):
monkeypatch.setenv("OLLAMA_HOST", "http://myhost:11434")
captured_url: list[str] = []

def fake_open(url, timeout=None):
captured_url.append(url)
return MagicMock()

with patch("urllib.request.urlopen", side_effect=fake_open):
from codeforerunner.providers.ollama import is_available
assert is_available() is True
assert captured_url[0].startswith("http://myhost:11434")


def test_ollama_is_available_uses_explicit_host():
captured_url: list[str] = []

def fake_open(url, timeout=None):
captured_url.append(url)
return MagicMock()

with patch("urllib.request.urlopen", side_effect=fake_open):
from codeforerunner.providers.ollama import is_available
assert is_available(host="http://custom:9999") is True
assert captured_url[0].startswith("http://custom:9999")


def test_ollama_available_exported_from_providers_package():
from codeforerunner.providers import ollama_available
assert callable(ollama_available)
Loading