refactor(llm-access): extract provider.rs tests into provider/tests.rs (PR-A)#19
Merged
Merged
Conversation
First of two PRs splitting the 14.3k-line provider.rs (the largest file in the sequence). This PR-A is the mechanical, near-zero-risk half: move the single `mod tests` block (106 tests, ~7000 lines) into `provider/tests.rs` via `#[cfg(test)] mod tests;`. PR-B will domain-split the remaining ~7300 lines of code into submodules. `provider.rs`: 14326 -> 7301 lines. The module was already a separate `mod tests` accessing parents via `super::`, so moving it to a file changes nothing about name resolution. The `#[cfg(test)]` + `#[allow(clippy::await_holding_lock, reason=...)]` attributes ride on the `mod tests;` decl (outer attrs apply to the file's contents). Behavior-preserving: extracted the body verbatim (no hand-dedent) and let rustfmt normalize it, so raw-string literal interiors (e.g. the `format!` JSON templates) stay byte-for-byte identical to master rather than being shifted by a manual dedent. Verified: identical fn-name set vs pre-split, same 106 test attrs, rustfmt-idempotent. Verification: - cargo clippy -p llm-access --all-targets -- -D warnings -> clean - cargo test -p llm-access provider:: -> 111 passed - rustfmt on the 2 changed files only; deps/lance, deps/lancedb untouched Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
Owner
Author
|
/gemini review |
Owner
Author
|
@codex review |
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
acking-you
added a commit
that referenced
this pull request
May 31, 2026
#20) Second of two PRs splitting the 14.3k-line provider.rs (PR-A #19 extracted the 106 tests; this PR-B domain-splits the remaining ~7300 code lines). Free-fn DAG shape (like the duckdb split), already mod.rs-style. provider.rs: 7301 -> 603 lines. 182 free fns + 16 impls (63 methods) move into 19 concern submodules: usage_meta, state, limiter, route_selection, codex_dispatch, kiro_media, kiro_dispatch, stream_guards, kiro_protocol, client, errors, kiro_model, kiro_usage, entry, codex_auth, codex_sse, kiro_summary, util, codex_models. Behavior-preserving structural move: identical top-level name multiset (336), same 106 tests. Fat facade keeps all 47 types + 2 traits + 24 consts/statics (submodules read private fields as ancestor-privates, no field-vis change). Moved free fns -> pub only where cross-region referenced (102/182); inherent methods -> pub(super) only where a foreign region calls them (25/46); trait-impl methods never bumped. Facade re-imports in 3 categories: external API preserved via pub/pub(crate) use (provider_entry*, codex_upstream_base_url, compute_codex_upstream_url, resolve_codex_client_version, codex_public_model_catalog_response, default_codex_public_model_catalog_response, call_kiro_generate_for_route, decode_kiro_events_from_bytes); unconditional `use` for fns the facade's LazyLock statics call (build_provider_client + client-pool tunables); and `#[cfg(test)] use` for fns/types tests reach via super:: (incl. axum Method). Named the kiro-header module `kiro_protocol` to avoid colliding with the `use crate::kiro_headers` binding (E0255). Pre-existing kiro_error.rs calls super::{anthropic_json_error, summarize_error_bytes} -> facade re-imports them so those super:: paths still resolve. Verification: - cargo clippy -p llm-access --all-targets -- -D warnings -> clean - cargo test -p llm-access provider:: -> 111 passed - rustfmt on the 20 changed files only; deps/lance, deps/lancedb untouched Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
First of two PRs splitting the 14326-line
llm-access/src/provider.rs— the largest file in thellm-access*refactor sequence and the only one in the production binary crate. This PR-A is the mechanical, near-zero-risk half: move the singlemod testsblock (106 tests, ~7000 lines) intoprovider/tests.rsvia#[cfg(test)] mod tests;.provider.rs: 14326 → 7301 lines. PR-B will then domain-split the remaining ~7300 lines of code into concern submodules (free-fn DAG shape, like the duckdb split #16).Why this is safe
mod testsaccessing parents viasuper::, so relocating it to a file changes nothing about name resolution.provider.rsis already amod.rs-style facade (it declaresmod kiro_error; mod kiro_session_affinity;with aprovider/dir present), so adding a submodule file is the established local convention.#[cfg(test)]+#[allow(clippy::await_holding_lock, reason=…)]attributes ride on themod tests;decl (outer attrs apply to the file's contents).Behavior-preserving — and a note on how
Extracted the body verbatim (no hand-dedent) and let rustfmt normalize it. This matters: a naive dedent-by-4 would shift the interiors of multi-line raw-string literals (e.g. the
format!(r#"{{…}}"#)JSON templates) — JSON-benign but a real byte change to test data. Verbatim→rustfmt leaves raw-string interiors byte-for-byte identical to master (rustfmt reindents code, never literal contents) while collapsing the now-fits-in-100-cols wraps at the shallower indent.Verified:
format!JSON-template interiors confirmed unchanged (16-space indent, as on master)Verification
cargo clippy -p llm-access --all-targets -- -D warnings→ cleancargo test -p llm-access provider::→ 111 passedrustfmton the 2 changed files only;deps/lance/deps/lancedbuntouched🤖 Generated with Claude Code