fix(rt-detr-v2): make needs_sanitize independent of HashMap order

[inureyes]

Summary

needs_sanitize returned from inside its weights.keys() walk as soon as either marker family was hit, so for any weight map carrying both families the verdict belonged to whichever key the randomized HashMap walk reached first. WeightMap is a std::collections::HashMap (src/lib/mlxcel-core/src/weights.rs:29) and RandomState is seeded per map instance, not per process, so the same checkpoint could sanitize on one run and not the next. This scans the full key set before deciding and then applies an explicit precedence, making the verdict a pure function of the key set.

The wrong direction is the expensive one. The module doc in the same file notes that MLX-layout checkpoints must skip the pipeline because re-running it double-transposes conv weights, so a spurious true corrupts the weights while the model still loads and still emits detections, just wrong ones.

Design decision: silent-but-deterministic, plus a warning

The issue asked whether a map carrying both families should be rejected loudly instead. It resolves toward MLX silently, with a tracing::warn!, for three reasons.

MLX-wins is the only recoverable direction, so it does not actually need an error to stay safe. If the map really was raw HF, skipping the pipeline fails loudly a moment later: RtDetrV2Model::from_weights looks up vision.backbone.* names the raw map does not have and returns Err. Running the pipeline over already-MLX weights has no such backstop, since a double-transposed conv is a shape-valid tensor that nothing downstream can flag.

Two of the six HF markers, .convolution. and .normalization., are unanchored substring tests, and the issue puts anchoring them out of scope. Making the mixed case fatal would build a hard load failure on top of a detector that is known to be over-broad, so any future false positive would refuse a checkpoint that loads correctly today. An imprecise heuristic should fail soft.

needs_sanitize is pub and returns a bare bool. Moving it to Result<bool, String> and threading the error through RtDetrV2Model::load is real API churn for a case the issue itself describes as latent rather than observed.

The tracing::warn! recovers most of what a hard error would have bought: it names one offending key from each family, which is what the issue asked a loud variant to report, and it needs no call-site change. It picks the lexicographically first match of each family rather than the first the walk meets, so the message is reproducible for the same checkpoint instead of reintroducing the ordering nondeterminism in the diagnostic. If a mixed-layout checkpoint ever shows up in practice, that warning is the evidence needed to justify promoting it to an error.

What changed

src/vision/detection/rt_detr_v2/sanitize.rs

• needs_sanitize now accumulates has_mlx_marker / has_hf_marker across every key and decides afterwards. MLX takes precedence over HF; HF wins when only HF markers are present; a map with no markers is unchanged at false.
• The marker tests moved into is_mlx_marker / is_hf_marker so the scan loop is visibly accumulate-only. The six HF marker strings and the two MLX prefixes are byte-for-byte unchanged.
• New warn_mixed_layout emits the both-families diagnostic.
• The // Early out once both decisions are unambiguous. comment is gone; it described a guard the code never implemented.
• The trailing has_hf_marker return is now an explicit false. At that point it could only ever have been false, but it read as though it could return true.
• New test needs_sanitize_mixed_markers_is_order_independent: 64 iterations, each building a fresh 7-key map holding both families, asserting a stable false. The map is rebuilt per iteration because RandomState varies per instance, so a single call cannot distinguish a fixed verdict from a lucky one.
• New test needs_sanitize_marker_free_maps_are_no_ops: pins the empty-map and unrecognized-key edge cases from the issue.

No change to src/vision/detection/rt_detr_v2/model.rs: the signature stays bool, so the call site at lines 110-115 is untouched. The rename rules, the transpose, and the drop rules are untouched.

Regression test evidence

The new test was written and run against the unfixed function first. It failed as expected:

running 14 tests
...
test vision::detection::rt_detr_v2::sanitize::tests::needs_sanitize_detects_layout ... ok
test vision::detection::rt_detr_v2::sanitize::tests::needs_sanitize_mixed_markers_is_order_independent ... FAILED

failures:

---- vision::detection::rt_detr_v2::sanitize::tests::needs_sanitize_mixed_markers_is_order_independent stdout ----

thread '...needs_sanitize_mixed_markers_is_order_independent' panicked at src/vision/detection/rt_detr_v2/sanitize.rs:363:9:
assertion `left == right` failed: needs_sanitize said `true` for 27 of 64 mixed-marker maps freshly built from the same key set; the verdict must be a stable `false` and must not depend on HashMap iteration order. Observed: [false, false, false, false, false, true, false, false, true, true, false, false, false, true, false, false, false, false, false, true, true, false, false, true, true, false, false, true, false, false, true, false, false, true, true, false, true, false, true, true, false, true, true, false, false, true, true, true, false, true, true, false, false, false, true, true, false, false, true, false, true, false, true, false]
  left: 27
 right: 0

test result: FAILED. 13 passed; 1 failed; 0 ignored; 0 measured; 5859 filtered out; finished in 0.37s

27 of 64 is close to the 3/7 predicted by the key mix, which is what a first-key-wins walk over that map should produce. Note that needs_sanitize_detects_layout passed in the same run: its two single-key maps cannot express the mixed case, so its green status was never evidence about this defect.

After the fix the same filter is green, and repeating it five times in a row (320 independently built maps in total) never observed a differing verdict.

Test plan

• cargo test --profile test-fast --features cuda --lib vision::detection::rt_detr_v2::sanitize (exit 0, 14 passed)
• Same command repeated 5 more times back to back (exit 0 each, 14 passed each)
• cargo test --profile test-fast --features cuda --lib vision::detection::rt_detr_v2 (exit 0, 32 passed)
• cargo fmt --all -- --check (exit 0)
• cargo check --lib --tests --features cuda (exit 0)
• cargo clippy --lib --tests --features cuda -- -D warnings -A clippy::err_expect (exit 0, no findings in the touched file)

One note on that last line. Plain cargo clippy --lib --tests --features cuda -- -D warnings exits 101 on main at ca114220, before this branch, on clippy::err_expect at src/multimodal/host_preprocessor_tests.rs:416. That file is byte-identical to origin/main here and this branch does not touch it. The repo's own lint gate is make verify-clippy, which runs --features metal,accelerate, and the Makefile notes there is deliberately no verify-clippy-cuda yet, so the CUDA lint path is currently ungated. Allowing only that one pre-existing lint is what makes the run above meaningful about this change; it is not a suppression added to the code.

The full make verify-test-cuda merge gate has not been run here and is left to the maintainer.

Closes #1276

[inureyes]

Gate run notes, since the branch was cut before two commits landed on main.

The branch base (ca114220) is behind main (bf69b83e), which has since taken #1278 (documentation and benchmark scripts) and #1280 (code plus tests). To gate the tree that will actually merge rather than the older base, the commit was rebased onto bf69b83e locally and the gate was run there. The rebase was clean, and after it the branch's diff against main is a single file, src/vision/detection/rt_detr_v2/sanitize.rs (+134 / -18), which no commit on main touches. The rebase was not pushed, so this PR still shows its original commit; a squash merge onto current main produces the same tree that was gated.

make verify-test-cuda on GB10 (CUDA sm_121, Linux aarch64), rebased onto bf69b83e: 8229 passed, 0 failed, 311 ignored, 101 suites, exit 0. No link or compile errors in the log.

No cross-run delta is quoted on purpose. The previous full gate this session ran at ca114220 (8194 passed) and main has gained tests since, so the two totals are not comparable. What is checkable is this branch's own contribution: the diff adds exactly 2 #[test] functions and removes none.

cargo fmt --all -- --check and cargo check --lib --tests --features cuda are clean on the same tree.