Uh oh!
There was an error while loading. Please reload this page.
docs: correct configuration reference inaccuracies across admin-api, api-keys, caching, and models pages - #348
Merged
moonming merged 1 commit intoMay 19, 2026
Conversation
…api-keys, caching, and models pages Closes#347. Five corrections, each anchored to a runtime consumer (or non-consumer) verified on `origin/main` at `71ea97e`. The cluster mirrors the consumer-trace discipline locked in by #326 / #344: for every behavioral claim, verify the runtime consumer actually reads the documented value, rather than only verifying the field exists with a default. - admin-api.md:76 — group label "health" → "livez". The public route is `/livez` (registered at crates/aisix-admin/src/lib.rs:67); the `/admin/v1/health` endpoint requires admin auth (registered at :145). - api-keys.md:86 — rotate response example: 16-char suffix replaced with a 32-char suffix. Rotate handler at crates/aisix-admin/src/apikeys_handlers.rs:168 uses `Uuid::new_v4().as_simple()`, which produces 32 hex chars. - caching.md — explicit note that `CachePolicy.backend` is parsed and stored on each row but not consulted at runtime. No consumer of `entry.value.backend` / `policy.backend` / `cache_policy.backend` exists in `crates/`. The proxy uses the bootstrap-config (`cache.backend`) instead. - models.md:80 — `ignore_statuses` default. Field at crates/aisix-core/src/models/model.rs:259-260 (drifted from :159-160 cited in the issue body) is declared `#[serde(default, skip_serializing_if = "Vec::is_empty")] pub ignore_statuses: Vec<u16>`, so the actual default is the empty vector — not `[408, 429]` as the doc claimed. Re-presented `[408, 429]` as a recommended explicit setting. Consumer at crates/aisix-proxy/src/background.rs:84,88 calls `cfg.ignore_statuses.contains(...)` against the empty default. - models.md:163 — `Model.cost` consumer split. Standalone OSS proxy hard-codes `cost_usd = 0.0` at crates/aisix-proxy/src/chat.rs:989 (with comment "cp-api recomputes cost server-side from its pricing catalog"). The `Model.cost` field is consumed by AISIX Cloud's cp-api, not by the OSS proxy.
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR updates four configuration documentation files to clarify operator-facing behavior: public API routes, API key examples, cache backend runtime semantics, and model health check and pricing behavior for both AISIX Cloud and OSS deployments. ChangesConfiguration Documentation Clarifications
🎯 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 doc-only PR corrects several inaccuracies in the configuration reference docs to better reflect actual runtime behavior and response shapes across the admin API, API keys, caching, and models documentation.
Changes:
- Rename the public operator helper label from
healthtolivezto reflect the unauthenticated/livezroute. - Update the API key rotation example to show a 32-hex UUID suffix consistent with
Uuid::new_v4().as_simple(). - Clarify runtime boundaries for
CachePolicy.backend,background_model_check.ignore_statusesdefaults, andModel.costconsumption.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| docs/configuration/admin-api.md | Corrects the public helper group label to livez. |
| docs/configuration/api-keys.md | Fixes rotate response example to show a 32-char key suffix. |
| docs/configuration/caching.md | Adds an explicit note that CachePolicy.backend is currently parsed but not consulted by the proxy runtime. |
| docs/configuration/models.md | Corrects ignore_statuses default behavior and clarifies cost handling expectations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -160,7 +160,7 @@ curl -sS -X POST http://127.0.0.1:3001/admin/v1/models \ | |||
| - `provider` currently supports `openai`, `anthropic`, `google`, `deepseek`, `cohere`, and `jina`. | |||
Uh oh!
There was an error while loading. Please reload this page.
janiussyafiq added a commit
that referenced
this pull request
May 20, 2026
Integrate origin/main (commit 2c1d485 = post-PR-#326 / #348 plus #330 / #341 / #343 / #345 / #346) into this branch via `git merge --squash` to clear PR #344's lingering `mergeable: dirty` state. Conflict on `docs/quickstart/self-hosted.md` was a 3-way-merge-base artifact: base (3596c0a) read `- a reachable etcd instance`, main changed `a` → `A` (via #326), this branch additionally inserted the glossary link. Both changes are wanted; resolution per Umar's approved plan was `git checkout --ours`, which preserves the branch's self-hosted.md state (already integrates capital A + glossary link + first-time-build paragraph + keep-running framing). Other 4 overlapping doc files auto-merged cleanly (`bootstrap-config.md`, `core-concepts.md`, `first-model-first-key-first-request.md`, `openai-sdk.md`). Code files all auto-merged cleanly. Additional Copilot review (post-`167196a` cycle) addressed: - `docs/index.md:7` — change link display text from `[data-plane]` to `[data plane]` to match the canonical glossary term. The URL anchor `#data-plane` stays kebab-case (matches the glossary heading's auto-anchor); only the display text changes. Comment id 3271145422. - `docs/quickstart/openai-sdk.md:43` — change `All three steps below` to `All commands below`. The Install-the-SDK section has two command blocks (mkdir+cd, npm install), not three; the prior wording originated from a mental model (mkdir, cd, install) that doesn't match the typographic count of code blocks under the heading. Comment id 3271145458. Copilot's third comment on `docs/overview/core-concepts.md` Observability Exporter wording (id 3271145444) auto-resolves via this merge — main's #326 rewrite supersedes the branch's pre-#326 wording at that location ("ships per-request span telemetry… OTLP/HTTP-compatible backend…" replaces "Use this concept when documenting…"). No separate edit needed; the merge IS the fix.
janiussyafiq added a commit
that referenced
this pull request
May 20, 2026
…ickstart-polish Resolve PR #344's lingering mergeable: dirty state by linking the branch history to origin/main (2c1d485 = post-#326 / #348 / #330 / #341 / #343 / #345 / #346). The squash-merge commit landed earlier (e2af197) integrated main's content into the branch tree but did not link the histories, so GitHub's mergeable computation still saw the 3-way-merge-base artifact conflict on docs/quickstart/self-hosted.md (a vs A + the glossary link / "In another terminal" vs "Keep the gateway running" framing). This explicit merge commit ties the branch to main's history. Self-hosted.md conflict resolved by taking OUR side — the branch's edits already contain main's substantive changes (capital A, first-time-build paragraph) plus this PR's additions (glossary link, keep-running framing, YOUR_ADMIN_KEY note, config.yaml location anchor). The auto-merge of first-model-first-key-first-request.md duplicated the :::warning callout that was already integrated via the squash commit; removed the duplicate.
This was referenced May 20, 2026
jarvis9443
deleted the
docs/issue-347-configuration-reference-corrections
branch
June 25, 2026 06:25
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 freeto 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.
Summary
Closes#347. Five surgical corrections to the configuration-reference cluster:
admin-api.md:76— group label "health" → "livez"./livezis the public route;/admin/v1/healthrequires admin auth.api-keys.md:86— rotate response example: 16-char key suffix replaced with a 32-char suffix matching what callers actually receive (Uuid::new_v4().as_simple()produces 32 hex chars).caching.md— explicit note thatCachePolicy.backendis parsed and stored on each row but not consulted by any runtime consumer; the proxy uses the bootstrap-config (cache.backend) instead.models.md:80—background_model_check.ignore_statusesdefault:Vec<u16>with#[serde(default)]resolves toVec::new(), not[408, 429]. Re-presented[408, 429]as a recommended explicit setting.models.md:163—Model.costconsumer split: AISIX Cloud's cp-api recomputes cost server-side; the standalone OSS proxy hard-codescost_usd = 0.0and does not consult the field.Doc-only diff. No code, schemas, configs, or test fixtures touched.
Changes
docs/configuration/admin-api.mddocs/configuration/api-keys.mdsk-abcd1234ef567890(16 hex) replaced withsk-550e8400e29b41d4a716446655440000(32 hex) to matchUuid::new_v4().as_simple()output.docs/configuration/caching.mdCachePolicy.backend-conservative bullet (line 83) explicitly stating the field is parsed but not consulted by the runtime proxy, that the runtime backend selection comes from bootstrap-config (cache.backend), and that the field is preserved for forward compatibility.docs/configuration/models.md[408, 429]default claim and stated that without an explicit value no probe statuses are ignored, then re-presented[408, 429]as a recommended explicit setting with the operational rationale (tolerates transient 408/429 during probes).docs/configuration/models.mdModel.costdescription now makes the substrate split explicit — AISIX Cloud's cp-api consumes the field; the standalone OSS proxy does not consult it at request time and emitscost_usd=0.0; pricing-aware budget enforcement requires the Cloud control plane.Net diff: 4 files,
+6 / -4. Source:git diff --stat.Test plan
Doc-only diff — no
.rsfiles, schemas, configs, or test fixtures touched. The cargo trio was still run end-to-end as the canonical pre-merge gate. The firstcargo test --workspaceinvocation OOMed under default parallelism (this VPS has 8 GB and the workspace compiles 30+ crate test binaries); re-ran withcargo test --workspace -j 2(andCARGO_BUILD_JOBS=2) to cap compile concurrency — all suites green.cargo fmt --check— PASS (exit 0).cargo clippy --workspace --all-targets -- -D warnings— PASS (exit 0, 18.30s, no warnings).cargo test --workspace -j 2— PASS (exit 0, 33 suites all green; 1067 tests passed, 0 failed, 3 ignored).grep -rln <each affected page> tests/e2e/for all 4 pages returns empty.pnpm testundertests/e2e/is not applicable for this diff.cargo clippytailcargo testsummaryAffected pages
docs/configuration/admin-api.mddocs/configuration/api-keys.mddocs/configuration/caching.mddocs/configuration/models.mdPre-merge-check verification log
Issue #347 called out three pre-merge checks. All three resolved as follows:
ignore_statusesdefault re-verified at PR-time HEAD. Field declaration onorigin/mainat71ea97eis atcrates/aisix-core/src/models/model.rs:259-260(drifted from:159-160cited in the issue body — file reorganization between audit-window and PR-window; substance unchanged). Declaration is verbatim:#[serde(default, skip_serializing_if = "Vec::is_empty")] pub ignore_statuses: Vec<u16>. No customdefault = "..."attribute, so the#[serde(default)]resolves to<Vec<u16> as Default>::default()=Vec::new(). Consumer atcrates/aisix-proxy/src/background.rs:84,88is unchanged:cfg.ignore_statuses.contains(&status)against the empty default, so the silent-failure mode is exactly as documented.Model.costhard-code re-verified at PR-time HEAD.crates/aisix-proxy/src/chat.rs:989still readslet cost_usd = 0.0;, preceded by the explanatory comment at:987-988: "cp-api recomputes cost server-side from its pricing catalog when ingesting telemetry; the DP just records 0.0 on the wire." The chat.rs file also hard-codescost_usd: 0.0at four additional call sites (:242,:648,:699,:832), confirming the pattern is workspace-wide, not just at one site. NoModel.costfield read site exists incrates/aisix-proxy/.71ea97e) and PR open (also71ea97e):/livezroute mount oncrates/aisix-admin/src/lib.rs:67✓/admin/v1/healthadmin-scoped mount oncrates/aisix-admin/src/lib.rs:145✓Uuid::new_v4().as_simple()for rotate plaintext oncrates/aisix-admin/src/apikeys_handlers.rs:168✓entry.value.backend/policy.backend/cache_policy.backendconsumers incrates/✓cfg.ignore_statuses.contains(...)consumer oncrates/aisix-proxy/src/background.rs:84,88✓let cost_usd = 0.0;hard-code oncrates/aisix-proxy/src/chat.rs:989✓Model.costexample placeholder noteThe 32-char replacement plaintext in the rotate example is
sk-550e8400e29b41d4a716446655440000. The 32-hex segment is the canonical "all-zeros after the variant byte" UUID example value used as a placeholder across many published examples; it is structurally identical to a realUuid::new_v4().as_simple()output (no hyphens, 32 hex chars) without being a copy of any real generated key.Coexistence with PR #326 and PR #344
This PR's affected pages (
admin-api.md,api-keys.md,caching.md,models.md) do not overlap with PR #326 or PR #344's affected files (which cover the overview / quickstart / bootstrap-config cluster). Nogit merge-treecollision check needed — disjoint file sets.References
docs: rebuild customer-facing docs across overview, config, cloud, ops, reference, and tutorials(the docs rebuild whose pages this PR amends)Summary by CodeRabbit
Documentation