docs(api): clarify max_output_length and is_ready semantics - #935
docs(api): clarify max_output_length and is_ready semantics#935lloydmak99 wants to merge 2 commits into
Conversation
The SKALE team reported both fields as misleading during their OpenClaw integration, and both reports were valid. max_output_length (and its top_provider.max_completion_tokens mirror) is admin-curated catalog metadata published for OpenRouter-schema compatibility. Nothing enforces it: the request path forwards max_tokens to the provider unchanged. The binding limit is the model's context window, shared between prompt and completion. is_ready is OpenRouter's catalog publication flag, stored and exposed verbatim. It says nothing about NEAR AI Cloud availability — every proxied third-party model reports false while being fully serviceable, and the field is omitted entirely when unset. Doc comments only; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
✅ OpenCodeReview: Review complete: 0 finding(s) across 1 selected item(s). |
|
Review — docs-only change to model metadata semantics Verified the claims in the diff against the code. The core assertions hold:
No critical issues. Notes below; none block merge, but (1) is a factual error in the new text. 1. "curated per model by NEAR AI Cloud administrators" is not the whole story — a backend can override it The published value is not purely admin-curated. On every model-list cache refresh, a positive backend-advertised value overwrites the stored one — if let Some(max_output_length) = metadata.max_output_length.filter(|value| *value > 0) {
model.max_output_length = Some(max_output_length);
}That feeds from The PR description says "nothing that feeds 2. The old text ("Maximum number of output tokens…") was replaced with read-path prose, so this request field now documents what the value means on read but nothing about what a PATCH does. That matters here because it differs from its neighbours: -- crates/database/src/repositories/model.rs:454
max_output_length = COALESCE($21, max_output_length),
-- vs. crates/database/src/repositories/model.rs:458
is_ready = CASE WHEN $27 THEN NULL ELSE COALESCE($25, is_ready) END,Combined with the plain 3. Stale copy of the removed sentence in the service layer
Minor "presence in Note: I did not run ✅ Approved — comment-only, no behaviour change, claims verified. Items 1 and 2 are worth a quick follow-up commit. |
🧭 IronLoop Run · ReviewThis comment updates in place as the Run moves through its stages. 🟩 Final result · Completed
Automatic trigger · attempt 1 of 3 · completed in 32s IronLoop completed the review and posted it to GitHub. 🔗 Result |
There was a problem hiding this comment.
🔍 IronLoop review
🟢 No actionable findings
No additional findings. The substantive documentation concerns applicable to this change were already raised in the captured review discussion.
Validation
- ✅ Changed-area inspection — Inspected the complete documentation-only change and traced the described fields through model listing, backend metadata merging, request forwarding, and admin update paths.
- ✅ CI checks — Captured unit, integration, end-to-end, lint, release-build, audit, and advisory checks passed.
Review details
- Run:
ddf11799-c46e-44a6-b979-3cc60942d7ae - Workflow: Review
- Attempts: 1
Addresses four points from the PR #935 review. The provenance claim was wrong. The comments said the value is "curated per model by NEAR AI Cloud administrators", which describes the current fleet rather than the mechanism: apply_backend_model_metadata overwrites the stored value whenever a backend advertises a positive one, on every model-list cache refresh. No NEAR backend advertises an output value today, which is why stored values survive — but the comment would have gone silently wrong the moment one did. Now states the override. UpdateModelApiRequest.max_output_length documented what the value means on read but nothing about what a PATCH does, and it differs from its neighbours: plain COALESCE against is_ready's tri-state CASE WHEN, with a plain Option<i32> rather than Nullable<T>. An admin sending null to clear a wrong value gets a silent no-op. Now says so. services/models/ports.rs still carried the exact sentence this branch removed from the API layer. "Presence in /v1/models is the availability signal" over-promised: listing is gated by is_active, not live provider health. Doc comments only; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Addressed all four review points in bc422c3 — doc comments only, no behaviour change. 1. Provenance was wrong — good catch. The comment said "curated per model by NEAR AI Cloud administrators", which describes the current fleet rather than the mechanism. 2. PATCH semantics added. The request field now documents that an omitted value or 3. Stale duplicate removed. 4. Absolute softened. "Presence in Verified locally: One clarification on the PR description, since it is the same distinction as point 1: "nothing that feeds 🤖 Generated with Claude Code |
Doc-comment-only change to the OpenAPI descriptions for
max_output_length,top_provider.max_completion_tokens, andis_ready. No behaviour change — the diff contains nothing but///lines.The SKALE team reported both fields as misleading during their OpenClaw integration. Both reports were valid.
max_output_lengthThe name promises an enforced cap; nothing enforces it.
crates/api/src/routes/completions.rsforwardsrequest.max_tokensto the provider unchanged and never clamps it against this column.Live on 2026-08-18, the gateway published
max_output_length: 8192fordeepseek-ai/DeepSeek-V4-Flashwhile the model's own endpoint reportedmax_model_len: 1048576and advertised no output cap. A sweep of all 20 direct completions endpoints found that none advertises an output cap — every one reports onlymax_model_len, which bounds prompt and completion together. A separate audit ofcvm-compose-filesconfirmed no serving configuration imposes a fixed per-response output cap on any model.Worth noting the contrast with the neighbouring field:
apply_backend_model_metadataoverwrites a stored value whenever a backend advertises a positive one. Backends advertisemax_model_len, which feedsadvertised_context_length(), but nothing that feedsadvertised_max_output_length(). Of the 15 NEAR-hosted models, 13 have a backend reportingmax_model_lenand all 13 publishedcontext_lengthvalues match it exactly.context_lengthself-heals from the deployment;max_output_lengthhas nothing to heal it from.The comments now state that the field is advisory, that
max_tokensis forwarded unchanged, and that the binding limit is the context window.is_readyThe comments already described the OpenRouter mechanics but not the consequence for clients. Live data shows every proxied third-party model (
openai/*,anthropic/*,google/*,qwen/qwen3.7-max) reportingfalsewhile being fully serviceable, and several models omitting the field entirely — so a client treating it as a boolean availability gate gets three different wrong answers.The comments now state explicitly that it does not indicate NEAR AI Cloud availability and that presence in
/v1/modelsis the availability signal.Verification
cargo fmt --checkandcargo check --all-targetsboth pass. Tri-state PATCH semantics text is unchanged.Not addressed here
Two follow-ups surfaced during the audit and are deliberately left out of this doc-only PR:
openai/privacy-filterandopenai/whisper-large-v3publish an output limit larger than their context;Qwen/Qwen3-Embedding-0.6Bandblack-forest-labs/FLUX.2-klein-4Bcarry token limits on non-generative models).8192and16384cover 31 of 48 rows.max_output_lengthcannot be cleared throughPATCH /v1/admin/models: the update usesmax_output_length = COALESCE($21, max_output_length), whereasis_ready,deprecation_date, andopenrouter_slugthree lines below useCASE WHEN $clear THEN NULL ELSE COALESCE(...) END, and the request field is a plainOption<i32>rather thanNullable<T>. Correcting the data needs either direct SQL or a small tri-state change.🤖 Generated with Claude Code