fix(surface): 169 tests referencing binaries that do not exist, and a unit test that shelled out to cargo clippy - #2520
Closed
noahgift wants to merge 1 commit into
Closed
fix(surface): 169 tests referencing binaries that do not exist, and a unit test that shelled out to cargo clippy#2520noahgift wants to merge 1 commit into
noahgift wants to merge 1 commit into
Conversation
…es to binaries that do not exist, and a unit test that shelled out to cargo clippy Binary-surface audit, continued. 27 crates build 29 binaries; 24 of those crates are named nowhere in ci.yml, so their ~204 integration test files never run. This is what was hiding in them. DEAD BINARY REFERENCES (169 genuine) A crate's `[lib] name` is not its binary name, so CARGO_BIN_EXE_<libname> never existed and every test using it was dead: aprender-serve -> "realizar" 32 refs lib-only, no bin at all aprender-mcp -> "apr" 5 refs lib-only, no bin at all aprender-shell -> "aprender-shell" 4 refs bin deleted 2026-04-08 aprender-orchestrate -> "batuta" 1 ref aprender-test-cli -> "probador" 1 ref (aprender-profile -> "renacer", 126 refs, is #2516 on its own branch.) Per crate: * aprender-serve: DELETED tests/integration_cli.rs (423 lines, 32 tests). No `realizar` binary exists anywhere in the workspace and none was ever deleted -- the file was imported wholesale from the standalone repo during consolidation and the binary stayed behind. Not silently passing: 32/32 hard-failed. Every behaviour it claimed is already covered in-process by tests that DO run (artifact_falsification, active_pygmy_inference, cli/tests_03, ...). 5 of the 32 were tautologies that could not fail. * aprender-shell: DELETED 7 files / 88 tests. The bin was deliberately removed on 2026-04-08 in f5db50a under contracts/apr-mono-binary-rule-v1.yaml; the tests outlived it by four months because ci.yml:317 names no aprender-shell target. 11 salvaged into src/robustness_tests.rs against the public API, deliberately under --lib because that is the only aprender-shell target CI runs. * aprender-mcp: resolves the apr binary from cargo's OWN --message-format=json compiler-artifact record -- the scripts/apr_bin.sh doctrine, ask cargo rather than guess. Immune to CARGO_TARGET_DIR redirects. Asserts exactly one distinct apr executable so an ambiguous graph fails loudly instead of being decided by luck. Builds unconditionally: the "file already exists" short-circuit IS the stale-artifact hole. Note the old code was unreachable by construction: `if candidate.is_file() { candidate } else { build_apr_binary() }` -- cargo_bin PANICS rather than returning a missing path, so the repair arm could never run. THE 202-SECOND UNIT TEST bug_hunter::tests ran hunt(Path::new(".")) -- against the REAL crate, since cargo test's cwd is the manifest dir. That fans out to `cargo clippy --all-targets` (a full nested compile), `pmat query` over the whole tree, and `git blame` per source file. hunt_ensemble does it three times. One test passed /tmp, so pmat walked the entire system temp dir. test_bh_mod_001_hunt_all_modes 202.7s test_bh_mod_001_hunt_returns_result 157.3s test_bh_mod_046_..._no_pmat 118.0s Rewritten onto a fixture with one src/lib.rs and an lcov.info carrying one deterministic trigger per mode. It deliberately has NO Cargo.toml, so the nested cargo clippy finds no manifest and exits without compiling -- the fix REMOVES the nested build rather than serialising it, so these do not need nextest's serial-build group. bug_hunter: 688 tests, 202.7s -> 0.30s Why local and CI disagreed: bug_hunter caches into <project>/.pmat/bug-hunter-cache/, so a warm dev box looked fine (34s) while a fresh CI checkout paid full price every run. TWO FAILING TESTS * oracle::local_workspace::tests::test_get_git_status_current_repo asserted on the git status of whatever directory it ran in -- passes on a clean checkout, fails in a dirty one or a detached worktree. Now builds its own repo in a per-process temp dir and asserts a known state. Mutation-verified RED. * pixel_coverage::wasm_demo::tests::h0_perf_02_fill_pass_reasonable_time was a wall-clock assertion (banned here) AND it was failing at 30.8s. Both timing bounds replaced with value oracles plus non-vacuity assertions. NEW COVERAGE aprender-ptx-debug had a hand-rolled `match args[1]` parser -- the pattern banned after the identical one in simular silently dropped --seed. Converted to clap derive; 78 tests where there were none, asserting that an unknown flag, a valueless flag, and an unparseable value are all ERRORS rather than defaults, plus Cli::command().debug_assert(). Three smoke tests for previously untested binaries (presentar, train-distill, verificar), each mutation-verified RED. One mutation did NOT turn red and was diagnosed rather than shrugged at: a redundant second branch in validate_teacher also catches the empty string, so the property was re-mutated instead. VERIFICATION lib 13,757 passed 0 failed integration 45 targets 13,929 passed 0 failed (rc=0, read directly) aprender-serve cargo check --tests rc=0 clippy --all-targets (9 crates) rc=0, 0 errors cargo fmt --all --check rc=0 Cargo.lock is deliberately NOT in this commit; main's lockfile is stale and that is #2518. Findings that are recommendations, not code, are filed as #2519: three train-* binaries report confident results without doing the work (one of them published to crates.io). Refs #2503, #2519
noahgift
added a commit
that referenced
this pull request
Aug 16, 2026
…oolchain, and ban hand-rolled argv parsing
WHAT WAS MISSING
Neither existing skill covered the shipped surface. Measured before writing this:
.claude/skills/apr-dogfood/SKILL.md (828 lines)
references 26 distinct `apr` subcommands out of 103
occurrences of mcp / MCP / /v1/ / curl / endpoint / route: 0
.claude/skills/pre-release/SKILL.md
same: 0
The 0.63.0 audit that probed 104 CLI commands, 9 MCP tools and 45 routes was
done BY HAND and was never reproducible.
scripts/dogfood_surfaces.sh covers all three interface kinds across every binary
the workspace builds, and its receipt is byte-identical across runs.
pass=209 fail=0 skip=1 (skip 0%) rc=0
--twice: DETERMINISTIC, byte-identical receipts
IT USES THE DETERMINISTIC TOOLCHAIN, IT DOES NOT REIMPLEMENT IT
pv contract validation (never yq, never a python YAML walk)
bashrs shell quality (never shellcheck)
probar endpoint testing (never a hand-rolled curl loop)
pmat code search / quality (never grep for discovery)
Each is asserted PRESENT with its version rather than skipped-if-missing: a
sweep that silently drops its verification tools reports a clean pass having
checked less, which is the vacuous-scan defect the script exists to avoid.
The first draft violated this. It parsed contracts/apr-mcp-tool-schemas-v1.yaml
with python and counted `tools:` entries by hand -- muda by CLAUDE.md's explicit
rule, AND redundant, because FALSIFY-MCP-008 already asserts byte-identity
between the codegen constants and the live tools/list response at four layers.
Reimplementing a weaker version of an existing falsifier is the opposite of
dogfooding. It is `pv validate` now, plus `pv lint contracts/` over the whole
directory. The live endpoint probe runs `probar llm test`, not curl. The script
holds itself to the rule it enforces: it bashrs-lints its own source.
ENUMERATED AT RUNTIME, NEVER FROM A LIST
binaries cargo build --message-format=json (executables cargo REPORTS)
apr subcommands apr --help
HTTP routes the ("GET","/path",handler) table in api/router.rs
MCP tools const NAME in aprender-mcp/src/tools/
A written-down list is the defect this repo keeps finding: the falsification
spec asserts "exactly 36 top-level commands" and now finds 0 because the enum
moved file; CLAUDE.md has claimed 77, 103 and 111. Grepping source is no better
-- a regex over clap Subcommand enums reports 0 subcommands for `simular`, which
IS a clap-derive CLI. Only the binary knows what the binary accepts. Every
enumeration is vacuity-guarded: too few items FAILS.
A PASS MUST EXCLUDE AN OUTCOME
`--help` exiting 0 is not a pass -- a binary that prints nothing also exits 0.
Each binary must ALSO reject an unknown flag, which catches a parser that is not
parsing. Skips are counted, never silent, and a run skipping more than
MAX_SKIP_PCT FAILS.
HAND-ROLLED PARSERS: FIXED AND BANNED
scripts/check_no_hand_rolled_parsers.sh bans the CONSTRUCT, structurally and
ratcheted. It is complementary to the behavioural probe: the probe catches
today's broken parsers, the ban stops one returning. Self-test 3/3, including
two false-positive controls (a clap CLI that also calls env::args() must NOT be
flagged).
Four were hand-rolled; this converts three to clap derive (aprender-ptx-debug is
#2520). Baseline 4 -> 1.
aprender-compute-xtask --help exited 1
aprender-qa-certify apr-qa-readme-sync
aprender-zram-generator --help printed 0 BYTES and an unknown flag was
ACCEPTED at exit 0 -- so a typo'd flag was treated
as one of its DIRECTORY arguments. It is a systemd
generator; the three positional dirs are preserved
exactly, and --help now explains the protocol.
WHAT THE FIRST RUN FOUND
aprender-train-lora PANICKED on any argument
trueno-zram PANICKED on any argument
Both declared a short option twice -- `-m` for `model` AND `method`, `-p` for
`pages` AND `pattern`. clap's check is #[cfg(debug_assertions)], so RELEASE
builds do not panic; they ship the ambiguity. Verified on a release build,
`aprender-train-lora plan --help` listed BOTH `-m, --model` and `-m, --method`.
Fixed by making the colliding argument long-only in each.
TWO BUGS IN THIS SCRIPT, FOUND AND FIXED WHILE WRITING IT
1. It parsed `apr --help` with `^[[:space:]]+[a-z]`, scraping WRAPPED
DESCRIPTION lines: `apr yet)`, `apr clip.wav`, `apr existing` were reported
as subcommands and the count read 114 against a real 105.
2. It built binary paths from `cargo metadata`'s target_directory. In a worktree
that reports /mnt/nvme-raid0/targets/aprender while cargo writes to
<worktree>/target/debug -- .cargo/config.toml holds the redirect and is
gitignored. The script was probing binaries built from a DIFFERENT TREE. It
asks cargo now. The repo's own binary-pinning doctrine, violated by the first
draft.
Route enumeration was wrong once too: globbing every "/..." string literal
reported 284 routes. Reading the route table gives 34.
VERIFICATION
dogfood --self-test 3/3 (incl. permissive-CLI caught)
hand-rolled ban --self-test 3/3 (incl. 2 false-positive controls)
dogfood --twice byte-identical receipts
full sweep pass=209 fail=0 skip=1, rc=0
pv lint contracts/ 0 errors
cargo test (3 converted crates) 226 passed, 0 failed
cargo clippy --all-targets 0 errors
cargo fmt --all --check rc=0
NOTE for sequencing: scripts/check_shell_lint_ratchet.sh (#2511, not yet on
main) baselines the repo-wide bashrs error count; these two new scripts add 13
(all documented false-positive classes on embedded python/awk -- `bash -n` is
clean on both), so it needs a re-baseline when both land.
Refs #2503
noahgift
added a commit
that referenced
this pull request
Aug 17, 2026
…N_EXE_...") broke the CI build
The falsifier wiring from the previous commit worked: two of the three targets
ran and passed in CI (3 and 9 tests). The third failed to COMPILE:
error: environment variable `CARGO_BIN_EXE_aprender-train-shell`
not defined at compile time
--> crates/aprender-train-shell/tests/falsify_no_fabricated_fetch_2519.rs:164
let exe = env!("CARGO_BIN_EXE_aprender-train-shell");
This is the SAME CLASS I spent yesterday removing -- the 126 dead
cargo_bin("renacer") references (#2516) and the `realizar` / `aprender-shell`
ones (#2520) -- reintroduced in a brand-new test of my own. Worth stating
plainly: the class is easy to reintroduce precisely because it compiles fine
wherever the binary happens to have been built already.
WHAT I COULD AND COULD NOT ESTABLISH
The package declares `[[bin]] name = "aprender-train-shell"` with no
`required-features`, so the variable should exist. I reproduced CI's exact
command locally after touching the test file to force a rebuild:
cargo test -p aprender-train-shell --test falsify_no_fabricated_fetch_2519
test result: ok. 8 passed
It PASSES here. So my first hypothesis -- that `--test <name>` skips building
the package's bins -- is wrong, and I did not identify the real difference
(cargo version, or a fresh vs warm target dir).
Rather than keep guessing, the fix removes the dependency on compile-time
resolution entirely, which is correct regardless of the cause.
FIX: ask cargo at RUNTIME which executable it produced --
`cargo build --bin ... --message-format=json-render-diagnostics`, then take the
`executable` field. Same pattern already proven for aprender-mcp in #2520, and
the same doctrine as scripts/apr_bin.sh: never construct or assume a binary
path, ask the tool that built it.
The helper FAILS LOUDLY if cargo reports no executable. A test that silently
skipped when the binary was unavailable would be the skip-class escape this repo
bans -- and would have hidden the very defect #2519 is about. It uses a substring
match on the JSON rather than adding a serde dependency to a test crate.
VERIFICATION
cargo test -p aprender-train-shell --test falsify_no_fabricated_fetch_2519
8 passed, 0 failed (all 8, including the -c CLI surface test)
cargo clippy -p aprender-train-shell --all-targets 0 errors
cargo fmt -p aprender-train-shell -- --check rc=0
No compile-time CARGO_BIN_EXE remains in the file; the only `env!` left are a
comment and `env!("CARGO")`, which cargo always sets for tests.
Refs #2519, #2516, #2520
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.
Binary-surface audit, continued. 27 crates build 29 binaries; 24 of those crates are named nowhere in
ci.yml, so their ~204 integration test files never run. This is what was in them.Dead binary references — 169 genuine
A crate's
[lib] nameis not its binary name, soCARGO_BIN_EXE_<libname>never existed and every test using it was dead:aprender-serverealizaraprender-mcpapraprender-shellaprender-shellaprender-orchestratebatutaaprender-test-cliprobador(
aprender-profile → renacer, 126 refs, is #2516 on its own branch.)aprender-serve— deletedtests/integration_cli.rs(423 lines, 32 tests). Norealizarbinary exists anywhere in the workspace and none was ever deleted; the file was imported wholesale from the standalone repo during consolidation and the binary stayed behind. Not silently passing — 32/32 hard-failed. Every behaviour it claimed is already covered in-process by tests that do run. 5 of the 32 were tautologies that could not fail.aprender-shell— deleted 7 files / 88 tests. The bin was deliberately removed on 2026-04-08 inf5db50ae0undercontracts/apr-mono-binary-rule-v1.yaml; the tests outlived it by four months becauseci.yml:317names no aprender-shell target. 11 salvaged intosrc/robustness_tests.rsagainst the public API — deliberately under--lib, the only aprender-shell target CI runs.aprender-mcp— resolves theaprbinary from cargo's own--message-format=jsonartifact record (thescripts/apr_bin.shdoctrine: ask cargo, don't guess). Asserts exactly one distinctaprexecutable, so an ambiguous graph fails loudly instead of being decided by luck.Worth noting the old code was unreachable by construction:
if candidate.is_file() { candidate } else { build_apr_binary() }—cargo_binpanics rather than returning a missing path, so the repair arm could never run.The 202-second unit test
bug_hunter::testsranhunt(Path::new("."))— against the real crate, sincecargo test's cwd is the manifest dir. That fans out tocargo clippy --all-targets(a full nested compile),pmat queryover the whole tree, andgit blameper source file.hunt_ensembledoes it three times. One test passed/tmp, sopmatwalked the entire system temp dir.Rewritten onto a fixture carrying one deterministic trigger per mode. It deliberately has no
Cargo.toml, so the nestedcargo clippyfinds no manifest and exits without compiling — the fix removes the nested build rather than serialising it, so these don't need nextest'sserial-buildgroup.bug_hunter: 688 tests, 202.7s → 0.30s.Why local and CI disagreed:
bug_huntercaches into<project>/.pmat/bug-hunter-cache/, so a warm dev box looked fine (34s) while a fresh CI checkout paid full price every run.Two failing tests
test_get_git_status_current_repoasserted on the git status of whatever directory it ran in — passes on a clean checkout, fails in a dirty one or a detached worktree. Now builds its own repo in a per-process temp dir. Mutation-verified RED.h0_perf_02_fill_pass_reasonable_timewas a wall-clock assertion (banned here) and failing at 30.8s. Both bounds replaced with value oracles plus non-vacuity assertions.New coverage
aprender-ptx-debughad a hand-rolledmatch args[1]parser — the pattern banned after the identical one in simular silently dropped--seed. Converted to clap derive; 78 tests where there were none, asserting an unknown flag, a valueless flag, and an unparseable value are all errors rather than defaults.Three smoke tests for previously untested binaries (
presentar,train-distill,verificar), each mutation-verified RED. One mutation did not turn red and was diagnosed rather than shrugged at: a redundant second branch invalidate_teacheralso catches the empty string, so the property was re-mutated instead.Verification
aprender-serve cargo check --tests--all-targets, 9 cratescargo fmt --all --checkCargo.lockis deliberately not in this commit — main's lockfile is stale, and that is #2518.Recommendations rather than code are filed as #2519: three
train-*binaries report confident results without doing the work, one of them published to crates.io.Refs #2503, #2519