Skip to content

fix(serve): refuse over-length prompts; report finish_reason=length on every path (#3718) - #4550

Closed
noahgift wants to merge 8 commits into
mainfrom
49/3718-prompt-overlength
Closed

noahgift wants to merge 8 commits into
mainfrom
49/3718-prompt-overlength

Conversation

@noahgift

@noahgift noahgift commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Closes #3718

A prompt that fills the model's context window is now refused whole, with an OpenAI-style 400 (code: context_length_exceeded, both counts in the body). Before this, the wgpu handler capped max_tokens at 4096 and nothing else. The prompt prefilled anyway, the KV cache grew past the trained context, and the reply said finish_reason: "stop".

  • context_budget_3718.rs: context_token_budget gives min(requested, ctx - prompt) or refuses. finish_reason_for is the single "length vs stop" rule.
  • Every serve path now reports "length" when the budget ran out: wgpu (both copies), CUDA non-stream, CUDA→CPU fallback, and SafeTensors chat.
  • Tests: 25 pass. Clippy is clean on default and on --features wgpu. cargo check --features cuda passes.

Quorum: docs/audits/quorum-PMAT-3718.json, 3/3 PASS at 6df7689. Round 1 caught that CUDA still said "stop"; round 2 fixed it.

CI fix carried here (cop CI rule 3, 2026-09-27)

x86-main's mutation section failed with "cargo test failed in an unmutated tree": the cargo-mutants baseline (cargo test -p apr-cli --lib) hit its 300 s timeout at 7434/7435 tests. All 17 test_llm_band tests SHA-256 current_exe() (the 472 MB test binary) with sha2 at opt-level 0 (~11 MB/s). 8d86812 sets sha2 to opt-level 3 in dev (byte-identical to the hunk in #4554, so either may land first): 65 test_llm_band tests now pass in 1.39 s. Deterministic, so it is fixed, not rerun.

🤖 Generated with Claude Code

@noahgift
noahgift enabled auto-merge September 27, 2026 11:35
@github-actions

github-actions Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

§13.11 rung 1 — quorum shadow verdict

S13-SHADOW pr=4550 head=a6acb67602a069a9ce6028405f4db20daa661c76 verdict=REFUSE class=Q1 arm_rc=1

Shadow mode: this records a verdict and merges nothing. A refusal
to arm is not a block (§13 adds zero rows to §7) — the pull request is
exactly as green as it was.

noahgift and others added 3 commits September 27, 2026 15:13
…eports a cut as "length" (#3718)

#3718 done_when 3: a prompt too long for the context says so, never a silent
cut. The CPU (effective_max_tokens), CUDA and Qwen3.5 (Session) paths already
refuse it. The wgpu handler was the gap: it capped max_tokens at 4096 and
nothing else, so an over-length prompt prefilled anyway, the decode loop grew
the KV cache past the model's window, and the stream's final chunk hardcoded
finish_reason "stop".

- WgpuInferenceState carries the model's context_length (GGUF config).
- wgpu_chat_completion refuses prompt_len >= context_length with HTTP 400 and
  an OpenAI-shaped body (error.code = "context_length_exceeded", both counts),
  before any prefill, and clamps max_tokens to the room left.
- The streaming done chunk reports "length" when the budget was spent (the
  blocking path already did).
- context_token_budget / context_length_exceeded_body are pure and not gated
  on the wgpu feature, so default-feature CI runs their 4 case rows.

Verified: cargo test -p apr-cli --lib context_budget_3718 (4 pass);
cargo clippy -p apr-cli --lib --no-default-features --features wgpu -D warnings
clean. Note: `--features wgpu` WITH default features does not compile on main
(finetune.rs uses entrenar wgpu types without enabling entrenar/wgpu); that is
pre-existing and not touched here.

Refs #3718

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… reported a cut as "stop" (#3718)

Quorum round 1 (gemini-3.1-pro-high, FAIL): handler_gpu_completion.rs
hardcoded "finish_reason": "stop" on the CUDA non-streaming reply and on the
CUDA->CPU fallback, so a reply cut at max_tokens still read as finished. The
SafeTensors /v1/chat/completions path (chat.rs build_chat_response) did the
same whenever there were no tool calls.

All four serve paths now take finish_reason from ONE rule,
finish_reason_for(generated, max_tokens): "length" when the budget ran out,
else "stop" (tool_calls still wins on the SafeTensors path). The two inline
copies in the wgpu handler are replaced by it.

Tests: finish_reason_for's table, and a SafeTensors reply of 16/16 tokens
reads "length". cargo test -p apr-cli --lib (filtered) 25 pass; clippy
-D warnings clean on default and on --no-default-features --features wgpu;
cargo check --features cuda clean.

Refs #3718

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@noahgift
noahgift force-pushed the 49/3718-prompt-overlength branch from 751483e to acf8adb Compare September 27, 2026 13:13
@noahgift noahgift added the owner:aprender-49 owning session (cop inbox claims) label Sep 27, 2026
… out hashing the test binary (#4550)

x86-main's mutation section went red with "cargo test failed in an unmutated
tree": cargo-mutants' baseline `cargo test -p apr-cli --lib` hit its 300 s
timeout with 7434/7435 tests done. The stragglers were all 17 test_llm_band
tests: provenance hashes current_exe() (PP-25), which in a test is the 472 MB
apr-cli lib test binary, and sha2 at opt-level 0 runs ~11 MB/s, so each test
paid a ~40 s SHA-256 and one never finished (cuda_without_the_server_feature_is_refused).
Deterministic, not a flake: not in any flake ledger, so fixed rather than rerun.

Same hunk as 49/x86-slow-2 (88449ce) and #4554, byte-identical, so the two
PRs merge in either order. Optimizing only sha2 keeps the hash real.

Measured: `cargo test -p apr-cli --lib test_llm_band` 65 passed in 1.39 s
(CI baseline: 17 tests >60 s, one past the 300 s kill).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@noahgift
noahgift disabled auto-merge September 27, 2026 20:42
@noahgift

Copy link
Copy Markdown
Contributor Author

quorum-review (AD-04): NOT agreed (auto_merge: checked=true was_armed=true disarmed=true)

{
 "ticket": "PMAT-3718",
 "head": "8d86812d9c7f45d75b6a7579e5a2806305c4b7dd",
 "width": 3,
 "executor": "agy",
 "agreed": false,
 "auto_merge": {
  "checked": true,
  "was_armed": true,
  "disarmed": true,
  "note": "auto-merge was armed from an earlier round; disarmed before lanes launched"
 },
 "lanes": [
  {
   "lane": 1,
   "verdict": "FAIL",
   "findings": 0
  },
  {
   "lane": 2,
   "verdict": "FAIL",
   "findings": 4
  },
  {
   "lane": 3,
   "verdict": "PASS",
   "findings": 0
  }
 ]
}

…uorum), it lands once via #4554

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@noahgift

Copy link
Copy Markdown
Contributor Author

quorum-review (AD-04): NOT agreed (auto_merge: checked=true was_armed=false disarmed=false)

{
 "ticket": "PMAT-3718",
 "head": "c2e56c93db48e4a2b607221995471b034094e06c",
 "width": 3,
 "executor": "agy",
 "agreed": false,
 "auto_merge": {
  "checked": true,
  "was_armed": false,
  "disarmed": false,
  "note": "auto-merge not armed"
 },
 "lanes": [
  {
   "lane": 1,
   "verdict": "PASS",
   "findings": 0
  },
  {
   "lane": 2,
   "verdict": "PASS",
   "findings": 5
  },
  {
   "lane": 3,
   "verdict": "NO-VERDICT",
   "findings": 0
  }
 ]
}

@noahgift

Copy link
Copy Markdown
Contributor Author

quorum-review (AD-04): NOT agreed (auto_merge: checked=true was_armed=false disarmed=false)

{
 "ticket": "PMAT-3718",
 "head": "c2e56c93db48e4a2b607221995471b034094e06c",
 "width": 3,
 "executor": "agy",
 "agreed": false,
 "auto_merge": {
  "checked": true,
  "was_armed": false,
  "disarmed": false,
  "note": "auto-merge not armed"
 },
 "lanes": [
  {
   "lane": 1,
   "verdict": "PASS",
   "findings": 0
  },
  {
   "lane": 2,
   "verdict": "FAIL",
   "findings": 4
  },
  {
   "lane": 3,
   "verdict": "PASS",
   "findings": 0
  }
 ]
}

…_length_exceeded, finish_reason judged against the clamped budget (#3718)

Quorum finding on #4550: the SafeTensors chat/completions paths judged
finish_reason against the requested max_tokens while Session clamps the
budget to context_length - prompt_len silently, so a context cut read as
"stop"; a prompt >= the window surfaced as a 500. Both now go through
context_token_budget / context_length_exceeded_body via st_context_budget.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@noahgift

Copy link
Copy Markdown
Contributor Author

quorum-review (AD-04): three PASS — agreed (auto_merge: checked=true was_armed=false disarmed=false)

{
 "ticket": "PMAT-3718",
 "head": "42eb448d14e6982756d6eca5a5d1a6cb7e1b4628",
 "width": 3,
 "executor": "agy",
 "agreed": true,
 "auto_merge": {
  "checked": true,
  "was_armed": false,
  "disarmed": false,
  "note": "auto-merge not armed"
 },
 "lanes": [
  {
   "lane": 1,
   "verdict": "PASS",
   "findings": 1
  },
  {
   "lane": 2,
   "verdict": "PASS",
   "findings": 0
  },
  {
   "lane": 3,
   "verdict": "PASS",
   "findings": 0
  }
 ]
}

@noahgift

Copy link
Copy Markdown
Contributor Author

Moved into #4606 (PRCAP fold, cop order 14:02Z). Fold = MOVE: head a6acb67602a069a9ce6028405f4db20daa661c76 is an ancestor of the pushed fold head, so no work is lost. The branch is kept. Agent: aprender-59

@noahgift noahgift closed this Sep 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

owner:aprender-49 owning session (cop inbox claims)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

apr run --json: add prompt_tokens (and completion_tokens) so consumers can see how many tokens the model actually read — RAH asked (#3716)

1 participant