Repository navigation
test(ArcGate): fail CI when a feature is built, wired, and unreachable - #157
Merged
Merged
Conversation
Arc's recurring fault is not "unbuilt" — it is "built, wired, and switched off by something adjacent". Nothing fails when it happens: the symbol compiles, its unit tests pass, and the only signal is a dead_code warning in mistralrs-core, which is deliberately outside the -D warnings clippy lane. This adds the check that closes that hole. Two independent checks in mistralrs-core/tests/capability_reachability.rs, which runs in the existing `cargo test -p mistralrs-core` CI lane on all three OSes with no GPU: 1. REGISTRY — named capabilities, each asserting a production call path exists (not that the symbol compiles). Entries marked Tracked are known-dark on purpose and carry a reason; those go red if they BECOME reachable, so the list cannot rot in either direction. 2. DEAD_SYMBOL_BASELINE — a ratchet over Arc-owned source. Any definition with no production reference fails unless it is in the checked-in baseline. This catches features nobody thought to register. The ApiPromise check encodes the logit_bias class: a parameter accepted at the API boundary that must be READ by the runtime meant to honour it. Accepting and silently dropping is worse than rejecting. Baseline records five pre-existing dark symbols, three of which rustc's own dead_code analysis independently confirms (assign_row_lens, extend_draft_kv, run_target_forward) — two instruments agreeing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SpVNMpb13HkUXqSqbN1o9H
Code Metrics Report━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Language Files Lines Code Comments Blanks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ C Header 5 305 210 52 43 CSS 2 1181 1036 34 111 CUDA 72 24645 17702 4215 2728 Dockerfile 1 39 22 8 9 JavaScript 16 3546 2676 482 388 Jinja2 7 694 656 5 33 JSON 74 4600 4597 0 3 Makefile 1 6 5 0 1 Metal Shading Lan| 33 12224 9431 1142 1651 PowerShell 1 300 227 30 43 Python 144 14955 12317 810 1828 Shell 26 6312 4307 1564 441 Plain Text 4 3801 0 2479 1322 TOML 33 1488 1293 45 150 YAML 3 25 23 2 0 ───────────────────────────────────────────────────────────────────────────────── HTML 4 2687 2604 43 40 |- CSS 2 543 479 37 27 |- JavaScript 1 1233 1215 12 6 (Total) 4463 4298 92 73 ───────────────────────────────────────────────────────────────────────────────── Jupyter Notebooks 4 122 83 23 16 |- Markdown 1 60 30 22 8 |- Python 1 122 113 1 8 (Total) 304 226 46 32 ───────────────────────────────────────────────────────────────────────────────── Markdown 197 39646 0 30479 9167 |- BASH 72 1652 1200 331 121 |- C 3 17 17 0 0 |- CUDA 2 84 56 16 12 |- JSON 18 708 708 0 0 |- PowerShell 1 1 1 0 0 |- Python 23 1008 787 113 108 |- Rust 65 2048 1713 77 258 |- TOML 6 207 164 0 43 |- YAML 4 38 33 5 0 (Total) 45409 4679 31021 9709 ───────────────────────────────────────────────────────────────────────────────── Rust 668 318690 275524 15018 28148 |- Markdown 484 26172 471 22580 3121 (Total) 344862 275995 37598 31269 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Total 1295 469159 339700 79625 49834 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Jish's standing order: "its built and its switched off should never happen again." This is the structural half — the guard that makes it fail CI instead of being noticed months later.
Why nothing catches this today
mistralrs-coreis deliberately outside the-D warningsclippy lane (.github/workflows/ci.yml:131— the workspace-wide form floods with pre-existing upstream findings). So the one signal the compiler does emit for this class,dead_code, is discarded. A feature can land wired-to-nothing and the only trace is a warning nobody gates on.What changed
One new file,
mistralrs-core/tests/capability_reachability.rs. It runs in the existingcargo test -p mistralrs-corelane (ci.yml:88) on ubuntu/windows/macOS — no new workflow, no GPU, no model.every_registered_capability_has_a_production_call_pathREGISTRYentry has a reference outside tests.Trackedentries are known-dark with a reason and go red if they become reachable — the list cannot rot in either directionno_new_unreachable_symbols_in_arc_owned_codeDEAD_SYMBOL_BASELINECheck::ApiPromiseencodes thelogit_biasclass the coordinator asked for: a parameter accepted at the API boundary that must be read by the runtime meant to honour it. Accepting a parameter and silently dropping it is worse than rejecting it — the caller believes it took effect.The guard was proven able to fail, three ways
1. Live acceptance against PR #125, not a synthetic case.
prefill_chunk_is_intermediate(pipeline/mod.rs:421) is introduced byfeat/chunked-prefill-cursorwith zero callers — not even tests. Copying this file onto that branch:Exactly one symbol, correct file:line, no noise from the rest of that branch. Not fixed here — that is #125's author's call.
2. A hidden live call site.
batch_can_be_raggedhas one production caller (kv_cache/mod.rs:1520) and four test callers. Inlining the production one left the four test callers in place; both checks went red independently. This proves the production/test discrimination, not merely symbol presence. Reverted.3. The guard caught itself passing vacuously. On its first run the
logit_biasentry was green — on a tree where the wire is dead.logits_biasoccurs insampler.rsexactly twice, as the declaration (:85) and itsNonedefault (:124), and both were counted as reads. Fixed byreads_field, which excludes any occurrence immediately followed by:(declaration or struct-literal write). The entry then correctly reported:It is registered
Trackednaming PR #151, which implements it. When #151 lands this test goes red asking to be promoted toLive— the bidirectional design doing its job rather than the fix being forgotten.Two scanner bugs were also found by running it, both of which under-reported test regions and would have mislabelled test code as production: a fixed lookahead window missed
#[cfg(test)]separated from itsmodby a comment block, and whole-file test modules (#[cfg(test)] mod parallel_bake_tests;) were not detected at all.Baseline — five pre-existing dark symbols, now tracked rather than forgotten
Cross-checked against
cargo check -p mistralrs-core --message-format=jsonatcca5e5c6e; rustc's owndead_codeanalysis independently reports three of them.arc-cuda-graph/src/decode_forward.rs:115arc_launch_gemv_bf16_silu_mul_downlibarccudagraph.aon every CUDA build and launched by nothing; declared twice, called from neithermistralrs-core/src/kv_cache/xs_rolling.rs:516assign_row_lens#[cfg(test)]regionmistralrs-core/src/pipeline/mod.rs:588cuda_graph_runner_mutPipelinemethod implemented by all eight pipelines and called by nobodymistralrs-core/src/pipeline/mtp_pipeline.rs:1761extend_draft_kvmistralrs-core/src/pipeline/mtp_pipeline.rs:2527run_target_forwardAlso registered
Tracked:AutonomousDecodeConfig(arc-cuda-graph/src/autonomous.rs:70-82) carriestemperature,top_p, both penalties andgreedy— and neithertop_knormin_p. Latent rather than live only because that path is itself unreachable today.Scope — deliberately not included
The default flips are held, and the reasons are evidence, not caution:
fix/ragged-gate-coherence, which must land first. Flipping the default now would ship a silent wrong answer. D14 also bans a CPU-only correctness verdict and this path has never run on a GPU.ARC_PREFILL_MAX_SEQS/ARC_PREFILL_FLOOR_STEPSdeserve CLI flags butmistralrs-cli/src/args/mod.rsis being rewritten by feat(cli): cut the menu to 12 flags — opinionated surface, --help-all for the rest (D18) #110 and feat(ArcSched): --v4-ragged-decode — V4's ragged decode path had no surface but an env var #153 concurrently; adding flags here would conflict textually for no benefit.Known limitation, stated rather than hidden
Reachability here is "a production reference exists", not "that reference is itself reachable" — a cluster of dead functions calling only each other satisfies check 1. It also cannot see env-gated-but-default-off features, which is most of the inventory; those need the CLI-flag work above.
How verified
No GPU; the GPU box was not touched.
cargo test -p mistralrs-core --test capability_reachability→ 11 passed, 0 failed atcca5e5c6e.rustfmt --edition 2021on the single new file only (never--all, per fork policy).