alishahryar1--free-claude-code
Ali Khokhar
f3ea35a777
## Problem Sixteen OpenAI-compatible providers were represented by configuration-only subclasses and one factory function each. The sole provider family also lived under a multi-transport namespace that no longer described the codebase, while provider defaults, IDs, and instance caching passed through redundant forwarding layers. ## Changes | Before | After | | --- | --- | | Sixteen provider IDs used configuration-only subclasses. | Immutable OpenAI-chat profiles configure one concrete provider while preserving each provider's request policy. | | Every provider required a dedicated factory function. | Generic profile construction is the default; only eight adapters with real state or algorithms retain factories. | | The sole provider family lived under `providers/transports/`. | `providers/openai_chat/` directly owns shared request, stream, recovery, tool, and usage behavior. | | Provider defaults and IDs passed through forwarding modules, and constructors repeated fallback resolution. | The neutral catalog resolves complete immutable provider configuration once. | | `ProviderRuntime` wrapped a pass-through provider cache. | `ProviderRuntime` directly owns lazy provider instances and cleanup. | | Production carried the duplicated adapter structure. | The final shape removes a net 873 production lines with no compatibility shim or customer-facing provider change. | <!-- greptile_comment --> <details open><summary><h3>Greptile Summary</h3></summary> This PR consolidates OpenAI-compatible providers behind shared profiles and runtime construction. The main changes are: - OpenAI-chat behavior moved into `providers/openai_chat`. - Configuration-only provider subclasses replaced by immutable profiles. - Provider runtime now owns lazy instance caching and cleanup directly. - Provider defaults and IDs now resolve through the neutral catalog. - Tests and smoke helpers updated for the new provider shape. </details> <h3>Confidence Score: 5/5</h3> This looks safe to merge. No blocking issues found in the changed code. No files need attention. <details><summary><h3><a href="https://www.greptile.com/trex"><img alt="T-Rex" src="https://greptile-static-assets.s3.amazonaws.com/trex/trex_green.svg" height="20" align="absmiddle"></a> T-Rex Logs</h3></summary> **What T-Rex did** - T-Rex produced a proof for a posted P1 finding and linked it to the corresponding review comment for details. - T-Rex saved contract-validation logs for provider consolidation and preserved four log files that show the test hanging boundary and the timeout exit code 124. <a href="https://app.greptile.com/trex/runs/14125428/artifacts"><picture><source media="(prefers-color-scheme: dark)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/ViewAllArtifactsDark.svg?v=4"><source media="(prefers-color-scheme: light)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/ViewAllArtifacts.svg?v=4"><img alt="View all artifacts" src="https://greptile-static-assets.s3.amazonaws.com/badges/ViewAllArtifacts.svg?v=4"></picture></a> <sub><a href="https://www.greptile.com/trex"><img alt="T-Rex" src="https://greptile-static-assets.s3.amazonaws.com/trex/trex_green.svg" height="14" align="absmiddle"></a> Ran code and verified through T-Rex</sub> </details> <details open><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | src/free_claude_code/providers/runtime/config.py | Builds resolved provider configuration from catalog descriptors, including static credentials for local providers. | | src/free_claude_code/providers/openai_chat/provider.py | Creates shared OpenAI-chat clients from resolved configuration and immutable provider profiles. | | src/free_claude_code/providers/openai_chat/profiles.py | Defines declarative profiles for formerly configuration-only OpenAI-compatible adapters. | </details> <!-- greptile_failed_comments --> <h3>Comments Outside Diff (2)</h3> 1. General comment <a href="#"><img alt="P1" src="https://greptile-static-assets.s3.amazonaws.com/badges/p1.svg?v=9" align="top"></a> **NIM OpenAI-compatible 504 exhausted streaming retry path hangs instead of completing with the expected user-facing error** - **Bug** - The broader provider consolidation suite failed all parametrized `test_nim_stream_openai_5xx_exhausted_emits_user_message` cases under xdist with worker crashes. A serial isolation run narrowed this to the same OpenAI-compatible NIM exhausted 5xx streaming path: 500, 502, and 503 completed, but the 504 case did not finish before the explicit 120s timeout, producing exit code 124. This contradicts the expected streaming retry contract that exhausted transient provider failures terminate and emit the configured user-facing error. - **Cause** - The OpenAI-compatible streaming retry/exhaustion path for NIM 504 responses appears to wait indefinitely or otherwise fail to terminate after retries are exhausted. The exact code-level loop/await point was not isolated within the validation budget, but the failure is anchored to the provider streaming retry contract exercised by `tests/providers/test_openai_compat_5xx_retry.py::test_nim_stream_openai_5xx_exhausted_emits_user_message[504-temporarily unavailable]`. - **Fix** - Inspect the OpenAI-compatible/NIM streaming retry exhaustion handling for 504 responses and ensure retry limits are enforced, the async stream is closed/cancelled on exhaustion, and the provider raises/emits the same terminal user-facing error contract as the 500/502/503 cases. Add or keep a serial regression test for the 504 exhausted stream path to prevent hangs. <sub><a href="https://www.greptile.com/trex"><img alt="T-Rex" src="https://greptile-static-assets.s3.amazonaws.com/trex/trex_green.svg" height="14" align="absmiddle"></a> Ran code and verified through T-Rex</sub> 2. General comment <a href="#"><img alt="P1" src="https://greptile-static-assets.s3.amazonaws.com/badges/p1.svg?v=9" align="top"></a> **OpenAI-compatible 5xx retry suite hangs on exhausted 502 retry path** - **Bug** - The recommended non-xdist retry validation does not complete. A verbose rerun with a 60 second timeout shows the suite passes tests through the exhausted 500 case, then times out while running `test_nim_stream_openai_5xx_exhausted_emits_user_message[502-temporarily unavailable]`. This indicates the consolidated OpenAI-compatible provider retry/error path can hang for at least the 502 exhausted-error scenario, preventing reliable validation and potentially blocking callers from receiving the expected `ExecutionFailure`. - **Cause** - The OpenAI-compatible provider's exhausted 502 retry/error handling path appears not to terminate promptly under the mocked repeated `openai.InternalServerError` scenario. The exact code location was not changed during validation, but the failure is isolated to the shared OpenAI-chat 5xx retry behavior exercised by `NvidiaNimProvider.stream_response`. - **Fix** - Debug the shared OpenAI-chat retry loop/error classification for 502 responses. Ensure retries are bounded, patched `asyncio.sleep` is awaited without real backoff during tests, and exhausted 502/503/504 errors consistently raise `ExecutionFailure` with the expected temporary-unavailable message instead of continuing work indefinitely. <sub><a href="https://www.greptile.com/trex"><img alt="T-Rex" src="https://greptile-static-assets.s3.amazonaws.com/trex/trex_green.svg" height="14" align="absmiddle"></a> Ran code and verified through T-Rex</sub> <!-- /greptile_failed_comments --> <sub>Reviews (2): Last reviewed commit: ["Prove local provider credential resoluti..."](https://github.com/alishahryar1/free-claude-code/commit/ba226d97efdae1889bea0372c7bacf268122d65c) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=43580983)</sub> <!-- /greptile_comment -->
26 行
693 B
Python
26 行
693 B
Python
"""Shared MagicMock request objects for OpenAI-compatible provider tests."""
|
|
|
|
from unittest.mock import MagicMock
|
|
|
|
|
|
def make_openai_compat_stream_request(
|
|
*, model: str = "test-model", stream: bool = True
|
|
) -> MagicMock:
|
|
"""Minimal request stub matching :meth:`OpenAIChatProvider._build_request_body` needs."""
|
|
req = MagicMock()
|
|
req.model = model
|
|
req.stream = stream
|
|
req.messages = []
|
|
req.system = None
|
|
req.tools = None
|
|
req.tool_choice = None
|
|
req.metadata = None
|
|
req.max_tokens = 4096
|
|
req.temperature = None
|
|
req.top_p = None
|
|
req.top_k = None
|
|
req.stop_sequences = None
|
|
req.extra_body = None
|
|
req.thinking = None
|
|
return req
|