Skip to content

metal: message marks kept inside one prompt call (prefill_marked) - #792

Merged
midagedev merged 8 commits into
mainfrom
mac-marks
Oct 24, 2026
Merged

midagedev merged 8 commits into
mainfrom
mac-marks

Conversation

@midagedev

@midagedev midagedev commented Oct 24, 2026 •

Copy link
Copy Markdown
Owner

One Metal prompt call per request, with points at the message starts inside it. Part of #593.

The server cuts a prompt call at the first and the last message start, so a later request can be cut back there and keep the recurrent state. The Metal engine asked for every cut, so chat turn 2 ran as three calls (5, 16 and 4 ids), each paying a pass or steps and a checkpoint. Its doc said such a cut is bit-neutral; since the batched pass it is not (a part of up to 8 ids steps, a longer one passes).

  • Engine::prefill_marked(ids, media, marks) (serve): the server runs every part of a prompt call through it, with the marks that fall in that part. The default is prefill / prefill_media and reads no mark, so every other engine runs as before, byte for byte. SwapEngine forwards text calls.
  • Target::prompt_marked (runtime): the default runs one prompt a part, cut at each mark.
  • The Metal engine cuts nowhere and hands the marks to Session's prompt_marked. A mark inside a pass is a copy the pass leaves after the row before it (StateOut::after_rows, up to two a pass), taken as a point once the pass settles. A mark inside a run of steps ends that run. A third mark ends the pass there.
  • Delta kernels: gdn_rows_snap writes the lane's registers after each named row into a copy; gdn_ring_snap writes the ring a chunk ending at that row would leave, before the ring commit. The pass's own outputs do not move. The copy buffers open on the first pass that takes one.

Gates:

  • kernels delta_rows_snap: each copy equals, bit for bit, the lane and ring of a launch over rows 0..=r, and the copies move no output. FAIL-first: a copy one row late, a ring not copied.
  • e2e q_mark: copies move none of the pass's bits (pick, logits row, every store); the copy after the last row, cut back to, is the pass's end state; a copy's ring slots before the pass are the ring the pass found; a copy is the same beside any other. FAIL-first: a copy one row late, a ring not copied, a point taken from the live ring.
  • e2e q_mark_cut: 1024 ids marked at 700 and 1017, cut back to each and continued twice: the same bits both times, and four teacher-forced tokens within q_free's band of the stepped run.
  • e2e k_keep: prefill_splits cuts nowhere and prefill_marked keeps each mark.
  • serve: the genloop hands each part its marks (default engine: the same calls as before); an engine that keeps marks gets one call with them.

Gates (M1 Pro, fixture), each rc 0 at this branch's head (after the review fix: cargo fmt --all --check, check-comments, just mac-metal-lint, just lab-metal, just lab-metal-e2e again): cargo fmt --all --check, check-unsafe, check-comments, just check-recipes, just mac-metal-lint, just mac-lint (3 warnings, the ratchet's 3), cargo test -p bloomery-serve --lib --test serve -- --include-ignored (367 + 106 passed), cargo test -p bloomery-runtime (289 passed), just lab-metal, just lab-metal-e2e. Output lines:

clause q_mark: PASS (40 ids as one pass copying after rows [6, 25, 39] two at a time: the pick, logits row and every store of the pass with none; the copy after the last row is the pass's end, the copy after row 6 keeps the found ring before the pass, and each copy the same beside either other)
clause q_mark_cut: PASS (1,024 ids in passes of 512 marked at [700, 1017], cut back to each and continued twice: the same pick, logits row and stores; from 1017: 4 teacher-forced tokens worst layer output 1.319e-3 of the stepped run's (band 0.26), 0 router sites with other picks (cap 12), ...; from 700: ... 1.497e-1 of the stepped run's (band 0.26), 8 router sites with other picks (cap 12), ...)
gate_metal_qwen35moe_kernels --mutants-all: PASS (114 mutants, each red on its clause)
gate_metal_qwen35moe_e2e --mutants-all: PASS (11 mutants, each red on its clause)

The CUDA engine is unchanged: its binding keeps prefill_splits and takes the default prefill_marked, so it runs the same calls as before.

M4 Pro 48 GB, Qwen3.6-35B-A3B MLX 4-bit, development-loop numbers (not public). Chat through bloomery-serve-metal -c 8192, greedy, base and after interleaved, twice each:

base after
turn 2 calls (ms) 55.4+80.4+43.5 / 51.9+80.7+43.4 102.5 / 101.9 (one call, points at 883 and 899)
turn 2 prompt_ms 190.8 / 187.0 113.9 / 113.2
turn 1 prompt_ms 1250.0 / 968.3 991.4 / 993.7

Turn 1 is not settled at two runs (the first base run was an outlier). The first marked pass also opens the copy buffers (about 142 MB) once per process.

The plain prompt path (no marks) is unchanged, interleaved, arm order rotated:

base after
873 ids, prompt tok/s 957.7 / 967.4 965.0 / 961.7
7985 ids, prompt tok/s 1001.5 / 999.1 998.5 / 996.5
pass wall 9 / 25 / 64 rows (ms) 61.03 / 79.84 / 113.14 61.52 / 79.78 / 113.10
decode control, tok/s 102.4 / 101.0 99.8 / 100.1

Evidence: base = main e294ea7; after = 0de8a60 (the fix after it changes the ring copy's slot index only near the last u32 position, which no timed run reaches). macOS 27.0.1, Apple M4 Pro 48 GB; model file Qwen3.6-35B-A3B-mlx4-00001-of-00001.gguf; generate_metal --prompt "$(cat p.txt)" -n 8 --ctx 8192, --pass-times 9,25,64, --prompt "The capital of France is" -n 64; release builds, one process per run. Load (vm.loadavg, 1 min) 2.97–7.98.

The code and gates were written with Claude Code rounds and reviewed by the maintainer.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Q5zTV27LbaVZFTjnkdmTjJ

midagedev and others added 6 commits October 25, 2026 04:58
…fault runs a prompt a part

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5zTV27LbaVZFTjnkdmTjJ
… it; the default is prefill

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5zTV27LbaVZFTjnkdmTjJ
…after named rows of a chunk

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5zTV27LbaVZFTjnkdmTjJ
…s each mark as a point in one call

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5zTV27LbaVZFTjnkdmTjJ
…stale bit-neutral claim

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5zTV27LbaVZFTjnkdmTjJ
@github-actions github-actions Bot added host Host crates: qdot, threads, engram, engram-lab, runtime, sampler, levers, decision, vision, refset serve The HTTP server: crates/serve labels Oct 24, 2026

@github-actions github-actions Bot 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.

Verdict: MUST_FIX: valid positions corrupt ring snapshots, and a new comparator duplicates an existing owner.

Must fix

1. Prevent overflow in the ring snapshot index

crates/metal/kernels/delta.metal:918 A public delta_rows call with pos = u32::MAX - 9, nine rows and after = [8] passes the position check at crates/metal/src/kernels/delta.rs:519. Adding the ring length wraps: ring slot 2 copies input row 1 instead of row 8, without a fault.
Fixing the index now prevents callers from restoring incorrect recurrent inputs. Replace the index calculation:

    uint back = (last % (uint)BLOOMERY_DELTA_RING_ROWS + (uint)BLOOMERY_DELTA_RING_ROWS - s)
                % (uint)BLOOMERY_DELTA_RING_ROWS;

The existing delta_rows_snap ring comparison detects this error with the boundary case added:

const SNAP_CASES: [SnapCase; 5] = [
    (12, 7, (0, 1), &[3, 9]),
    (64, 5, (0, 0), &[0, 40]),
    (9, 0, (0, 1), &[1, 8]),
    (20, 3, (1, 0), &[12]),
    (9, u32::MAX - 9, (0, 1), &[8]),
];

2. Reuse the existing bit comparator (R14)

crates/metal/src/bin/gate_metal_qwen35moe_kernels/clauses_rows.rs:427 duplicates crates/metal/src/bin/gate_metal_qwen35moe_kernels/clauses_rows.rs:326: both check equal lengths and compare each float's bits. Only the diagnostic differs.
Reusing the existing owner removes a function before its comparison logic diverges. Delete same_copy and its doc comment. Replace its two calls with:

            same_bits(
                &format!("{at}: copy {k}'s lane"),
                &copies.lane[k * lane_len..(k + 1) * lane_len],
                lane,
            )?;
            same_bits(
                &format!("{at}: copy {k}'s ring"),
                &copies.ring[k * ring_len..(k + 1) * ring_len],
                &end.ring,
            )?;

In same_bits, replace its diagnostic string with:

            "{what}[{i}] is {:?} ({:#010x}), expected {:?} ({:#010x})",

Non-blocking

⚠ Identify the timed revision: crates/metal/src/kernels/delta.rs:701 adds one ring-copy launch per delta layer per marked pass. Gate and rotated A/B results are supplied. Record the measured after commit hash instead of “this branch's tree” so the timing evidence identifies a fixed revision.

Checked

Duplicates: compared same_copy with same_bits and checked all SnapCase references.
Complexity: snapshot handling adds validation and a conditional ring-copy dispatch.
Silent failure: traced the position check through snapshot dispatch and indexing.
Tests: delta_rows_snap compares each copied ring with a prefix launch; the added boundary case exercises the overflow.
Gates: no band or tolerance changes in the reviewed snapshot hunks.
Escape catalog: no patterns matched the retained findings.

Notes

Metal gates were not run here; Clippy is unavailable. A host model confirmed the overflow and checked the replacement arithmetic against sequential ring writes in 464,706 slot cases.

🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)

Comment thread crates/metal/kernels/delta.metal Outdated
return;
}
uint last = args.pos + snaps.after[k];
uint back = (last + (uint)BLOOMERY_DELTA_RING_ROWS - s) % (uint)BLOOMERY_DELTA_RING_ROWS;

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.

Reduce the position before adding the ring length; valid positions near u32::MAX otherwise copy the wrong input row.

Suggested change
uint back = (last + (uint)BLOOMERY_DELTA_RING_ROWS - s) % (uint)BLOOMERY_DELTA_RING_ROWS;
uint back = (last % (uint)BLOOMERY_DELTA_RING_ROWS + (uint)BLOOMERY_DELTA_RING_ROWS - s)
% (uint)BLOOMERY_DELTA_RING_ROWS;

midagedev and others added 2 commits October 25, 2026 05:53
…, so a chunk at the u32 end does not wrap

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5zTV27LbaVZFTjnkdmTjJ
…2 end and compares with same_bits

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5zTV27LbaVZFTjnkdmTjJ

@github-actions github-actions Bot 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.

Since the last review:

Verdict: CLEAN: Both earlier must-fix items are resolved, and the incremental changes introduce no blocking defect.

Checked

Duplicates: 0 new functions, types or constants; the duplicate comparator was removed. Dryer reports 0 pairs.
Complexity: crapper's 10 flagged functions have no structural changes in this incremental diff.
Silent failure: 2 paths followed, snapshot validation and comparator errors; neither discards an error.
Tests: in a host model, restoring the old index at delta.metal:919 copies row 1 instead of row 8 into slot 2 in the added boundary case. The fixed index matches sequential ring writes in 2,083,796 slot cases, including empty initial history, one-row chunks and the final legal position.
Gates: one boundary case added; no bands, thresholds or clauses removed.
Escape catalog: no patterns matched.

Notes

Metal gates were not run here. Validation used source review and host arithmetic.

🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)

Filed outside this PR: #795

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

Labels

host Host crates: qdot, threads, engram, engram-lab, runtime, sampler, levers, decision, vision, refset serve The HTTP server: crates/serve

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant