Skip to content

Fix prompt len in parallel sampling - #928

Merged
Ying1123 merged 2 commits into
sgl-project:mainfrom
yichuan-w:fix_promptlen
Aug 5, 2024
Merged

Ying1123 merged 2 commits into
sgl-project:mainfrom
yichuan-w:fix_promptlen

Conversation

@yichuan-w

Copy link
Copy Markdown
Contributor

Thank you for your contribution, we really appreciate it. The following instructions will help improve your pull request and make it easier to receive feedback. If there are any items you don't understand, don't worry. Just submit the pull request and ask the maintainers for help.

Motivation

Fix prompt len in parallel sampling

Modification

openai Adapter and resolve one fixme in test openai API

Checklist

  1. Ensure pre-commit pre-commit run --all-files or other linting tools are used to fix potential lint issues.
  2. Confirm that modifications are covered by complete unit tests. If not, please add more unit tests for correctness.
  3. Modify documentation as needed, such as docstrings or example tutorials.

@yichuan-w
yichuan-w requested review from Ying1123 and merrymercy and removed request for Ying1123 August 5, 2024 06:22
@Ying1123
Ying1123 enabled auto-merge (squash) August 5, 2024 07:55
@Ying1123
Ying1123 merged commit fd7926e into sgl-project:main Aug 5, 2024
timethink pushed a commit to timethink/sglang that referenced this pull request Mar 9, 2025
JinyanYi pushed a commit to JinyanYi/sglang that referenced this pull request Jul 10, 2026
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 27, 2026
… why sgl-project#929 is not this

Addendum to the record on the same branch; still test-only.

THE INPUT THAT NARROWS IT. The sgl-project#929 analysis reads mamba_exist=True with
len(key) == 0 at the re-admit insert (unified_radix_cache.py:1569/:1668): the
FULL prefix is already in the tree and it DOES carry a mamba value, and the
decode is garbage regardless. "No anchor" is therefore not the explanation --
the anchor exists and is found. What is wrong is what the resume reads out of
it, which is this branch's root: a slot id resolved against the pool of the
phase that is computing, not the pool that holds its bytes.

DEPTH IS NOT THE OPEN TERM EITHER. The donate pairs key and state at one
position by construction (cache_len = req.mamba_last_track_seqlen,
mamba_component.py:721; mamba_ckpt_utils' "Never floor" states why it may not
be corrected after the fact). Provenance is the term nothing holds.

sgl-project#929 IS A DIFFERENT BOOKING, and the counter-check is cheap enough to state
rather than schedule. On the plain-finished path with mamba_exist=True the
freshly allocated donate slot (mamba_component.py:902-903) is dropped without a
free: cleanup_after_caching_req's enable_mamba_extra_buffer branch (:947-956)
frees only req's own slots, and the insert_params.mamba_value free exists only
on the int8 branch (:942-943) and the unfinished branch (:961-963). It leaks
BECAUSE it was never attached to a node -- and the resume's COW source is
last_node.component_data[ct].value, a slot the tree HOLDS. Orphaned-unreachable
and tree-held are disjoint, so no resume can read the orphan. The rank-0 +1
mamba slot divergence is sgl-project#929's insert-side booking, not a second root here,
and it is struck from this trace.

TEST RESULT (desk, CUDA_VISIBLE_DEVICES=""): unchanged, 1 failed as intended --
"AssertionError: <mamba pool tp> is not <mamba pool pp-primary>".
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 27, 2026
…not by who computes now

THE GARBAGE. Boot 2g and the 2g-2 length ladder: under strict phase purity
every prompt -- 718 tokens as much as 9447, one prefill chunk as much as three
-- answered with text unrelated to the prompt, and a second send of the SAME
prompt at temperature 0 answered differently again. One send emitted a single
EOS. No chunk boundary is needed, so the chunked-prefill handoff is not it.

THE ASYMMETRY, IN TWO LINES. The WRITE side resolves by phase:
mamba_component.py:916 donates through active_mamba_state_pool
(mamba_state_pool.py:32-35, via phase_active_mamba_pool), so the anchor's
conv/ssm bytes are placed in the pool of the phase computing AT DONATE TIME.
The READ side does not: model_runner.py:4368-4373 copies out of
pool.mamba_pool with pool = self.req_to_token_pool (:4317), unconditionally,
and model_runner.py contains no occurrence of the phase-aware accessor at all.
Across a flip the writer and the reader resolve to DIFFERENT pool objects
(gdn_flip_mover.py:848-851 names both).

WHY EVERY REQUEST. Strict purity prefills in PP, retracts the request at the
pp_to_tp cutover and re-admits it in TP against its own just-cached prefix
(boot_2f_698cd396ce_0827_0704.log:3805/3808, :4672/:4673 -- "ADMIT
prefix_lens=9447" then "phase=tp, #new-token: 1, #cached-token: 9447"). The
decode's recurrent state therefore comes from the radix anchor, and the anchor
is a bare slot id. Nothing moves the bytes at the cutover either: gdn_flip_mover
moves RESIDENT slots and the log reads "0 live slots, sent 0 cells / 0.00 MiB".
So the resume read slot N of the TP pool while the state sat at slot N of the
PP pool, and slot N of the TP pool holds whatever the TP stack last left there
-- three identical sends, three different answers.

This is sgl-project#767's unfixed half, and the fix says so: install_phase_aware_mamba_
state_pool scopes itself in its own docstring to "only the byte copies
(checkpoint donation copy, int8 store)". All six call sites of
active_mamba_state_pool are donate/store. Not one is a resume.
mamba_state_pool.py's header records the same failure from the write side,
measured 2026-08-19: "a kite prompt answered with a foreign river essay".

THE FIX. active_mamba_state_pool answers "who computes NOW"; at a TP resume
that is TP while the anchor sits in PP, so the accessor alone cannot serve the
read side. It needs PROVENANCE. mamba_state_pool.py gains a bounded ledger
(one entry per mamba slot, overwritten on every insert, so a recycled slot
cannot be vouched for by its previous tenant) plus anchor_bytes_reachable /
anchor_provenance_verdict. Single-pool boots answer True unconditionally and
are bit-for-bit unchanged; on a split boot an anchor nothing recorded answers
False, the conservative direction -- refusing costs a re-prefill, trusting
costs the wrong answer.

RECORDED AT THE TREE INSERT, NOT IN A DONATE BRANCH. This rig runs
mamba_radix_cache_strategy='no_buffer', so enable_mamba_extra_buffer is False
(server_args.py:19472-19476) and the ping-pong branch is never entered; the
live branch is the plain one at :901-918. There are four donate branches and
they disagree about whether they copy at all, so teaching provenance to one
would have been inert on a rig running another -- the enablement-gap class.
commit_insert_component_data is the single place an anchor becomes a node's
value, so the hook is there and covers every branch by construction.

REFUSAL, NOT A SILENT READ, at finalize_match_result -- two arms, both
previously silent:
 (a) mamba_value is None with no host copy. The `if` had no else, so the
     request kept mamba_needs_clear True (memory_pool.py:2090), ZEROED its
     recurrent state, and reused the full KV prefix anyway.
 (b) the anchor's bytes belong to the other phase's pool.
Both zero the match and keep branching_seqlen, as the strict-resume zeroing
already does. The rule is quoted from thirty lines below, where the same
function already applies it to slot starvation: "Reusing the KV prefix without
the matching mamba state would be silently wrong, so the whole match is zeroed."

WHY REFUSE RATHER THAN FETCH: the PP pool is layer-axis sharded and the TP pool
head-axis sharded. Translating is what gdn_flip_mover does, on a plan and a
collective; a per-request copy at match time cannot.

THE COST, NAMED RATHER THAN DISCOVERED ON METAL. Refusing sends the request
back to a full prefill, which under strict purity runs in PP, donates a fresh
PP-pool anchor, and meets the same verdict at the next re-admission. A request
that is RETRACTED instead of carried across the cutover can therefore refuse
forever, two cutovers a lap. The loop is not created here -- it is the hole the
wrong answer was hiding -- and the way out is the resident carry of the mamba
state across pp_to_tp, not a softer refusal. A SECOND refusal of the same
request logs that by name at ERROR instead of looping in silence.

SIBLINGS, SAME MOVE:
 * second lineage (sgl-project#747's rule): mamba_radix_cache.py records at both insert
   sites and refuses at its resume, same shape.
 * int8 checkpoint pool, the same defect in the opposite direction
   (_commit_int8_checkpoint writes the TREE-BOUND pool, model_runner.py:4361
   reads the EXECUTING runner's). Inert today only because mamba_ckpt_size is
   unset, which is a boot flag away from the same wrong answer, so
   install_phase_aware_mamba_state_pool announces the mismatch at boot -- the
   only place both objects are in scope.
 * model_runner.py:4354 clear_slots writes the computing stack's own slot and
   is correct as it stands.

BOOT INSTRUMENT, one grep. The match seam logs the pool that WROTE the anchor
("[sgl-project#928 anchor] ... anchor_pool=0x..") and model_runner.py logs the pool that
READS it ("[sgl-project#928 cow] ... state=0x.."). One boot puts the two ids side by side;
no tensor is touched and no D2H sync is added on that line.

TEST RESULTS (desk, CUDA_VISIBLE_DEVICES="", /spinning/htsglang-gpu/.venv):
 * test_anchor_bytes_follow_provenance_928.py -- 6 passed. Six arms: the
   defect; same-phase resume NOT refused (a fix that refuses everything is an
   outage); single-pool boots never refused; unrecorded anchor refused; a
   recycled slot not vouched for by its previous tenant; and provenance
   recorded by the tree insert rather than a donate branch (the no_buffer
   enablement guard).
 * MUTATION, both danger directions and the hook itself:
     A  anchor_bytes_reachable -> always True (the pre-fix silence)  3 failed
     B  anchor_bytes_reachable -> always False (over-refusal)        3 failed
     C  insert drops note_anchor_bytes                              1 failed
   restored: 6 passed.
 * REGRESSION: test/registered/unit/mem_cache/ plus every flip/gdn/phase test
   in managers/ -- 3580 passed, 927 skipped, 616 subtests passed, 6 failed.
   All six re-run on the parent 290fea9 and fail there identically:
   2x test_acceptance_emitters_758 "No CUDA GPUs are available" (desk has no
   GPU) and 4x test_phase_flip_rotation_wiring_809 "retry() exceed maximum
   number of retries". Pre-existing, not this change.
 * ruff clean on every file this touches; the 5 remaining in model_runner.py
   and 21 in mamba_radix_cache.py are byte-identical on the parent.

STILL OPEN, and it is the capacity half: cached anchors are not carried across
the cutover the way resident slots are. Until they are, a strict-purity resume
gives up its hit instead of giving a wrong answer.
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 27, 2026
…layer ids, or refuse

sgl-project#706 slice 2 (a38f39f, 2026-08-17) built the geometry-neutral {hash}.mamba
blob and build_mamba_window to cut it for either phase. It has never once been
built at runtime, on any model, on any boot.

THE HOP. HiCacheController._canonical_mamba_window opened with

    cache_params = getattr(model_config, "mamba2_cache_params", None)
    mamba_layer_ids = list(getattr(cache_params, "layers", None) or [])
    if not mamba_layer_ids:
        return None

and mamba2_cache_params is a property of the CHECKPOINT config
(Qwen3NextConfig, configs/qwen3_next.py:288, layers=self.linear_layer_ids),
never of sglang's ModelConfig: `grep -c mamba2_cache_params
configs/model_config.py` is 0. The getattr missed on every model, the list was
always empty, and the one silent branch fired every time. The correct hop is
hf_text_config -- the one gdn_flip_mover.py:929 already uses for GDN geometry.

MEASURED, boot 2g (boot_2f_698cd396ce_0827_0704.log). Format explicitly armed
(phase_flip_canonical_kv_page=True, phase_flip_writeback=True, backend 'file',
page_size 1, so the pairing guard at server_args.py:8716 held), and the log
carries "sgl-project#706 canonical KV page active" THREE times -- one per rank -- against
"sgl-project#706 canonical GDN blob active" ZERO times and zero refusals. Exactly the split
_canonical_mamba_window's own docstring calls fatal: "a canonical KV page beside
a phase-local GDN blob delivers ZERO usable prefix ... Silently running KV-only
would look like the feature was on while every cross-phase lookup missed."

THAT IS WHERE sgl-project#928 COMES FROM. KV crossed the flip geometry-neutrally while the
recurrent anchor stayed phase-local; sgl-project#856 then drops the tree at every cutover
trusting the canonical store ("Nothing is carried across"), and read-through
returned a correct KV prefix beside an anchor in the writing phase's layout. The
on-device fallback could not cover it either: the GDN mover runs AFTER the drop
and logged "moved 0 slot(s)" on all ten flips of that boot.

THE FIX IS THE SIBLING LADDER, NOT A SECOND BARE LOOKUP. resolve_attn_layer_ids
was given exactly this treatment on the same day, for the same reason ("RESOLVED,
not guessed" -- it could not tell NOT HYBRID from NOT POPULATED). The linear half
kept a raw getattr aimed at the wrong object. canonical_page_store gains
resolve_linear_layer_ids with the same four rungs: (a) ModelConfig's own list if
a future one populates it; (b) the checkpoint config's own properties through
hf_text_config, linear_layer_ids first and mamba2_cache_params.layers second;
(c) [] ONLY on a positive dense proof; (d) otherwise a NAMED REFUSAL. The
caller's `return None` is now sound rather than a hole: an empty list can only
mean a proven dense model.

BEHAVIOUR CHANGE THE NEXT BOOT WILL SHOW, stated rather than discovered. A
hybrid whose config class exposes neither property now REFUSES the boot instead
of silently running KV-only -- the conversion the caller's docstring demanded
("this must succeed or attach fails"), with the remedy in the error text. And
because the window is now actually built, derive_mamba_blob_spec is reached for
the first time; if this checkpoint's config lacks a GDN field it will say so
loudly. So the first boot on this commit yields either "sgl-project#706 canonical GDN blob
active" three times or a named refusal. Both are results. Neither is silent, and
silence was the defect.

TEST RESULTS (desk, CUDA_VISIBLE_DEVICES="", /spinning/htsglang-gpu/.venv):
 * test_canonical_gdn_blob_attaches_931.py -- 5 passed. Five arms: a
   Qwen3-Next-shaped hybrid resolves its 48 GDN layer ids; the wrapped source
   (mamba2_cache_params.layers) resolves too; a proven-dense model returns []
   (a ladder that refuses everything is an outage); an unresolvable hybrid
   RAISES; and _canonical_mamba_window gets PAST its early return -- the attach
   itself, probed by making the next call raise a sentinel, because a green
   resolver beside a caller still holding its own getattr looks identical from
   outside.
 * MUTATION, one per claim:
     M1 caller reverts to the bare getattr on ModelConfig   1 failed (the attach arm)
     M2 resolver aims at ModelConfig instead of hf_text_config  4 failed
     M3 the (d) refusal degrades back to a silent skip      1 failed (the refusal arm)
   restored: 5 passed.
 * REGRESSION: test/registered/unit/mem_cache/ -- 2822 passed, 927 skipped,
   471 subtests passed, 2 failed. Both are test_acceptance_emitters_758
   "RuntimeError: No CUDA GPUs are available" and fail identically on the
   parent; the desk has no GPU. The sgl-project#706 format suites
   (test_canonical_mamba_blob_706, test_canonical_page_store_706,
   test_mamba_phase_uniform_706, test_mamba_gates_the_hit_706) are inside that
   run and stay green.
 * ruff: canonical_page_store clean; cache_controller 13 errors before and 13
   after, all E402/F541 at lines 16-44 and 1414, none in the edited region.
   (First count of "0" was a broken extraction -- ANSI codes in the concise
   output defeated the per-file grep; recounted verbatim.)

NOT BUILT, AND NAMED RATHER THAN IMPLIED: the end-to-end content proof --
archive a PP-phase anchor through the fence, load it back in the TP phase,
compare against the reference state -- needs live pools and a storage backend
and is not hermetically cheap at the desk. It is a boot-proof item, and the
measurement window's coherence probe drives exactly that path.
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 27, 2026
…he allocator per access

sgl-project#927 IS A CHECKER-SIDE DEFECT, AND THE on_idle RAISE WAS A FALSE POSITIVE.
Nothing in the KV pool is repaired here, because the evidence now says nothing
in it was broken: the guard read the wrong object.

THE MECHANISM, and it is constructed rather than inferred.
`SchedulerInvariantChecker` stored `token_to_kv_pool_allocator` as a dataclass
field taken once at construction (`scheduler.init_invariant_checker`). The
phase flip REBINDS that allocator -- `hicache_phase_binding._stamp` sets
`token_to_kv_pool_allocator = incoming.allocator`, and `phase_pools_for` takes
that object from the incoming phase's OWN worker stack (`:341-343`), so it is a
DIFFERENT OBJECT per phase. But `readers_of` names exactly three readers --
scheduler, tree_cache, cache_controller -- and the checker is not one of them.
Its own docstring states the consequence in advance: "a reader this function
forgets is a reader the rebind silently leaves behind." Nothing else refreshed
it either; `vram_dial._refresh_capacity_snapshots` touches only
`max_total_num_tokens`.

So after the first cutover, `cache_controller.load` allocated load-back rows
from the INCOMING allocator (it IS rebound) while the ledger read the BOOT one.
Both address the same id space, so those rows read as FREE to the checker while
the tree legitimately named them, and `_live_double_claimed_rows` reported the
overlap as `double_owned src=live` -- in the magnitude of the loaded-back
prefix, on the rank that matched, at the instant `load_back` filled the nodes'
`value`. Every measured property of the crash, including the timing that
refuted four earlier candidates.

WHY THE TIMING FITS EXACTLY, which is what makes this the answer rather than
another candidate: the tree was refilled long before the hit, by
`_insert_helper_host`, which creates nodes carrying ONLY `host_value` (`:1788`)
-- `value` stays None, and `all_values_flatten` reads `value`. So the nodes were
invisible to the ledger from the cutover until `load_back` populated them. That
is why `double_owned` read 0 on every census and then jumped at the first hit.

THE FIX IS PER-ACCESS RESOLUTION, AND A `readers_of` ENTRY WOULD NOT HAVE
WORKED AT ALL. That was the smaller-looking option and it is unavailable: all
three affected components are `@dataclass(kw_only=True, slots=True,
frozen=True)`, and `_stamp` moves a reader by `setattr`. On a frozen instance
that raises, `rebind` catches it mid-set and escalates to `RebindIncoherent`
("One reader failed mid-way: the set is now split") -- so adding them to the
dict would have converted a silent stale read into a hard flip failure. Per
access is the only shape that works here, and it is also the one the next
component to hold a binding cannot be forgotten out of.
`_allocator()` derives the allocator when it reads, from a callable the
scheduler supplies -- the idiom this class already uses for `get_last_batch` /
`get_running_batch`. Absent getter falls back to the field, so constructions
outside the phase-flip boot are byte-identical.

SIBLING SWEEP, and the class had THREE members, not one. The class is
"construction-time reference to a rebindable object, on a component outside
`readers_of`":
  * `SchedulerInvariantChecker`      (invariant_checker.py:92)   -- FIXED
  * `SchedulerPoolStatsObserver`     (pool_stats_observer.py:144) -- FIXED;
    this one feeds the SAME ledger (`session_held`, the `available` fallback),
    so it was reading the stale pool alongside the checker.
  * `SchedulerDPAttnAdapter`         (dp_attn.py:270)             -- FIXED
All three now resolve per access and are wired from their construction sites.

BOOT PREDICTION, stated so it can be falsified: the three on_idle crashes
should DISAPPEAR with this commit, with nothing in the pool having been
repaired. If they do not, this is not the producer and the reframing above is
wrong.

MANDATORY RE-READ BEFORE ANYONE BUILDS ON THE OLD NUMBERS. If the ledger has
been auditing the boot phase's allocator since the first cutover, then every
post-flip reading it produced is suspect -- sgl-project#913's standing "live rows already
unmapped" lines on every boot, and the sgl-project#912 `withheld`/`available` readings,
included. They must be re-read on the next boot AFTER this fix before any of
them is treated as a measurement. This warning belongs in both task contexts.

ONE REGRESSION OF MY OWN, CAUGHT BY THE GATE AND FIXED IN THE DOUBLE. The
first full run came back 20 failed against 17 known pre-existing, and all three
new ones were `test_kv_page_invariants.py` with
`AttributeError: '_FakeChecker' object has no attribute '_allocator'`. That
double binds `_check_kv_page_invariants` off the real class but carried only
the FIELDS, so once the production method resolved through `self._allocator()`
the double stopped modelling production -- the drift class this tree has been
bitten by before ("the suite's own double had the attribute and not the method,
exactly backwards from production"). Fixed in the DOUBLE, not in production: it
now binds `_allocator` off the real class alongside the method under test.
Swept the siblings -- every other test touching these three classes uses
`_check_pool_invariant`, a @staticmethod with explicit args, so no other double
carries the same risk.

Tests, hermetic, CUDA_VISIBLE_DEVICES="": test_checker_reads_bound_pool_927.py
4 passed; the three sgl-project#927 files together 10 passed; test_kv_page_invariants.py
5 passed. Combined mem_cache+managers gate: 17 failed / 7477 passed, and the
17 are NAME-IDENTICAL to f1a3391's -- zero new failures. ruff: 105 errors at
base and 105 now across the four touched files, none added. Genuinely red-first --
mutating `_allocator` back to the construction reference turns
`test_the_checker_resolves_the_live_allocator` RED while the rest stay green,
then restored. The characterisation test reproduces the crash as arithmetic:
rows allocated from the INCOMING allocator and held by the tree read as N
doubly-claimed against the BOOT allocator and 0 against the bound one.

OWNERSHIP BOUNDARY held: the mamba twin of the same host re-population
(`mamba_exist=True` out of `_insert_helper_host`) is the sgl-project#928 agent's, shared
and not touched here.
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 27, 2026
…spec in sgl-project#936

NOT A FIX. This adds one function and calls it from nowhere. It is committed
alone so it cannot be lost in a worktree, and so the successor building sgl-project#936
starts from a written rule rather than from a description of one.

WHAT IT IS. `lower_protected_to(req, new_protected_len)` lowers
`req.cache_protected_len` by `min`, never assigns and never raises it.
`cache_protected_len` means "the tree owns the KV below this" -- the premise
both free-sites in `cache_finished_req` rest on and the one
`retention_shrinks_protected` states in this same module. Raising it would
promise ownership nobody holds; a flat assignment would clobber a HIGHER claim
a later legitimate match had earned. So: min.

WHY IT EXISTS. When the tree does NOT take a request's prefix, the promise is
false for the rows that request still holds, and the two branches that free
FROM `cache_protected_len` while inserting nothing then skip over them:

  * `unified_radix_cache.cache_unfinished_req`, the `effective_cache_len <= 0`
    skip branch -- frees `kv_indices[req.cache_protected_len:]` (:1196) and
    returns (:1212) without inserting and without lowering cpl;
  * `unified_radix_cache.cache_finished_req`, the `is_insert=False` branch
    (:1130-1131) -- identical shape.

The `effective_cache_len <= 0` that opens the first one comes from mamba's
`_decline_retention(is_finished=False)`, which returns 0. That refusal is
measured, not inferred -- boot 2g, in its own words: "mamba retention would
truncate a protected prefix: tracked position 4096 under cache_protected_len
8192, caching without a mamba anchor".

THE RULE sgl-project#936 SHOULD APPLY: if the tree does not take the prefix, there is
nothing left to protect -- cpl falls in the SAME moment to what the tree
actually holds (0 when nothing was inserted), and the free then covers
everything. At the moment of the refusal, never blanket on the request: a
later successful match may legitimately grant a protected prefix again.

EXPECTED TO BE REPLACED. sgl-project#930's `truncate_prefix_to` carries the same
min-never-assign discipline; when it lands, sgl-project#936 should use it and delete this
one. Two helpers for one rule is the shape that lets them drift.

sgl-project#935's guard (fa7dd3b) is the falsifier for that work and stays as it is: it
MEASURES the orphaned interval rather than repairing it, so if it still fires
after sgl-project#936 there is a second source and it names the share.

Desk checks only, as befits an unwired helper: py_compile clean, ruff clean.
No test, because a function with no caller has no behaviour to pin -- the
red-first test belongs with the wiring, and its shape is specified in sgl-project#936
(mamba retention refused + a full cache_protected_len, asserting the interval
is neither inserted nor freed today).
efschu pushed a commit to efschu/htsglang that referenced this pull request Aug 27, 2026
…ted_len with it

`cache_protected_len` is HOW MANY LEADING ROWS OF THIS REQUEST'S KV THE TREE
OWNS. `init_next_round_input` sets it equal to `len(prefix_indices)`. The sgl-project#791
admission-uniformity block in `_get_new_batch_prefill_raw` then truncates
`prefix_indices` to the PP-agreed `told` -- on PP0 from the guard's clamped
candidate, downstream from PP0's decision -- and NEITHER branch touched the
protected length. The request was left claiming more tree-owned rows than it
holds.

I FILED THIS AS "THE SAFE DIRECTION" AND THAT WAS THE WRONG HALF. The surplus
IS harmless for `_insert_helper`'s duplicate free -- a larger `dup_start` frees
less -- which is exactly why it never surfaced as a double claim. It is the
DANGEROUS direction for `cache_finished_req`'s truncate branch
(`unified_radix_cache.py:1111-1116`):

    free_start = max(effective_cache_len, req.cache_protected_len)
    free(kv_indices[free_start:])    # starts ABOVE the interval
    ...                              # the insert covers only up to ecl

With `cache_protected_len > effective_cache_len` the rows in
`[effective_cache_len, cache_protected_len)` are neither freed nor inserted and
belong to nobody afterwards. That interval is sgl-project#935's per-request row leak
(36824 rows on the 2i acceptance boot).

SCOPE, kept sharp so the two tickets do not blur. The GAP is the root and is
sgl-project#935's: it must not be able to leak whatever the value is. This closes one of
the two PRODUCERS that make it reachable; the other is the sgl-project#928 refusal
re-prefill. Closing a producer does not close the gap, and closing the gap
makes the producers harmless -- both are owed, and neither substitutes for the
other. Different files, no collision with fix/935-finished-req-gap.

ONE HELPER, TWO SITES, because the two sites are siblings of each other and
drifted identically -- both sliced `prefix_indices` by hand and both forgot the
same field. `Req.truncate_prefix_to(told)` now owns the pair, and the wiring
pin asserts neither branch slices by hand again.

MIN, NEVER ASSIGN: the helper may only LOWER the claim. A request whose
protected length was already below `told` owns exactly that many, and raising
it here would invent protection the tree never granted -- which is the
dangerous direction for the duplicate free, i.e. the defect this commit is
NOT allowed to trade for. Pinned in both directions.

Tests, hermetic, CUDA_VISIBLE_DEVICES="":
test_truncation_keeps_protected_len_930.py, 6 passed. Combined
mem_cache+managers gate: 17 failed / 7483 passed, the 17 NAME-IDENTICAL to
f1a3391's -- zero new failures. Genuinely red-first with
the mutant the ticket names -- restoring "slice the prefix, never touch cpl"
turns 3 red, including the consequence test that computes the abandoned
interval from the real `max(ecl, cpl)` arithmetic, while the may-only-lower
direction stays green. Restored after.
efschu pushed a commit to efschu/htsglang that referenced this pull request Sep 1, 2026
…d a pp_to_tp/tp_to_pp livelock, 0 decode

Bisected against a51e5e8 (last decode-green boot, 297 decode batches).
git bisect narrowed to an ambiguous parent/child pair
(96bdb4a / 67b08b8) after several sgl-project#631-style crash-only commits
were skipped; direct-parent verification (67b08b8^ == fdb7b21, the
last confirmed-good commit) resolved it definitively in favor of
67b08b8 ("[sgl-project#1033] The flip hold asked half the one-chunk grant").

That commit added two new terms to the pp_to_tp/tp_to_pp quiescence
predicates: `fits_in_one_chunk` in `prefill_runnable_in_current_layout`
and a new `completed_chunk_pages_acked` ack-gate consumed by
`chunk_blocks_quiescence`. Root-caused via a first, WRONG partial revert
(kept `fits_in_one_chunk`, reverted only the ack-gate) that still failed
on the metal with a NEW symptom: 51 flips alternating every 27-40s (a
flip-flop livelock, 0 decode) plus a separate sgl-project#928-anchor retraction loop
on one request. Reading the full original diff showed the pre-sgl-project#1033 code
never had an ack-gate either (unconditional `return False`), so that half
of my hand revert was a no-op; `fits_in_one_chunk` was the actual driver.

This reverts 67b08b8's entire diff (both functions plus the
scheduler.py wiring in `build_flip_quiescence_fn`), applied at the
current branch tip via `git revert --no-commit` rather than a blanket
file-copy from the old commit: none of the 12 commits that touched these
two files between 67b08b8 and this tip touch these functions (only
one, [#1054d], and it edits an unrelated region ~11000 lines away), so
the revert applied with zero conflicts and preserves every later commit's
work in both files intact.

Per upstream-minimal: the sgl-project#1033 mechanism was itself a fork-own
compensation layer over the sgl-project#887 one-chunk grant; its own defect (an
unreachable-in-practice "both halves must agree" premise that instead
produced a livelock) makes it a deletion candidate, not a repair target.
Deleted the now-stale devtools/check_1033_quiescence_edge.py alongside it
(it exercised `fits_in_one_chunk`/`completed_chunk_pages_acked`, both
gone).

Test results (BOOT-PROVEN, two independent boots, driver load
disc631_driver.py --duration 480 --words 24000 --concurrency 2):
  - boot_855_fullrevert1033_67b08b87b6_0901_044806.log (smoke, at the
    bisect commit): VERDICT=GOOD, 15 decode batches, single clean
    pp_to_tp flip (no flip-flop), 0 crashes.
  - boot_855_tiprevert1033_2b23c255fb_0901_050011.log (this exact commit
    content, at branch tip): VERDICT=GOOD, 12 decode batches (accept
    rate 0.66), single clean pp_to_tp flip (3 reconciled rank-flips, no
    flip-flop), 0 Tracebacks, 0 sgl-project#631 crash markers, 0 sgl-project#928-retraction
    markers.

Not yet run: the >=30 min real-router-agent-load acceptance test
(Phase 3). This commit closes Phase 1/2 (bisection + root-cause fix,
boot-proven) only.
efschu pushed a commit to efschu/htsglang that referenced this pull request Sep 15, 2026
… Prefetch-Verdikt TOLD auf der Request-Leitung

Befund (Boot xsn116, 2026-09-15 17:32:17, rid e10588ca, zweite Anfrage mit
identischem Praefix, D hatte ihn durchgeschrieben): jeder P-Rang registriert
seinen HiCache-Storage-Prefetch, keiner wartet darauf (sgl-project#973 PP0 PREFETCH WAIT
DISARMED / #969Z), der spaeter fertige Host-Treffer wird als sgl-project#1245
'undistributable' verworfen, damit die Raenge gleich bleiben -- P prefillt
einen im Store byte-vollstaendigen Praefix neu (STORE WITNESS state=unprobed
loaded=0, 'sgl-project#928 anchor REFUSING resume ... re-prefilling'). D leg 2 liest
denselben Store korrekt (cached_tokens=4314/4316), weil der TP-Pfad wartet
(Policy timeout) und MIN-reduziert.

Fix: der Traeger ist die Request-Leitung, die es schon gibt (recv_reqs wird
je Pass PP0->PP1->PP2 gepickelt weitergereicht; sgl-project#1268 fix 1c faehrt dort
schon Weg2IdleVoteReq). Protokoll je rid:
  1. Intake: PP0 registriert wie bisher und haelt; Folgestufe registriert
     NICHTS ('declined:weg2_held', neuer sgl-project#915-Partitionsterm).
  2. PP0 vor dem Forward: fuer jeden gehaltenen rid mit terminiertem
     Prefetch (check_prefetch_progress, kollektivfrei bei tp_size 1) wird
     die geladene Zahl ohne Pop gelesen, als told gespeichert und als
     Weg2StoreTold(rid, told) auf die Leitung gehaengt. PP0 admittet nur,
     was es auf die Leitung gelegt hat.
  3. Folgestufe nach dem Forward, vor dem Dispatch: nimmt die Objekte aus
     der Liste, speichert told, registriert den Prefetch fuer EXAKT told
     Token (_prefetch_kvcache(limit_tokens=told)) -- gleiche Seiten, gleiche
     Laenge, gleicher Store: Host-Baum und Load-Back-Extent sind inhaltlich
     uniform, keine Praefix-Truncation an einem Mamba-Anker noetig.
  4. Admission auf jedem Rang: ohne told -> Skip 'weg2_store_told_pending';
     mit told -> bounded Warten auf den eigenen Read, Pop, und benannte
     Verweigerung Weg2StoreToldMismatch bei own != told (Raenge nie uneins).
Der sgl-project#1245-Drop bleibt fuer die traegerlose Form ohne told; mit told ist der
Extent verteilbar. Arm nur bei pp_size>1, tp_size==1, Storage an, kein
Zeilentraeger; Kill-Switch SGLANG_WEG2_STORE_TOLD=0. pp_size<=1 (Gruppe D)
unveraendert.

Tests: test_weg2_store_told_1400.py 12 gruen; Mutant (Admission ohne Verdikt
+ Mismatch-Refusal aus) 3 rot. 16 Test-Dateien der beruehrten Symbole:
34 rot vor und nach der Aenderung identisch (Sockel: Doubles ohne ps /
_pp_post_send / _pp_send_admission_decision), 0 neu. Provider-Smoke 19/19.
Metall-Abnahme steht aus (arm_xsn117): zweite Anfrage gleicher Praefix ->
P leg 1 cached_tokens>0 und 0x 'sgl-project#928 anchor REFUSING'.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants