docs(rust): add provider abstraction standards - #33865
Conversation
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
Greptile SummaryThis documentation-only PR adds provider abstraction standards to two existing developer-guide files in the
Confidence Score: 4/5Documentation-only change with no production code impact; safe to merge after considering the cross-language path references. Both files receive identical guidance prose, and the Rust-specific CLAUDE.md now points contributors at Python source paths that cannot be directly consumed from Rust. Neither issue breaks anything at runtime, but the duplication creates a maintenance hazard and the Python path references in a Rust-only guide may mislead contributors. litellm-rust/CLAUDE.md — the Python path references in a Rust-only guide deserve a second look.
|
| Filename | Overview |
|---|---|
| litellm-rust/ADDING_A_PROVIDER.md | Adds a "Coding standards" section with provider abstraction guidance; section is an exact copy of what's also added to CLAUDE.md, creating a duplication/maintenance burden. |
| litellm-rust/CLAUDE.md | Adds a "Provider Coding Standards" section; references Python-specific paths (litellm/llms/base_llm/, litellm_core_utils/) that are not directly usable from Rust code, which may confuse contributors working in this Rust-only file. |
Reviews (1): Last reviewed commit: "docs(rust): add provider abstraction sta..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ## Coding standards | ||
|
|
||
| Before writing new logic, look for an existing base to extend. When a change is | ||
| “the same behavior for one more provider/endpoint/integration”, the codebase | ||
| almost always already has a shared abstraction for it (for example, provider | ||
| `BaseConfig` transformation classes in `litellm/llms/base_llm/`, shared | ||
| helpers in `litellm_core_utils/`, typed request/response models, or factory | ||
| functions). Find it first with a search, then add the new variant by inheriting | ||
| from or composing that base, overriding only what genuinely differs (model | ||
| name, parameter mapping, or auth). | ||
|
|
||
| Never copy an existing implementation and edit it in place, and never hand-roll | ||
| a parallel version of logic a base already provides. If you catch yourself | ||
| writing a second copy of a pattern that exists twice already, stop and extract a | ||
| base instead: put the shared shape in one place and make both call sites thin | ||
| variants of it. The test for a good abstraction is that adding the next provider | ||
| is a few declarative lines, not a new file of duplicated flow. Only diverge from | ||
| the base when behavior is genuinely different, and say so explicitly in the PR. |
There was a problem hiding this comment.
The "Coding standards" section added here is character-for-character identical to the "Provider Coding Standards" block added to CLAUDE.md. If this guidance ever needs to change, both files must be updated in sync, and they will inevitably drift. Consider either keeping the canonical text in one file and having the other reference it (e.g., "See CLAUDE.md for provider coding standards"), or consolidating the two files so there is a single source of truth.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| ## Provider Coding Standards | ||
|
|
||
| Before writing new logic, look for an existing base to extend. When a change is | ||
| “the same behavior for one more provider/endpoint/integration”, the codebase | ||
| almost always already has a shared abstraction for it (for example, provider | ||
| `BaseConfig` transformation classes in `litellm/llms/base_llm/`, shared | ||
| helpers in `litellm_core_utils/`, typed request/response models, or factory | ||
| functions). Find it first with a search, then add the new variant by inheriting | ||
| from or composing that base, overriding only what genuinely differs (model | ||
| name, parameter mapping, or auth). | ||
|
|
||
| Never copy an existing implementation and edit it in place, and never hand-roll | ||
| a parallel version of logic a base already provides. If you catch yourself | ||
| writing a second copy of a pattern that exists twice already, stop and extract a | ||
| base instead: put the shared shape in one place and make both call sites thin | ||
| variants of it. The test for a good abstraction is that adding the next provider | ||
| is a few declarative lines, not a new file of duplicated flow. Only diverge from | ||
| the base when behavior is genuinely different, and say so explicitly in the PR. |
There was a problem hiding this comment.
The guidance references litellm/llms/base_llm/ and litellm_core_utils/ — Python filesystem paths — inside CLAUDE.md, which is declared as the guide for Rust work. A Rust contributor searching for BaseConfig in those paths will find Python classes that cannot be inherited or composed from Rust. It would be clearer to either state these paths as conceptual models to mirror (not reuse directly), or point to the corresponding Rust equivalents (e.g., the traits in crates/core/src/<route>/transformation.rs).
2 participants
Relevant issues
Linear ticket
Pre-Submission checklist
Screenshots / Proof of Fix
Documentation-only change; no runtime proof is applicable
Type
📖 Documentation
Changes
Added the provider abstraction guidance to
litellm-rust/CLAUDE.mdand the provider onboarding guide. The guidance directs contributors to search for and extend shared bases, avoid duplicated provider flows, and document intentional deviations in the PRFinal Attestation
Link to Devin session: https://app.devin.ai/sessions/5c46bf2e7662475aa32eb04f5a6f8aa4
Requested by: @ishaan-berri