refactor: provider HTTP dedup + fix context length false-positive

[Miyamura80]

Summary

• Provider HTTP dedup: Extracted ~95% duplicated HTTP logic from openai.rs and custom.rs into a shared http_base.rs HttpProvider. Both providers are now thin constructor wrappers (~500 lines removed, net -125 lines).
• Context length error fix: Removed overly broad "max_tokens" pattern from is_context_length_error() that was misclassifying parameter validation errors (e.g., "max_tokens must be less than X") as context length errors, triggering unnecessary trajectory-clearing fallbacks. Flagged by Sentry in PR refactor: split large modules into focused submodules #16.

Test plan

• All 372 tests pass (cargo test)
• Provider tests (47) verify HTTP request/response, error handling, tool calls, and trait dispatch
• Context tests (23) verify the false-positive fix with a new regression test case

🤖 Generated with Claude Code

---

[Miyamura80]

@greptileai

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

[greptile-apps]

Greptile Summary

This PR delivers two targeted improvements: it extracts shared HTTP logic from openai.rs and custom.rs into a new http_base.rs HttpProvider (both providers are now thin factory wrappers), and it removes the overly broad "max_tokens" string match from is_context_length_error() that was causing parameter-validation errors to be misclassified as context-length errors and triggering unnecessary trajectory resets.

Key changes:

• src/provider/http_base.rs — new shared HttpProvider struct implementing LlmProvider, parameterised by base_url and a label for logging; includes a full suite of wiremock-based tests.
• src/provider/openai.rs / src/provider/custom.rs — both reduced to empty structs with a single new() factory method that delegates to HttpProvider::new.
• src/provider/mod.rs — exposes the new http_base module and adds a minor explicit type cast in the "openai" branch that is inconsistent with the "custom" and "anthropic" branches (all three could use the implicit coercion).
• src/agent/openai.rs — OpenAiClient re-export now points at HttpProvider directly, but the module doc comment still refers to crate::provider::openai as the implementation home.
• src/agent/context.rs — removes the "max_tokens" false-positive pattern and adds a regression test asserting "max_tokens must be less than 128000" is not classified as a context-length error.

Confidence Score: 4/5

• This PR is safe to merge; the refactoring is mechanically sound, tests pass, and the context-length fix is well-targeted.
• The HTTP deduplication is clean — both providers now delegate to a single HttpProvider with no behaviour change. The context-length false-positive fix is a one-line removal backed by a regression test. The only issues are cosmetic: a stale module doc comment in src/agent/openai.rs and an inconsistent explicit type cast in src/provider/mod.rs.
• src/agent/openai.rs (stale doc comment) and src/provider/mod.rs (inconsistent type cast)

Important Files Changed

Filename	Overview
src/provider/http_base.rs	New shared HTTP provider consolidating duplicated logic from openai.rs and custom.rs; well-structured with a good test suite covering text, tool-call, error, and trait-dispatch paths.
src/agent/openai.rs	Re-exports updated to point at HttpProvider instead of OpenAiProvider, but the module doc comment still references the old crate::provider::openai location.
src/provider/openai.rs	Reduced to a thin factory wrapper; OpenAiProvider::new now returns HttpProvider directly, keeping the public API stable.
src/provider/custom.rs	Reduced to a thin factory wrapper delegating to HttpProvider; CustomProvider::new now returns HttpProvider, preserving the call-site API in create_provider.
src/provider/mod.rs	Adds http_base module export and an unnecessary explicit type cast in the openai branch that is inconsistent with the other provider branches.
src/agent/context.rs	Removes the overly broad "max_tokens" pattern from is_context_length_error() and adds a targeted regression test for the false-positive case.

Class Diagram

%%{init: {'theme': 'neutral'}}%%
classDiagram
    class LlmProvider {
        <<trait>>
        +chat_completion(messages, tools) Future~ChatMessage~
    }

    class HttpProvider {
        -http: reqwest::Client
        -api_key: String
        -model: String
        -base_url: String
        -label: String
        +new(api_key, model, base_url, label) HttpProvider
        +with_base_url(url) HttpProvider
        +completions_url() String
        +chat_completion(messages, tools) Future~ChatMessage~
    }

    class OpenAiProvider {
        <<factory>>
        +new(api_key, model) HttpProvider
    }

    class CustomProvider {
        <<factory>>
        +new(api_key, model, base_url) HttpProvider
    }

    class AnthropicProvider {
        -http: reqwest::Client
        -api_key: String
        -model: String
        -base_url: String
        +new(api_key, model) AnthropicProvider
        +with_base_url(url) AnthropicProvider
        +chat_completion(messages, tools) Future~ChatMessage~
    }

    LlmProvider <|.. HttpProvider : implements
    LlmProvider <|.. AnthropicProvider : implements
    OpenAiProvider ..> HttpProvider : creates
    CustomProvider ..> HttpProvider : creates

Loading

Comments Outside Diff (1)

1. src/agent/openai.rs, line 2-5 (link)

   Stale module doc comment

   The doc comment still says "The LLM provider implementation has moved to crate::provider::openai", but after this refactoring the actual implementation now lives in crate::provider::http_base. A developer following this comment to trace OpenAiClient's implementation would land in openai.rs, find an empty factory struct, and have to hunt further.

_{Last reviewed commit: "refactor: extract sh..."}