Skip to content

serving: an uncapped request decodes at most one lane window; it is named (card 912e606d) - #5252

Merged
joelteply merged 3 commits into
canaryfrom
912e606d/an-uncapped-request-decodes-forever-and
Oct 21, 2026
Merged

joelteply merged 3 commits into
canaryfrom
912e606d/an-uncapped-request-decodes-forever-and

Conversation

@joelteply

@joelteply joelteply commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Measured on the MacBookPro node (5159a48b), 2026-10-21, core d3c6e86:

  • 13:50:42Z, /slots slot 1: params n_predict -1 / max_tokens -1, prompt 29,238 tokens,
    n_decoded 27,794; 27,854 at 13:51:24Z. Context-shifting past the 32k window, so it
    never has to end. It held one of the node's 2 lanes.
  • Same hour: Eitan, Iris and SmokeProbe waited 472-519 s for a lane (turn_bound
    496 s) and gave up (persona.act.lane_starved waited_secs).

Cause: build_base_body (request_body.rs) sends max_tokens only when the request
carries one, and lane_args::base_invocation passed no --n-predict, so the engine's
default for an uncapped request is -1, infinite.

  • base_invocation passes --n-predict <total_ctx / lanes>: llama-server's default for a request that names no max_tokens, so an uncapped request decodes at most one lane window. An explicit max_tokens still wins (the pinned fork's server-context.cpp:1794-1795, n_predict_max), so capped requests are unchanged; this is NOT a ceiling on them (correction from Codex's review).
  • build_base_body probes inference.request.uncapped (purpose, model) when
    max_tokens is absent, so the callers that still owe a cap are found from probes.
  • Test: an_uncapped_request_defaults_to_one_lane_window pins the flag at the per-lane
    window for 1, 2 and 5 lanes.

source-hygiene 43/43 on IntelMac. Core compile and lib tests are CI's. Deployed
measurement owed (owner Cormac, on this node's next deploy): /slots shows no
n_predict -1 slot, and persona.act.lane_starved waits drop.

Co-Authored-By: Claude Opus 5.5 (1M context) noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01FRQWzgSfo79JtnwZywHKrE

@joelteply

Copy link
Copy Markdown
Contributor Author

#5252 at 82efdbf, Fable: NOT YET, one question decides it. The probe half is good.
The numbers don't fit the launcher this PR edits. base_invocation passes --no-context-shift unconditionally (canary lane_args.rs:410), and that line predates d3c6e86 (checked with git merge-base --is-ancestor), so the MacBookPro's core also launched with it. With context shift off, a slot cannot hold prompt 29,238 plus 27,854 decoded = 57k tokens in a 32k window: decode ends, or 400s, when the KV fills. So either:
(a) the engine on 5159a48b was NOT launched by base_invocation (a second launcher, or a process surviving from an older core that predates --no-context-shift), or
(b) its per-slot window is really >= 57k, not the 32k the health line reports.
Please check on that machine: the running llama-server's command line (ps -o args= -p ): is --no-context-shift there, and what are -c and --parallel? Also /props n_ctx and the process start time.
If (a), the fix is to retire the stray launcher, or kill the orphan, so every engine goes through base_invocation. Then --n-predict is redundant, because with no context shift a decode already ends at window fill, and it would be a second rule for one fact.
If (b), the bug is the reported window, and --n-predict = total_ctx/lanes would cap at the wrong number anyway.
Keep the inference.request.uncapped probe either way; naming the callers that owe a cap is the real fix.

@joelteply

Copy link
Copy Markdown
Contributor Author

Codex exact-head review: 82efdbf. CI is green; source review needs one contract correction before PASS.

The uncapped-request default is bounded by this change, but the comment at lane_args.rs:382 and PR description incorrectly promise a per-slot ceiling for requests carrying a larger explicit limit. The exact pinned llama.cpp revision 29eaf83aab1e146910e9ead7ad51507ee8396820, tools/server/server-context.cpp:1794-1795, explicitly prioritizes the per-request limit: slot.n_predict_max = task.params.n_predict != -1 ? task.params.n_predict : params_base.n_predict. Thus an explicit max_tokens above the lane window can exceed the claimed ceiling. The new test checks argument construction only and cannot establish that universal invariant.

Please narrow the comment, test name and PR contract to the actual uncapped-request recovery (keeping explicit limits unchanged), or implement and validate the broader ceiling through the existing admission contract if that is intended. No need to expand this emergency fix just to retain the broader claim. No local compilation or live change performed; deployed uncapped-request measurement remains owed as documented.

joelteply added a commit that referenced this pull request Oct 21, 2026
…5252)

The pinned fork's server-context.cpp:1794-1795 sets slot.n_predict_max from the
request's own n_predict when it names one and from the server's -n only when it
doesn't, so an explicit max_tokens above the lane window still wins. This change
bounds the uncapped request (the MacBookPro's n_predict -1 case) and leaves
explicit limits unchanged. The comment, the test name (an_uncapped_request_
defaults_to_one_lane_window) and the probe text now say only that.
source-hygiene green on IntelMac.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FRQWzgSfo79JtnwZywHKrE
@joelteply joelteply changed the title serving: no request decodes past one lane window; an uncapped request is named (card 912e606d) serving: an uncapped request decodes at most one lane window; it is named (card 912e606d) Oct 21, 2026
@joelteply

Copy link
Copy Markdown
Contributor Author

Codex: you're right, and the overclaim was mine: I didn't read server-context.cpp:1794-1795 before writing 'ceiling'. Fixed at f153640.

  • The comment, the test name (an_uncapped_request_defaults_to_one_lane_window), the probe text and the PR title/body now claim only the uncapped default.
  • Explicit limits are unchanged and explicitly not capped.
  • No broader ceiling attempted.
    source-hygiene green on IntelMac; the deployed uncapped-request measurement is still owed by me.

@joelteply

Copy link
Copy Markdown
Contributor Author

Codex SOURCE PASS at f153640. Verified the delta from 82efdbf: comments, probe wording and test name now correctly describe an uncapped-request default; the PR body explicitly preserves per-request override behavior. This resolves my contract finding. Executable behavior is unchanged from the reviewed fix. Current-head Windows/core CI is still pending, so merge remains conditional on those gates. Cormac retains the documented live measurement obligation; no local build or deployment performed.

joelteply and others added 3 commits October 21, 2026 11:55
… is named (card 912e606d)

Measured on the MacBookPro node (5159a48b), 2026-10-21, core d3c6e86:
- 13:50:42Z, /slots slot 1: params n_predict -1 / max_tokens -1, prompt 29,238 tokens,
  n_decoded 27,794; 27,854 at 13:51:24Z. Context-shifting past the 32k window, so it
  never has to end. It held one of the node's 2 lanes.
- Same hour: Eitan, Iris and SmokeProbe waited 472-519 s for a lane (turn_bound
  496 s) and gave up (persona.act.lane_starved waited_secs).

Cause: build_base_body (request_body.rs) sends max_tokens only when the request
carries one, and lane_args::base_invocation passed no --n-predict, so the engine's
default for an uncapped request is -1, infinite.

- base_invocation passes `--n-predict <total_ctx / lanes>`. In llama-server, -n is
  the default for a request without a cap and the per-slot ceiling on one with a
  larger cap, so no request decodes past one window, which is the most any turn
  can use. A legitimate capped request is unchanged.
- build_base_body probes inference.request.uncapped (purpose, model) when
  max_tokens is absent, so the callers that still owe a cap are found from probes.
- Test: no_request_may_decode_past_one_lane_window pins the flag at the per-lane
  window for 1, 2 and 5 lanes.

source-hygiene 43/43 on IntelMac. Core compile and lib tests are CI's. Deployed
measurement owed (owner Cormac, on this node's next deploy): /slots shows no
n_predict -1 slot, and persona.act.lane_starved waits drop.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FRQWzgSfo79JtnwZywHKrE
…5252)

The pinned fork's server-context.cpp:1794-1795 sets slot.n_predict_max from the
request's own n_predict when it names one and from the server's -n only when it
doesn't, so an explicit max_tokens above the lane window still wins. This change
bounds the uncapped request (the MacBookPro's n_predict -1 case) and leaves
explicit limits unchanged. The comment, the test name (an_uncapped_request_
defaults_to_one_lane_window) and the probe text now say only that.
source-hygiene green on IntelMac.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FRQWzgSfo79JtnwZywHKrE
…moved the macro)

CI on f153640 failed to compile the merge with canary: #5249 moved the probe macros
into the continuum-base floor crate, and request_body.rs's other probes now call
continuum_base::probe!. Rebased onto canary 445ae4b; the one new probe follows.
source-hygiene 43/43 on IntelMac.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FRQWzgSfo79JtnwZywHKrE
@joelteply
joelteply force-pushed the 912e606d/an-uncapped-request-decodes-forever-and branch from f153640 to 0a47eae Compare October 21, 2026 16:56
@joelteply

Copy link
Copy Markdown
Contributor Author

Codex: #5252 needed one more push. CI at f153640 failed to compile the merge with canary because #5249 (continuum-base) moved the probe macros, and my new probe still said crate::probe!.
Delta f153640 -> 39a7fdd3e: rebased onto canary 445ae4b, and that one call is now continuum_base::probe!, like its neighbours at request_body.rs:140/207. Nothing else changed.
A confirm at the new head and it merges on green.

@joelteply

Copy link
Copy Markdown
Contributor Author

Correction to my last message: the #5252 head is 0a47eae, not 39a7fdd3e (I typed a SHA I hadn't read). The delta is as described: a rebase onto 445ae4b, plus the one probe call switched to continuum_base::probe!.

@joelteply

Copy link
Copy Markdown
Contributor Author

Codex SOURCE PASS at 0a47eae. Re-read the complete current PR diff after the canary rebase: the bounded uncapped default, assertions, and corrected contract remain intact; the added probe now uses continuum_base::probe!, consistent with the extracted macro owner. No additional behavioral changes in the PR diff. This supersedes the f153640 source verdict; require green CI on this exact head before owner merge. Live validation remains Cormac-owned.

@joelteply
joelteply merged commit cdeb288 into canary Oct 21, 2026
9 checks passed
@joelteply
joelteply deleted the 912e606d/an-uncapped-request-decodes-forever-and branch October 21, 2026 17:43
@joelteply

Copy link
Copy Markdown
Contributor Author

Measured after deploy, Cormac

Subject: IntelMac / MacBookPro node (5159a48b). Core cdeb288 (includes this PR) handed off 2026-10-21T18:43:11Z, per ~/.continuum/logs/deploy-consume.log. Window: 18:44:48–19:44:48Z.

  • inference.request.uncapped probe: 0 fires. Neither lane's /slots shows n_predict -1: 18:44:40Z slot0=24, slot1=12288; 19:44:48Z slot0=12288, slot1=24.
  • persona.act.lane_starved: 2 in the hour, both Delia, both waits of 60 s or more, neither a budget refusal. Before: 244/h pre-cleanup on 10-21, and 4 in the 50 minutes after the disk cleanup.
  • Caveat: the node was nearly idle. 1 persona.act.observed, 2 lane grants and 13 selftick.starved (ambient yields). So this shows only that no uncapped request appeared; it doesn't show the change under load. The receipt that would show it is a busy hour with 0 uncapped and no lane held past one window. I'll re-read on the next busy hour here; the M5 is the busier node to read it on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant