#1212 Review: shared MTP head fix — sibling Q4 state omission and test configuration gaps

Description

# Focused shared MTP head review

Date: 2026-10-06
Verdict: concerns
Scope: current workspace diff in src/core/mtp.cpp, include/strata/core/mtp.hpp, CMakeLists.txt; new tests/core/mtp_shared_head_test.cpp; immediate callers and ownership paths. Read-only: no workspace changes, builds, or test execution.

## Findings

1. **Medium — existing sibling omission: shared slots lose Q4 projection state.** src/core/mtp.cpp:275-288 copies dense_, experts_, tensors_ but not dense4_/q4_off_; src/core/mtp.cpp:194-198 therefore dispatches the slots through original Q8_0 projections. src/program/generate.cpp:3570 configures Q4 only on the owner, and :3583-3592 creates slots through shared load without carrying that state. With --mtp-q4 proj/all, solo and batch-slot drafting use different projection formats. The new bind-only test bypasses load and cannot catch this sibling omission. Follow-up: explicitly share Q4 projections and offsets with correct ownership (destructor :154 currently unconditionally frees dense4_), or explicitly disallow/document the combination. Do not merely copy the pointer. No change requested to the focused head patch.

2. **Medium — new test is registered for builds that cannot execute its Q4 case.** CMakeLists.txt:462-468 registers unconditionally for GPU builds; tests/core/mtp_shared_head_test.cpp:91,121-124 unconditionally executes the Q4 override case. With STRATA_NATIVE_EXPERTS=OFF, src/core/mtp.cpp:258-260 returns false from make_q4_head, so this test fails on a GPU despite a valid non-native build. Guard only the Q4 case by STRATA_NATIVE_EXPERTS or arrange appropriate configuration-specific registration. Also the new target ignores STRATA_BUILD_TESTS, as do existing adjacent file_expert_source_test/vmm_test (:451-460) and expert_profile_save_test (:470-475); do not silently expand that cleanup.

3. **Low — inherited full-head diagnostic makes subset tests fail spuriously.** tests/core/mtp_shared_head_test.cpp:91-92 expects subsets regardless of environment; src/core/mtp.cpp:470-472,644 honors STRATA_MTP_FULL_HEAD=1 and then leaves dhead_type_=-1. The Q5_K subset case consequently expects 13 and fails. CMakeLists.txt:467-468 does not sanitize the environment. Set STRATA_MTP_FULL_HEAD=0 for this test before its first bind (the switch is cached), or set a test-specific environment in CTest.

## Ownership and comment audit

- New dhead_type_ is slot-owned scalar metadata; dvocab_host_ is a slot-owned vector copy. Both have object lifetime, not request lifetime; no per-request reset is needed. Device dhead_/dvocab_ remain owner-owned and are freed only when owns_draft_head_ is true (src/core/mtp.cpp:174-175). Ordinary bind failures leave cleanup to destruction. Owner is declared before slot_mtp in src/program/generate.cpp:3558-3562, so normal scope exit and early returns destroy slots first. No new lifetime defect found in the two copies.
- src/core/mtp.cpp:631-632 accurately explains format and token-map dependencies; record_rest :977-980 consumes the format and record_top2 :498 consumes the host map.
- tests/core/mtp_shared_head_test.cpp:2 and :31 overstate execution: native_mmvq_weight_bytes is a host format/size check, not a native_mmvq GPU launch; the explicit metadata assertion throws first if the copy is missing. Actual bind and allocation/free calls are exercised, but no draft output, top2 execution, or GPU MMVQ output is checked.
- Synthetic geometry/device/row-count friend-access comment matches the test setup. Diagnostics are environment-gated and contain no prompt/path values.

## Evidence and residual risk

Executed: git status --short; git diff --stat; git remote -v; git diff -- CMakeLists.txt include/strata/core/mtp.hpp src/core/mtp.cpp; git diff --check (no diagnostics); gh --version; gh auth status; date +%F. Read the new test, header, relevant mtp.cpp/native_head.cpp/CMakeLists.txt/gguf_fixture.hpp sections. Searched dhead_type_/dvocab_host_/dense4_/q4_off_, bind callers, slot_mtp/set_q4 wiring, full_head_env/coupled setup, native/test configuration, and WeightTable parsing with the repository grep tool.

Findings are static control-flow evidence, not runtime reproductions. The new real-bind assertions structurally cover deletion of either production metadata-copy assignment, but neither test execution nor a reverted-call/assignment failure was performed: the user prohibits builds and workspace edits, and the CUDA build is running separately. Fix is NOT claimed runtime-verified. No CUDA/HIP run, mutation test, or full-project review performed. Check both copy removals after the independent build, then run the test with the fix and STRATA_MTP_FULL_HEAD=1 and with a native-experts-disabled GPU build when available.