diff --git a/.gitignore b/.gitignore index a43db9727..c513cb17a 100644 --- a/.gitignore +++ b/.gitignore @@ -163,3 +163,5 @@ docs/HOSTED_PRODUCT_UPGRADE_PLAN.md docs/HOSTED_IMPLEMENTATION_GUIDE.md docs/HOSTED_PROGRESS.md scripts/md_to_docx.py + +# Local issue analysis scratch (not for OSS distribution) diff --git a/packages/cli/src/repowise/cli/commands/init_cmd/command.py b/packages/cli/src/repowise/cli/commands/init_cmd/command.py index 1ccde4186..9d329c079 100644 --- a/packages/cli/src/repowise/cli/commands/init_cmd/command.py +++ b/packages/cli/src/repowise/cli/commands/init_cmd/command.py @@ -158,6 +158,14 @@ def _run_generation_phase( lang_parts = [f"{lang} {pct:.0%}" for lang, pct in lang_items] console.print(f" Languages: {', '.join(lang_parts)}") + # Warn when a local provider runs with default concurrency + if provider.provider_name in ("ollama", "codex_cli", "opencode") and concurrency > 4: + console.print( + f" [yellow]Warning:[/yellow] {provider.provider_name} is a local provider " + f"running with concurrency={concurrency}. " + f"If you see timeout errors, try [bold]--concurrency 1[/bold]." + ) + console.print( f" Coverage: {int(chosen_pct * 100)}% / " f"~{est.estimated_input_tokens + est.estimated_output_tokens:,} tokens " diff --git a/packages/core/src/repowise/core/providers/llm/deepseek.py b/packages/core/src/repowise/core/providers/llm/deepseek.py index 2e8fa0949..5e768be3b 100644 --- a/packages/core/src/repowise/core/providers/llm/deepseek.py +++ b/packages/core/src/repowise/core/providers/llm/deepseek.py @@ -18,6 +18,7 @@ from typing import TYPE_CHECKING, Any import structlog +from openai import APIError as _OpenAIAPIError from openai import APIStatusError as _OpenAIAPIStatusError from openai import AsyncOpenAI from openai import RateLimitError as _OpenAIRateLimitError @@ -267,6 +268,10 @@ async def _generate_with_retry( ) from exc except _OpenAIAPIStatusError as exc: raise ProviderError("deepseek", str(exc), status_code=exc.status_code) from exc + except _OpenAIAPIError as exc: + raise ProviderError( + "deepseek", str(exc), status_code=getattr(exc, "status_code", None) + ) from exc usage = response.usage result = GeneratedResponse( @@ -341,6 +346,10 @@ async def stream_chat( ) from exc except _OpenAIAPIStatusError as exc: raise ProviderError("deepseek", str(exc), status_code=exc.status_code) from exc + except _OpenAIAPIError as exc: + raise ProviderError( + "deepseek", str(exc), status_code=getattr(exc, "status_code", None) + ) from exc tool_calls_acc: dict[int, dict[str, Any]] = {} @@ -410,3 +419,7 @@ async def stream_chat( ) from exc except _OpenAIAPIStatusError as exc: raise ProviderError("deepseek", str(exc), status_code=exc.status_code) from exc + except _OpenAIAPIError as exc: + raise ProviderError( + "deepseek", str(exc), status_code=getattr(exc, "status_code", None) + ) from exc diff --git a/packages/core/src/repowise/core/providers/llm/ollama.py b/packages/core/src/repowise/core/providers/llm/ollama.py index c9d2df15f..c5153f10e 100644 --- a/packages/core/src/repowise/core/providers/llm/ollama.py +++ b/packages/core/src/repowise/core/providers/llm/ollama.py @@ -25,15 +25,11 @@ from typing import Any import structlog +from openai import APIError as _OpenAIAPIError from openai import APIStatusError as _OpenAIAPIStatusError from openai import AsyncOpenAI -from tenacity import ( - RetryError, - retry, - retry_if_exception_type, - stop_after_attempt, - wait_exponential_jitter, -) +from openai import RateLimitError as _OpenAIRateLimitError +from tenacity import RetryError, retry from repowise.core.providers.llm.base import ( BaseProvider, @@ -42,18 +38,19 @@ GeneratedResponse, ProviderError, ProviderModelOption, + RateLimitError, ensure_reasoning_supported, fallback_model_option, + parse_retry_after, + provider_retry_stop, + provider_retry_wait, + provider_should_retry, ) from repowise.core.rate_limiter import RateLimiter from repowise.core.reasoning import ReasoningMode log = structlog.get_logger(__name__) -_MAX_RETRIES = 3 -_MIN_WAIT = 1.0 -_MAX_WAIT = 8.0 # Ollama can be slow on first load, allow more wait time - _DEFAULT_BASE_URL = "http://localhost:11434" @@ -188,13 +185,13 @@ async def generate( except RetryError as exc: raise ProviderError( "ollama", - f"All {_MAX_RETRIES} retries exhausted: {exc}", + f"All retries exhausted: {exc}", ) from exc @retry( - retry=retry_if_exception_type(ProviderError), - stop=stop_after_attempt(_MAX_RETRIES), - wait=wait_exponential_jitter(initial=_MIN_WAIT, max=_MAX_WAIT), + retry=provider_should_retry, + stop=provider_retry_stop, + wait=provider_retry_wait, reraise=True, ) async def _generate_with_retry( @@ -215,8 +212,21 @@ async def _generate_with_retry( {"role": "user", "content": user_prompt}, ], ) + except _OpenAIRateLimitError as exc: + raise RateLimitError( + "ollama", + str(exc), + status_code=429, + retry_after=parse_retry_after( + getattr(getattr(exc, "response", None), "headers", None) + ), + ) from exc except _OpenAIAPIStatusError as exc: raise ProviderError("ollama", str(exc), status_code=exc.status_code) from exc + except _OpenAIAPIError as exc: + raise ProviderError( + "ollama", str(exc), status_code=getattr(exc, "status_code", None) + ) from exc usage = response.usage result = GeneratedResponse( @@ -265,8 +275,21 @@ async def stream_chat( try: stream = await self._client.chat.completions.create(**kwargs) + except _OpenAIRateLimitError as exc: + raise RateLimitError( + "ollama", + str(exc), + status_code=429, + retry_after=parse_retry_after( + getattr(getattr(exc, "response", None), "headers", None) + ), + ) from exc except _OpenAIAPIStatusError as exc: raise ProviderError("ollama", str(exc), status_code=exc.status_code) from exc + except _OpenAIAPIError as exc: + raise ProviderError( + "ollama", str(exc), status_code=getattr(exc, "status_code", None) + ) from exc tool_calls_acc: dict[int, dict[str, Any]] = {} @@ -314,5 +337,18 @@ async def stream_chat( tool_calls_acc.clear() stop_reason = "tool_use" if finish == "tool_calls" else "end_turn" yield ChatStreamEvent(type="stop", stop_reason=stop_reason) + except _OpenAIRateLimitError as exc: + raise RateLimitError( + "ollama", + str(exc), + status_code=429, + retry_after=parse_retry_after( + getattr(getattr(exc, "response", None), "headers", None) + ), + ) from exc except _OpenAIAPIStatusError as exc: raise ProviderError("ollama", str(exc), status_code=exc.status_code) from exc + except _OpenAIAPIError as exc: + raise ProviderError( + "ollama", str(exc), status_code=getattr(exc, "status_code", None) + ) from exc diff --git a/packages/core/src/repowise/core/providers/llm/openai.py b/packages/core/src/repowise/core/providers/llm/openai.py index 483118ef1..7cee71040 100644 --- a/packages/core/src/repowise/core/providers/llm/openai.py +++ b/packages/core/src/repowise/core/providers/llm/openai.py @@ -18,6 +18,7 @@ from typing import TYPE_CHECKING, Any import structlog +from openai import APIError as _OpenAIAPIError from openai import APIStatusError as _OpenAIAPIStatusError from openai import AsyncOpenAI from openai import RateLimitError as _OpenAIRateLimitError @@ -324,6 +325,10 @@ async def _generate_with_retry( ) from exc except _OpenAIAPIStatusError as exc: raise ProviderError("openai", str(exc), status_code=exc.status_code) from exc + except _OpenAIAPIError as exc: + raise ProviderError( + "openai", str(exc), status_code=getattr(exc, "status_code", None) + ) from exc usage = response.usage cached = 0 @@ -406,6 +411,10 @@ async def stream_chat( ) from exc except _OpenAIAPIStatusError as exc: raise ProviderError("openai", str(exc), status_code=exc.status_code) from exc + except _OpenAIAPIError as exc: + raise ProviderError( + "openai", str(exc), status_code=getattr(exc, "status_code", None) + ) from exc # Track in-progress tool calls (OpenAI streams them incrementally) tool_calls_acc: dict[int, dict[str, Any]] = {} @@ -479,3 +488,7 @@ async def stream_chat( ) from exc except _OpenAIAPIStatusError as exc: raise ProviderError("openai", str(exc), status_code=exc.status_code) from exc + except _OpenAIAPIError as exc: + raise ProviderError( + "openai", str(exc), status_code=getattr(exc, "status_code", None) + ) from exc diff --git a/packages/core/src/repowise/core/providers/llm/openrouter.py b/packages/core/src/repowise/core/providers/llm/openrouter.py index 689fd0488..96c8525d2 100644 --- a/packages/core/src/repowise/core/providers/llm/openrouter.py +++ b/packages/core/src/repowise/core/providers/llm/openrouter.py @@ -18,6 +18,7 @@ from typing import TYPE_CHECKING, Any import structlog +from openai import APIError as _OpenAIAPIError from openai import APIStatusError as _OpenAIAPIStatusError from openai import AsyncOpenAI from openai import RateLimitError as _OpenAIRateLimitError @@ -330,6 +331,10 @@ async def _generate_with_retry( ) from exc except _OpenAIAPIStatusError as exc: raise ProviderError("openrouter", str(exc), status_code=exc.status_code) from exc + except _OpenAIAPIError as exc: + raise ProviderError( + "openrouter", str(exc), status_code=getattr(exc, "status_code", None) + ) from exc usage = response.usage result = GeneratedResponse( @@ -390,6 +395,10 @@ async def stream_chat( ) from exc except _OpenAIAPIStatusError as exc: raise ProviderError("openrouter", str(exc), status_code=exc.status_code) from exc + except _OpenAIAPIError as exc: + raise ProviderError( + "openrouter", str(exc), status_code=getattr(exc, "status_code", None) + ) from exc # Track in-progress tool calls (OpenAI-compatible streaming) tool_calls_acc: dict[int, dict[str, Any]] = {} @@ -463,3 +472,7 @@ async def stream_chat( ) from exc except _OpenAIAPIStatusError as exc: raise ProviderError("openrouter", str(exc), status_code=exc.status_code) from exc + except _OpenAIAPIError as exc: + raise ProviderError( + "openrouter", str(exc), status_code=getattr(exc, "status_code", None) + ) from exc diff --git a/tests/unit/test_providers/test_ollama_provider.py b/tests/unit/test_providers/test_ollama_provider.py index 8806d425b..ff44faa23 100644 --- a/tests/unit/test_providers/test_ollama_provider.py +++ b/tests/unit/test_providers/test_ollama_provider.py @@ -2,10 +2,16 @@ from __future__ import annotations +from unittest.mock import AsyncMock + +import httpx import pytest pytest.importorskip("openai", reason="openai SDK not installed") +from openai import APIConnectionError, APIError, APIStatusError, APITimeoutError + +from repowise.core.providers.llm.base import ProviderError from repowise.core.providers.llm.ollama import OllamaProvider @@ -40,9 +46,71 @@ def fake_get(url, *, timeout): options = OllamaProvider(base_url="http://localhost:11434").available_model_options() assert captured["url"] == "http://localhost:11434/api/tags" - models = [option.model for option in options] - assert models == ["llama3.2:latest", "qwen2.5-coder:7b"] + model_names = [option.model for option in options] + assert model_names == ["llama3.2:latest", "qwen2.5-coder:7b"] llama = options[0] assert llama.source == "local" assert llama.notes == "llama, 3B" assert llama.reasoning_modes == ("auto",) + + +async def _assert_generate_wraps(exc: Exception) -> None: + """Drive ``generate`` with a client that raises ``exc`` and assert the + error is caught and re-raised as a provider-tagged ``ProviderError``. + + Shared by the #445 regression cases below: only ``APIStatusError`` used to + be caught, so timeout/connection errors escaped uncaught, skipped tenacity + (which retries ``ProviderError`` only), and surfaced as + ``page_generation_failed``. + """ + provider = OllamaProvider(model="test", base_url="http://localhost:9999") + provider._rate_limiter = None + provider._client.chat.completions.create = AsyncMock(side_effect=exc) + with pytest.raises(ProviderError) as exc_info: + await provider.generate(system_prompt="", user_prompt="", max_tokens=10) + assert "ollama" in str(exc_info.value) + + +def _status_error() -> APIStatusError: + resp = AsyncMock() + resp.status_code = 500 + return APIStatusError("Internal Server Error", response=resp, body="") + + +def _connection_error() -> APIConnectionError: + request = httpx.Request("POST", "http://localhost:9999/v1/chat/completions") + return APIConnectionError(message="Connection refused", request=request) + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "make_exc", + [ + pytest.param(_status_error, id="api_status_error"), + pytest.param(lambda: APITimeoutError("Request timed out"), id="api_timeout_error"), + pytest.param(_connection_error, id="api_connection_error"), + ], +) +async def test_generate_wraps_openai_errors(make_exc): + """Every ``openai.APIError`` subclass is wrapped as ``ProviderError`` (#445). + + ``APITimeoutError`` and ``APIConnectionError`` are NOT subclasses of + ``APIStatusError``, so the old catch-only-``APIStatusError`` clause let them + escape uncaught, skipping tenacity's retry loop entirely. + """ + await _assert_generate_wraps(make_exc()) + + +@pytest.mark.asyncio +async def test_all_openai_errors_are_subclasses_of_api_error(): + """Verify the class hierarchy: all OpenAI errors inherit from APIError.""" + resp = AsyncMock() + resp.status_code = 500 + status_err = APIStatusError("err", response=resp, body="") + timeout_err = APITimeoutError("timed out") + request = httpx.Request("POST", "http://localhost:9999/v1/chat/completions") + conn_err = APIConnectionError(message="refused", request=request) + + assert isinstance(status_err, APIError), "APIStatusError must be APIError" + assert isinstance(timeout_err, APIError), "APITimeoutError must be APIError" + assert isinstance(conn_err, APIError), "APIConnectionError must be APIError" diff --git a/tests/unit/test_providers/test_ollama_provider_timeout_integration.py b/tests/unit/test_providers/test_ollama_provider_timeout_integration.py new file mode 100644 index 000000000..87e634338 --- /dev/null +++ b/tests/unit/test_providers/test_ollama_provider_timeout_integration.py @@ -0,0 +1,201 @@ +"""Integration test for Issue #445: APITimeoutError retry behavior. + +Tests the full chain: OllamaProvider._generate_with_retry → tenacity retry +→ generate(). Verifies that APITimeoutError is wrapped as ProviderError +and that tenacity actually retries (not just catches). +""" + +from __future__ import annotations + +from unittest.mock import AsyncMock + +import httpx +import pytest + +pytest.importorskip("openai", reason="openai SDK not installed") + +from openai import ( + APIConnectionError, + APIStatusError, + APITimeoutError, + AuthenticationError, +) + +from repowise.core.providers.llm.base import ProviderError +from repowise.core.providers.llm.ollama import OllamaProvider + + +@pytest.mark.asyncio +async def test_ollama_retries_on_timeout(): + """Verify tenacity retries N times before giving up on APITimeoutError. + + With the fix, APITimeoutError gets caught, wrapped as ProviderError, + and retried by tenacity up to _MAX_RETRIES (3) times. + """ + p = OllamaProvider(model="test-model", base_url="http://localhost:9999") + p._rate_limiter = None + + # Track how many times the client is called + call_count = 0 + + async def timeout_side_effect(*args, **kwargs): + nonlocal call_count + call_count += 1 + raise APITimeoutError(f"Request timed out (attempt {call_count})") + + p._client.chat.completions.create = AsyncMock(side_effect=timeout_side_effect) + + with pytest.raises(ProviderError) as exc_info: + await p.generate(system_prompt="", user_prompt="", max_tokens=10) + + # Tenacity should have retried _MAX_RETRIES times (= 3) + assert call_count == 3, ( + f"Expected 3 retries before ProviderError, got {call_count}. Fix may not be working." + ) + assert "ollama" in str(exc_info.value) + + +@pytest.mark.asyncio +async def test_ollama_retries_on_connection_error(): + """Same as above but with APIConnectionError.""" + p = OllamaProvider(model="test-model", base_url="http://localhost:9999") + p._rate_limiter = None + + call_count = 0 + + async def conn_error_side_effect(*args, **kwargs): + nonlocal call_count + call_count += 1 + request = httpx.Request("POST", "http://localhost:9999/v1/chat/completions") + raise APIConnectionError( + message=f"Connection refused (attempt {call_count})", + request=request, + ) + + p._client.chat.completions.create = AsyncMock(side_effect=conn_error_side_effect) + + with pytest.raises(ProviderError) as exc_info: + await p.generate(system_prompt="", user_prompt="", max_tokens=10) + + assert call_count == 3, ( + f"Expected 3 retries before ProviderError, got {call_count}. Fix may not be working." + ) + assert "ollama" in str(exc_info.value) + + +@pytest.mark.asyncio +async def test_ollama_succeeds_on_retry(): + """Verify that after a timeout failure, the retry succeeds and returns a result. + + This mimics the real-world scenario: Ollama is overloaded, first request + times out, but the retry succeeds. + """ + from openai.types.chat import ChatCompletion, ChatCompletionMessage + from openai.types.chat.chat_completion import Choice + from openai.types.completion_usage import CompletionUsage + + p = OllamaProvider(model="test-model", base_url="http://localhost:9999") + p._rate_limiter = None + + call_count = 0 + + async def flaky_side_effect(*args, **kwargs): + nonlocal call_count + call_count += 1 + if call_count == 1: + raise APITimeoutError("First attempt timed out") + # Second attempt succeeds + return ChatCompletion( + id="test", + model="test-model", + object="chat.completion", + created=1234567890, + choices=[ + Choice( + finish_reason="stop", + index=0, + message=ChatCompletionMessage( + role="assistant", + content="Generated wiki content", + ), + ) + ], + usage=CompletionUsage( + prompt_tokens=10, + completion_tokens=20, + total_tokens=30, + ), + ) + + p._client.chat.completions.create = AsyncMock(side_effect=flaky_side_effect) + + result = await p.generate(system_prompt="", user_prompt="Test", max_tokens=10) + + assert call_count == 2, f"Expected 2 calls (1 fail + 1 retry), got {call_count}" + assert result.content == "Generated wiki content" + assert result.input_tokens == 10 + assert result.output_tokens == 20 + + +@pytest.mark.asyncio +async def test_ollama_does_not_retry_invalid_request(): + """Verify 400 (invalid request) fails immediately without retries. + + Issue #445 point 3: non-retryable errors must fail fast. The shared + retry policy treats 400/401/403/404/405/413/422 as non-retryable, so + the client is called exactly once. + """ + p = OllamaProvider(model="test-model", base_url="http://localhost:9999") + p._rate_limiter = None + + call_count = 0 + + async def bad_request_side_effect(*args, **kwargs): + nonlocal call_count + call_count += 1 + resp = httpx.Response( + 400, + request=httpx.Request("POST", "http://localhost:9999/v1/chat/completions"), + ) + raise APIStatusError("Bad Request", response=resp, body=None) + + p._client.chat.completions.create = AsyncMock(side_effect=bad_request_side_effect) + + with pytest.raises(ProviderError) as exc_info: + await p.generate(system_prompt="", user_prompt="", max_tokens=10) + + assert call_count == 1, ( + f"Non-retryable 400 must fail immediately, but was called {call_count} times." + ) + assert "ollama" in str(exc_info.value) + + +@pytest.mark.asyncio +async def test_ollama_does_not_retry_authentication_error(): + """Verify 401 (authentication) fails immediately without retries. + + Issue #445 point 3: authentication errors must not burn retry attempts. + """ + p = OllamaProvider(model="test-model", base_url="http://localhost:9999") + p._rate_limiter = None + + call_count = 0 + + async def auth_error_side_effect(*args, **kwargs): + nonlocal call_count + call_count += 1 + resp = httpx.Response( + 401, + request=httpx.Request("POST", "http://localhost:9999/v1/chat/completions"), + ) + raise AuthenticationError("Unauthorized", response=resp, body=None) + + p._client.chat.completions.create = AsyncMock(side_effect=auth_error_side_effect) + + with pytest.raises(ProviderError) as exc_info: + await p.generate(system_prompt="", user_prompt="", max_tokens=10) + + assert call_count == 1, ( + f"Authentication error must fail immediately, but was called {call_count} times." + ) + assert "ollama" in str(exc_info.value)