Skip to content

fix(lanes): a pipelined stream adopted between rounds starts in verify - #928

Merged
ashhart merged 3 commits into
ashhart:mainfrom
chaog992:lane-adopt-pipelined
Oct 18, 2026
Merged

ashhart merged 3 commits into
ashhart:mainfrom
chaog992:lane-adopt-pipelined

Conversation

@chaog992

@chaog992 chaog992 commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

What this changes

A pipelined stream handed to the lane core with Engine.adopt (engine.zig:87) holds neither a mode nor a queued step. Its first pipelinedRound read the missing mode as pipe and unwrapped s.inflight.? (engine.zig:488), a step that was never queued: a panic in Debug and ReleaseSafe, undefined behaviour in ReleaseFast. It now starts such a stream as verify, which is where a drain step leaves one (engine.zig:494): it tries a copy window from pending, or queues pending and pipelines on as before.

Such a stream comes from detach (engine.zig:92, added with #780), which gives away a pipelined stream in verify or exit mode with no step queued and clears its mode, or from a lone driver that opened the stream itself (lane_host_lone.zig:71). The lane core queues a step when it opens a stream (open.zig:52), and every round on a stream it opened leaves one queued or leaves it in verify or exit. So only states that reach the null unwrap today take the new branch.

No shipped family reaches this. Lone drivers take only drafted requests (lane_host_lone.zig:18) and are installed only with a head or drafter (families/nemotron/cuda_native.zig:98, native/metal.zig:191, native/qwen27_served.zig:293, and on 1.0.12 native/xpu.zig:146), so their drafted streams take windowed rounds (lanes/config.zig:107, engine.zig:148). tensorfold run --hand-over (zig/src/main.zig:249) needs the head too (gpu_round.zig:283).

Refusing such a stream in detach would cover only the first way in, and fixing it in adopt would need step's windowed rule a third time, or a mode on every adopted windowed stream; pipelinedRound is the only code that acts on the mode. Starting in exit, or queuing pending and piping, also replies correctly but gives up the copy window: the hand-back test's copy-drafted stream then records 59 in s.rounds for its 60 tokens, against 27 with this change, the same as when it is never handed over.

Receipt

  • Environment: base c7137bb; fix 5d6a0ab; tests b0b630f and 721c29b (the head). Zig 0.17.0, Linux x86_64 under WSL2, no GPU or model. Line numbers are the base's unless marked, engine_test.zig's are the head's, and engine.zig is zig/src/core/lanes/engine.zig.
  • Tests, in engine_test.zig on the fake target. expectDecode builds the fake target's one-token decode to the case's max_new or first eos token and compares the whole reply and its finish reason. modelWith(false) is a family without a head, whose drafted streams step pipelined.
    • Hand-back test: a plain stream that a shared round leaves in verify, a drafted stream under the head (windowed rounds), and a drafted stream without a head that a drain step leaves in verify. The lone stream is detached and adopted whenever detach allows it, except in the step right after a hand-back, so a round also runs on what an adopted round leaves. Each set must hand back at least once.
    • Driver-opened test: a stream opened as lane_host_lone_test.zig's Lone opens one, then adopted, with and without a head.
    • The windowed set and the half with the head pass on the base too, so the shipped hand-back stays covered.
  • Base, the unfixed engine.zig with each test run alone: Debug panics with "attempt to use null value" at engine.zig:488:52, called from step (engine.zig:149); CI's zig build test builds these tests in Debug. ReleaseSafe panics too, with the site misattributed to depth.zig:262:81. ReleaseFast fails the whole-reply check (50 where the decode has 42 at index 4; 9 where it has 75 at index 1), an outcome undefined behaviour doesn't guarantee. The hand-back test in Debug, with the runner's counter, thread id, addresses, local paths, source lines and runner frames cut (the driver-opened test fails the same way, from engine_test.zig:545:57):
    engine_test.test.a lone stream handed to another driver between rounds and back replies as if kept, pipelined or windowed...thread … panic: attempt to use null value
    zig/src/core/lanes/engine.zig:488:52: … in pipelinedRound (lanes.zig)
    zig/src/core/lanes/engine.zig:149:93: … in step (lanes.zig)
    zig/src/core/lanes/engine_test.zig:503:28: … in test.a lone stream handed to another driver between rounds and back replies as if kept, pipelined or windowed (lanes.zig)
    
  • With the fix (at 721c29b): both tests pass in Debug, ReleaseSafe and ReleaseFast. Lanes 84/84 in each mode (82/82 on the base); engine_api with the lane host 110/110, as on the base. zig build test 46/46 steps and 745/750 tests with 5 skipped (743/748 on the base); test-golden (0 unexpected differences), fuzz-cortheon and zig fmt --check build.zig zig pass. Lean lists the same 527 problems as the base; the one in a touched file, engine.zig:86, is inherited.
  • On 1.0.12 (46a08c1): the three commits cherry-pick cleanly. Merged there, the No GPU job's steps pass locally: zig build test 84/84 steps and 1378/1385 tests with 7 skipped (1376/1383 there), test-kernel-contract, test-golden, fuzz-cortheon and lean. With 1.0.12's own engine.zig both tests panic in Debug at engine.zig:510:52.
  • Mutants: refusing such streams in detach instead fails the hand-back test's check that every set hands back. Ending or dropping an adopted stream at once, stopping one token early or running one late, or ending with the wrong reason each fail at least one test; the first version of expectDecode, which compared only the tokens a reply held, passed all of them. Leaving an adopted stream in pipe after its copy window, with no step queued, fails the hand-back test; both earlier versions of the test passed it.
  • Exactness: no device run. A local randomized check, not part of this PR, ran 20,000 random configs, with about 183,800 hand-backs and 13,000 streams another driver opened and handed over: with the fix every reply and its finish reason equalled the decode, and the base panics. A local probe that runs a copy of lane_host_lone_test.zig's serve at .mtp = false panics on the base on the lane host's thread, and gives exact replies with the fix.
  • Decode speed: no device measurement. The changed line runs in each pipelined decode round of a lone "draft": false request without logprobs or a grammar on Metal Nemotron and on GLM on one Mac. In an x86_64 ReleaseFast host fixture, single-step counts show one fewer instruction for each additional pipelined round with the mode unset, and no change with it set to pipe.
  • Prompt speed: n/a, since prompt passes don't reach pipelinedRound.
  • Not run: no Mac or device run of my own. cwschroeder's review below reports zig build test test-golden -Dcpu=apple_m1 passing on an M5 Max at 5d6a0ab, and the base failure reproduced there. The two test commits change only engine_test.zig, which here only cross-compiles for aarch64-macos. No Metal or CUDA run, since no shipped family reaches the new branch.

A stream adopted between rounds holds neither a mode nor a queued step: detach drops a verify or exit stream's mode, and a stream another driver opened never had one. pipelinedRound read the missing mode as pipe and unwrapped a step that was never queued; it now starts such a stream as verify, from its pending token.

@cwschroeder cwschroeder left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mac receipt for 5d6a0abf on its c7137bb3 base: M5 Max, macOS 26.6.2 (25G83), Zig 0.17.0.

zig build test test-golden -Dcpu=apple_m1 --summary all
zig fmt --check build.zig zig

Both pass: 164/164 build steps, 1985 tests passed and eight skipped. The frozen server answers have zero unexpected differences.

I also ran each new handoff test separately in Debug on an isolated copy of the core. Both pass on this head. Replacing only lanes/engine.zig with the unchanged base makes each fail with attempt to use null value in pipelinedRound. The Mac control therefore reproduces the failure your tests target.

Source review found no material issue in the mode selection: a missing queued step starts in verify; a queued step retains pipe, and explicit modes retain their meaning. One small test gap: expectDecode also accepts a correct short prefix. Comparing the terminal reason and expected length would catch a prematurely finished reply too.

These are host-side checks. No GPU, model, throughput or current-main integration result is claimed.

… reason

expectDecode compared only the tokens a reply held with the fake target's decode, so a reply that ended early passed. It now builds the decode to the case's max_new or first eos token and compares the whole reply and the stream's reason; like expectRows, it takes the case.
… leaves

The hand-back test detached the lone stream again at every chance, so when an adopted round left no step queued, no round ran on what it left: after a copy window that keeps more than one token, the stream still holds neither a mode nor a queued step and starts in verify again. It now skips the hand-back in the step right after one, and the copy-drafted set runs 60 tokens, so that round comes before its reply ends.
@chaog992

Copy link
Copy Markdown
Contributor Author

Thanks for the Mac run and the review, @cwschroeder.

You're right about expectDecode: it compared only the tokens a reply held, so a reply that ended early passed. A variant that ends an adopted stream at once, instead of starting it in verify, passed both tests. b0b630f builds the decode to the case's max_new or first eos token and compares the whole reply and the stream's finish reason. That variant now fails both tests, as do replies cut one token short or run one past max_new, and the hand-back test also catches a reply that runs past eos or ends with the wrong reason.

Looking for more of the same kind, I found one: the hand-back test handed the stream back at every chance, so when an adopted round left no step queued, no round ran on what it left. 721c29b skips the hand-back in the step right after one, so a round runs on what the adopted round leaves, and a variant that leaves an adopted stream in pipe after its copy window, with no step queued, now fails the hand-back test too.

With the fix, both tests pass in Debug, ReleaseSafe and ReleaseFast, and on the unchanged base they still panic in Debug at engine.zig:488:52. Both test commits change only engine_test.zig. On 1.0.12 (46a08c1) the three commits cherry-pick cleanly and the tests behave the same, with the unwrap at engine.zig:510:52. I've updated the receipt in the description.

@cwschroeder cwschroeder left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated assertions close the test-coverage gap from my earlier review. expectDecode now constructs the complete expected reply independently of the observed prefix and checks the finish reason. The extra settled round also exercises the state left by the adopted round before another hand-back.

I independently cherry-picked all three commits onto current main 46a08c18, preserving authorship. Both named adoption/hand-back regressions pass in Debug, Safe and Fast. The complete Lane Core CPU suite passes 120/120 tests there. With only engine.zig restored to unchanged main, each regression independently panics at the s.inflight.? unwrap at line 510.

Whole-tree formatting, whitespace and lean comparison pass (517 on base and port, zero new). Source review found no issue in the two test commits. All four checks on the original PR head 721c29b6 are green. My current-main verification covers the core CPU behavior; it is not a full-workflow, device or performance receipt for that port.

@ashhart ashhart mentioned this pull request Oct 17, 2026
@ashhart
ashhart merged commit 8bf9f9c into ashhart:main Oct 18, 2026
4 checks passed
@ashhart

ashhart commented Oct 18, 2026

Copy link
Copy Markdown
Owner

Thank you, @chaog992, for extending the adoption controls through the next round and comparing complete replies.

We merged your three commits for 1.1.0 in 8bf9f9c. An adopted stream without a mode or queued step enters verify from its pending token; existing modes and queued steps keep their meaning.

The complete-token and finish-reason assertions catch the earlier false passes. No shipped family reaches this adoption branch yet.

Posted by Ash's AI Assistant Synthena

BlivionIaG pushed a commit to BlivionIaG/TensorFold that referenced this pull request Oct 20, 2026
…n verify instead of unwrapping a step it never queued
@chaog992
chaog992 deleted the lane-adopt-pipelined branch October 21, 2026 07:53
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.

3 participants