docs(core): clarify Model.model_name is the upstream id (#302 / AISIX-Cloud#470) - #384
Conversation
#302 / AISIX-Cloud#470) The Model struct has two identifiers — `display_name` (customer-facing alias the chat client sends) and `model_name` (upstream id the LLM API expects). The field NAMES alone do not make this obvious, and some other proxy gateways in the ecosystem use `model_name` to mean the opposite — i.e. the alias, with a separate `model` field for the upstream id. A reader who knows that convention from a neighbour project will get this backwards on first read. Renaming the field to `upstream_id` to remove the ambiguity is tracked at api7/AISIX-Cloud#470, but it's a coordinated wire-format change across ai-gateway + cp-api + dashboard + ~110 test fixtures + PG column. Not justified by the cost-vs-benefit today (see #470 audit discussion). This commit takes the cheap-and-correct path: a 14-line doc comment on `Model.model_name` that explicitly names the convention reversal so the next reader doesn't get tripped up. Zero behaviour change, zero wire change, no test impact. `cargo check -p aisix-core` clean. `cargo fmt` applied.
|
Warning Review limit reached
Your plan currently allows 3 reviews/hour. Refill in 14 minutes and 25 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR expands documentation for the ChangesModel field documentation clarification
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates aisix-core documentation to clarify the semantic difference between Model.display_name (customer-facing alias used on gateway APIs) and Model.model_name (the upstream/provider-facing identifier), explicitly warning about the common convention mismatch with other gateways.
Changes:
- Expanded the rustdoc comment on
Model.model_nameto describe its role and common confusion with other projects. - Added a NOTE explaining the “footgun” naming reversal vs other gateways and linked the longer-term rename tracking issue.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Upstream model id sent to the provider — the literal string | ||
| /// the upstream LLM API expects in its `model` field | ||
| /// (e.g. `"gpt-4o"`, `"claude-sonnet-4-5"`, | ||
| /// `"gpt-4o-mini-2024-08-06"`). `None` for routing models. |
…c expansion The JsonSchema derive macro emits Rust doc comments as the schema's `description` field. The longer doc on `Model.model_name` (previous commit) expanded the schema's description string; the CI `schema drift (resources)` job caught it and asked for the regenerated file to be committed. `cargo run -p aisix-core --bin dump-schema` regenerated all 8 resource schemas; only `model.schema.json` actually changed (the others are unchanged because no other resource's docs were touched).
Summary
The `Model` struct has two identifiers:
The field names alone don't make this obvious. Some other proxy gateways in the ecosystem use the name `model_name` to mean the opposite — i.e. the customer-facing alias — with a separate `model` sub-field for the upstream id. A reader who knows that convention from a neighbour project will get the AISIX struct backwards on first read.
This PR is a 14-line doc comment on `Model.model_name` (`crates/aisix-core/src/models/model.rs:220`) that explicitly calls out the convention reversal. Zero behaviour change, zero wire change, no test impact.
Why not just rename it?
Renaming `model_name` → `upstream_id` to remove the ambiguity is tracked at api7/AISIX-Cloud#470. It's a coordinated wire-format change that touches ai-gateway + cp-api + dashboard + ~110 test fixtures + PG column. Cost-vs-benefit isn't there today — the field is internal-only (no customer-facing surface), and a doc comment captures 80% of the clarity benefit at 1% of the migration cost.
The rename can ride along the next big wire revision; until then, the comment carries the disambiguation.
Test plan
Closes the doc-clarity half of the audit finding on #302 A2/B8/M19 (Model rename). cp-api side has a companion 1-line doc comment landing as a separate PR in api7/AISIX-Cloud.
Summary by CodeRabbit