Repository navigation
Conversation
There was a problem hiding this comment.
Verdict: MUST_FIX: The join gate accepts NaNs, and the change adds duplicate logic and unused state.
Must fix
1. Reject non-finite output in the join gate
gate_qwen35moe_mtp.rs:342 f64::max discards NaNs. Replacing the hidden-half store in mtp.rs:184 with f32::NAN therefore makes wh zero. Clause (j) still passes: its four-stream comparison checks only the embedding half, and the mutant raises no fault.
Fixing this now prevents the new numerical gate from certifying invalid output. Replace one_ok with:
let one_ok = one[..2 * HIDDEN].iter().all(|v| v.is_finite()) && we <= band && wh <= band;2. Reuse the shared argmax owner (R14)
gate_qwen35moe_mtp.rs:139 repeats the same first-index tie rule as shared/e2e.rs:33. Only the return type differs.
Reusing the existing owner removes the second implementation and its maintenance cost. Delete first_argmax, add this beside main, and replace its call below:
#[cfg(feature = "gpu")]
#[path = "shared/e2e.rs"]
#[allow(dead_code, reason = "this gate uses only the shared argmax")]
mod e2e;let at = super::e2e::argmax(&logits) as usize;3. Remove the head-gain name retained after construction
mtp35.rs:56 stores Names::head_gain for the draft's lifetime. Its only read is during construction; Head::with_gain copies it into Head::gain, and the public getter reads that copy.
Removing the unused field now avoids establishing two owners for the same name. Delete the field and its initializer. Replace the second argument to Head::with_gain with:
(&w, &super::body35::blk(
index, model::arch::qwen35moe::mtp::SHARED_HEAD_NORM,
)),Non-blocking
⚠ Cover the draft's K/V row contract: gate_qwen35moe_mtp.rs:461. The gate's header documents that attending to one row stays above this floor; the PR reports the row-zero mutant passing. Add a case whose assertion fails when earlier K/V rows are lost.
Checked
Duplicates: both argmax implementations and both retained gain names were compared. Dryer reported zero pairs and excluded gpu-gates; the argmax duplicate was checked manually.
Complexity: Head::with_gain (18) and enqueue_over (16) move existing constructor/dispatch branches. New Mtp35::enqueue (16) combines validation, join and layer/head stages; the launches already have owners.
Silent failure: four new display defaults followed; each remains paired with a failing refusal verdict. The numerical NaN fold is finding 1.
Tests: static tracing of a NaN hidden-half store shows that (j) accepts it. The PR reports red mutants for gains, borrowing and open refusals, and surviving position/K/V mutants; those card runs were not reproduced.
Gates: no existing pin or clause relaxed; the PR reports FAIL-first evidence for the new acceptance floor. Escape catalog C1–C4: no match.
Adversarial checks: invalid token, short hidden row and position at context size are refused before writes; borrowed addresses are checked before launch; non-finite hidden input raises a fault and poisons the model.
Notes
GPU gates were not executed in this review. The PR summarizes card runs but omits the required nvidia-smi line and full gate command output; GPU evidence is recorded as false.
Card check: needs-gpu. This pull request changes crates/gpu-gates/Cargo.toml, crates/gpu-gates/src/bin/gate_qwen35moe_mtp.rs, crates/gpu/src/arch/qwen3moe/body35.rs … and carries no gate run on a card (CONTRIBUTING's evidence list); a maintainer adds gpu-verified after one.
🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)
| worst(&one[..HIDDEN], &re), | ||
| worst(&one[HIDDEN..2 * HIDDEN], &rh), | ||
| ); | ||
| let one_ok = we <= band && wh <= band; |
There was a problem hiding this comment.
The error fold discards NaNs, so reject non-finite join output before passing the band.
| let one_ok = we <= band && wh <= band; | |
| let one_ok = one[..2 * HIDDEN].iter().all(|v| v.is_finite()) && we <= band && wh <= band; |
There was a problem hiding this comment.
Verdict: MUST_FIX: the draft cache is shared across sequences, and the new gate duplicates an existing generator.
Must fix
1. Reject additional slots on an MTP load (kind A)
mtp35.rs:325 uses one draft K/V cache. open_mtp uses plain lanes, so add_slots(2) succeeds. Draft slot 0 at position 0, select slot 1 and draft its position 0, then select slot 0 and draft position 1: attention reads slot 1's row 0. body35.rs:2252 exchanges trunk state but leaves the draft cache in place. This violates the slot contract that calls act on the selected sequence.
Rejecting the combination now prevents draft history from crossing requests. Insert this after WHAT_N, before the lane check in Body35::new_seq:
if self.mtp.is_some() {
return Err(GpuError::shape(
WHAT_N,
"an in-file MTP load supports one resident sequence",
));
}Extend clause (r) to assert that add_slots(2) fails and leaves one slot. Removing this guard must turn that assertion red.
2. Reuse the gate's xorshift owner (kind B, R14)
gate_qwen35moe_mtp.rs:259 repeats the shifts and high-bit sampling already owned by lib.rs:490. Reusing GateRng removes the second algorithm and reduces the function from 11 lines to 6. Both formulas preserve the same generated inputs.
Replace values with:
fn values(n: usize, seed: u64, scale: f32) -> Vec<f32> {
let mut rng = bloomery_gpu_gates::GateRng(seed);
(0..n)
.map(|_| (2.0 * rng.unit() - 1.0) * scale)
.collect()
}Checked
Duplicates: the new generator and existing owner were read. Dryer reported zero pairs and excluded gpu-gates. The replacement matched all 20,480 f32 samples from the gate's four synthetic inputs in a host arithmetic check.
Complexity: diagnostics flag Head::with_gain (18), Head::enqueue_over (16), and Mtp35::enqueue (16).
Silent failure: token, position and hidden-row refusals precede the draft's first device write.
Tests: clause (r) checks those refusals and a repeated step; it does not exercise slot interleaving. The PR reports a swapped-gain mutant failing the acceptance clause and surviving position-offset and row-zero mutants.
Gates: card gates were not run during validation. The PR states that the final head was not rerun and supplies no nvidia-smi output line.
Escape catalog: neither retained finding matches C1–C4.
Card check: needs-gpu. This pull request changes crates/gpu-gates/Cargo.toml, crates/gpu-gates/src/bin/gate_qwen35moe_mtp.rs, crates/gpu/src/arch/qwen3moe/body35.rs … and carries no gate run on a card (CONTRIBUTING's evidence list); a maintainer adds gpu-verified after one.
🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)
- Gate (d) derives the draft's card bytes from the tensor table (bf16 widened where the card widens it); the in-file draft-bytes API and Mtp35::ctx are gone. - (s)'s 30 % floor is part of its verdict, so a miswired join or gain is red. - Body35's resident bytes count the draft and its step scratch. - One owner each: the borrowed head's Q8_0 planes (mtp38::planes), the output_norm name (model::arch::names), the nextn stems (qwen35moe::mtp), the layer-name format (body35::blk); the head calls f32_gain directly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mtp35::check_borrowed repeated mtp38::at_open. at_open reads mtp35::addresses, and the draft step calls it. The caller names itself in the refusal.
…me have one owner Clause (j) rejects a non-finite pack before the band: the error fold drops a NaN, so the hidden half could pass. The draft gate takes the shared argmax. Names no longer keeps the head gain; the head's own name is what the gate reads.
The draft K/V is one cache on the body. Body35::new_seq refuses a second resident sequence, and clause (r) requires add_slots(2) to fail and leave the one slot. The draft gate draws its synthetic inputs from GateRng.
GpuError stores the caller's name as &'static str. at_open took &str, so the name escaped the function and the card build stopped.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Verdict: CLEAN: No must-fix finding remains.
Non-blocking
⚠ Add a history-sensitive draft check: crates/gpu-gates/src/bin/gate_qwen35moe_mtp.rs:451. The PR reports that forcing every draft position to zero still passes (s), with 68.8% agreement over the 64-id prompt's last 16 positions on the RTX A6000, above the 30% floor. The gate header confirms that single-row attention still passes. Add a reference-logit or longer-history clause that fails this mutant before generation uses the draft; checking this while the draft remains isolated makes a later KV wiring failure easier to locate.
Checked
Duplicates: dryer reported zero qualifying pairs in crates/gpu and crates/model; gpu-gates was excluded from that scan.
Complexity: crapper reported Head::with_gain (18), Head::enqueue_over (16) and Mtp35::enqueue (16). The draft enqueue combines input checks with an ordered launch sequence using the existing layer dispatch.
Silent failure: Mtp35::enqueue checks token, position and hidden-row length before launches. mtp_draft passes its result through note_fault.
Tests: (h) compares the returned token with the host argmax of device logits. Clause (r) requires named errors for invalid inputs and refuses a second resident sequence.
Gates: the new binary declares its required GPU feature. Its acceptance floor is added with the new gate.
Escape catalog C1–C4: no override applies to the remaining suggestion. No ledger lessons are in force.
Notes
Static validation only; hardware gates and mutation results were not rerun. The mutation rate above is reported in the PR.
Card check: gpu-evidence. This pull request changes crates/gpu-gates/Cargo.toml, crates/gpu-gates/src/bin/gate_qwen35moe_mtp.rs, crates/gpu/src/arch/qwen3moe/body35.rs … and carries a gate run on the author's card, which counts as verified.
🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)
There was a problem hiding this comment.
Verdict: CLEAN: No must-fix finding remains after validation.
Non-blocking
⚠ Verify the shared four-stream kernel: crates/gpu/src/mtp.rs:168. Four fixed embedding stores became a loop bounded by runtime streams: 4 × hidden scalar store iterations per Qwen3.8 draft row [derived]. Attach the base/head PTX and resource comparison and both families’ owning gate outputs, including FAIL-first for the new gate. Include a timed A/B if occupancy changes. Supply the card/driver line, both commits, model path and commands with output.
⚠ Pin draft attention history: crates/gpu-gates/src/bin/gate_qwen35moe_mtp.rs:451. The gate documents that attending only one row still clears its acceptance floor. Forcing the position passed to StepParams::write at mtp35.rs:289 to zero makes the live key count one. The argmax and refusal checks do not assert accumulated history. Add a history assertion that turns red on that mutant before wiring the draft into generation.
Checked
Duplicates: dryer reports zero qualifying pairs in gpu/model; gpu-gates was excluded from that diagnostic.
Complexity: Head::with_gain (18) and Head::enqueue_over (16) move existing branches. Mtp35::enqueue (16) adds input guards and a linear dispatch sequence.
Silent failure: the five new fallback sites supply display text; separate checks retain the failure verdict.
Tests: assertions inspected statically. Clause (j) checks join layout, (h) checks the returned argmax, and (q)/(r) check refusals. The history mutant lacks a dedicated assertion.
Gates: no existing threshold, case or gate was relaxed or removed. The new acceptance floor is 30% over the last 16 comparisons from the pinned model’s 64-id prompt.
Escape catalog: no C1–C4 pattern matched. Ledger: no lessons in force.
Notes
GPU gates were not executed during validation. The supplied PR body lacks the card/driver line and commands with output required by the GPU evidence list.
Card check: needs-gpu. This pull request changes crates/gpu-gates/Cargo.toml, crates/gpu-gates/src/bin/gate_qwen35moe_mtp.rs, crates/gpu/src/arch/qwen3moe/body35.rs … and carries no gate run on a card (CONTRIBUTING's evidence list); a maintainer adds gpu-verified after one.
🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Since the last review: no must-fix items were outstanding. The earlier advisories concern code unchanged by this increment.
Verdict: CLEAN: The incremental diff only changes whitespace in the gate's values expression.
Checked
Duplicates: no new items; the supplied dryer report has no qualifying pairs and excludes gpu-gates.
Complexity: no branches changed; the diagnostic entries concern earlier commits.
Silent failure: 0 new sites.
Tests: the removed and added expressions match after stripping whitespace at crates/gpu-gates/src/bin/gate_qwen35moe_mtp.rs:263. Iteration order, RNG calls and arithmetic grouping are unchanged.
Gates: no pin, threshold, clause, or case changed.
Escape catalog: no C1–C4 pattern matched. Ledger: no lessons in force.
Notes
Validation was static; GPU gates were not run.
Card check: needs-gpu. This pull request changes crates/gpu-gates/Cargo.toml, crates/gpu-gates/src/bin/gate_qwen35moe_mtp.rs, crates/gpu/src/arch/qwen3moe/body35.rs … and carries no gate run on a card (CONTRIBUTING's evidence list); a maintainer adds gpu-verified after one.
🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)
Part of #30 (qwen35moe NextN MTP draft), step M2a-1: one draft step on the card. Head
ce77eb1f(rebased onto main after #235; gate-paths.tsv refilled; same code as APPROVEDf1496c67plus recipes fill). Parent chain rebased; force-with-lease 17:01 KST.What
Mtp35(crates/gpu/src/arch/qwen3moe/mtp35.rs) runs the in-file NextN block of a Qwen3.6-35B-A3B file (layer 40):eh_proj · [enorm(embed(t)) | hnorm(h)], over one stream.dispatch::layerwith its own KV planes. This is the same attention and routed-MoE path as the trunk, not a copy.shared_head_normand the target'soutput, borrowed at the same device address.Body35 opens the draft from the same file after
qwen35moe::mtp::in_filesaysDraft; any other answer is refused by name. No serve seat, lever or default change: the draft is not wired into generation yet (M3).Evidence (unchanged from APPROVED head)
ce77eb1f(same diff asf1496c67+ gate-paths refill).f1496c67.f1496c67; this push is the post-gpu, model, gpu-gates, tools: the common pieces a Nemotron-H card body attaches to #235 rebase only.See prior body for full FAIL-first and commonization notes.
🤖 Generated with Claude Code
The bloomery AI assistant wrote this.