fix/ticket 2519 - #2548
Closed
noahgift wants to merge 3 commits into
Closed
Conversation
noahgift
commented
Aug 20, 2026
noahgift
commented
Contributor
- fix(train-shell): --session was a second door into the Three train-* binaries report confident results without doing the work — one is published to crates.io #2519 fabrication (Three train-* binaries report confident results without doing the work — one is published to crates.io #2519)
- fix(train-shell): execute_export reported success while writing nothing
…ion (#2519) #2519's three headline claims do NOT reproduce on origin/main 5c08e77: all three train-* binaries were repaired by 594b687 / 34a0f15 / 0f1b626 / 8c23a4e and now refuse instead of fabricating. Item 5 (apr-cli has no qa dependency) is fixed too -- aprender-qa-cli is reached as `apr qa-playbook`. So this does NOT implement the issue's DELETE recommendation: the code it describes is gone. What DOES still reproduce is a door the #2519 fix left open. Its falsifier drives state exclusively through `fetch`, so it enumerated one entrance and the room has two. `LoadedModel` derives `Deserialize`, and main.rs fed `SessionState::load` straight from a user-supplied `--session` path. MEASURED on origin/main 5c08e77, AFTER the fetch fix, with a hand-written sess.json naming /nonexistent: $ aprender-train-shell --session sess.json -c distill Training started... (simulated) # exit 0 $ aprender-train-shell --session sess.json -c "distill --dry-run" Teacher: does-not-exist/totally-fake-7b (7.0B) Student: does-not-exist/totally-fake-1b (1.0B) Ready to train # exit 0 $ aprender-train-shell --session sess.json -c memory Model: 16.0 GB / Total: 20.3 GB # exit 0 Every figure there was typed into a JSON file. This is the project-memory lesson "a guard's UNIVERSE built from the wrong side": the guard was correct and the defect simply was not in the set it iterated. THE FIX -- two independent rules, so neither is load-bearing alone: 1. SessionState::validate_model_provenance, called from load(): (a) a model the session says is cached must be ON DISK, and (b) nothing in this crate can produce a LoadedModel at all -- fetch refuses and no other production path calls add_model -- so any model in a session file was typed, not measured. Ordered so (b) can be retired on its own the day a real loader lands, leaving (a) standing. 2. execute_distill's non-dry-run arm refuses. #2519 names this line ("distill returns `Training started... (simulated)`"); it is reachable from any door that puts two models in the session, so fixing only the door would leave it. --dry-run still describes the configuration: describing a plan is not claiming to have executed it. The refusal deliberately does NOT quote the old success string -- a refusal that repeats the phrase it refuses is indistinguishable from the defect to any substring check. 3. main.rs fails CLOSED on a rejected session. It used to eprintln and fall through to SessionState::new(), so `--session <garbage>` exited 0 against a silently different session -- which would also have handed a rejected session back as "success with no models". MUTATION-VERIFIED (each defect restored in place, guard must go RED): M1 drop the provenance call from load() -> 3 FAILED / 12 passed M2 restore main.rs fail-open -> 2 FAILED / 13 passed M3 restore "Training started (simulated)"-> 1 FAILED / 14 passed clean tree -> 15 passed Each mutation reds a DIFFERENT subset, so no test is standing in for another. NON-VACUITY: an_honest_session_still_loads_and_keeps_its_preferences and provenance_check_passes_on_a_state_with_no_models must stay green -- a provenance check that rejects everything is a fail mode, not a pass. Verified end-to-end too: `--session <empty session> -c "memory --batch 8 --seq 512"` still exits 0 and prints batch=8 seq=512. CONTRACT: contracts/apr-train-shell-model-provenance-v1.yaml, kind kernel (the only kind on which the validator enforces PROVABILITY-001, so a misnamed falsification block is a hard error rather than a silent drop). pv validate -> rc=0, "0 error(s), 0 warning(s) / Contract is valid." pv lint contracts/ -> rc=0, Result: PASS, 0 errors (942 warnings, of which 940 are pre-existing PV-ENF-001 tree-wide; the 2 new are PV-ENF-002 "no lean_theorem -- proof recommended". No Lean project exists in-tree, so naming a theorem that does not exist would be exactly the governance-shaped YAML this repo bans. Left as advisories.) WIRING: the six citations resolve through scripts/check_contract_test_binding.sh (ci.yml:661-662, in the blocking guard-runner-labels job) -- 371 -> 377 resolved references, 27 dangling unchanged. Mutation-verified: renaming one cited fn to MUTANT_fn_that_does_not_exist gives rc=1 "FAIL contracts/apr-train-shell-model-provenance-v1.yaml: 1 dangling test reference(s), baseline allows 0". The tests themselves already run per-PR: the target falsify_no_fabricated_fetch_2519 is named at ci.yml:346, so ci.yml needs no edit (project memory: only one PR may edit that line). NOT DONE, deliberately: - #2519 item 4 (delete aprender-qa-certify's [[bin]]). It reproduces, but deletion is the wrong remedy and the repo says so: monorepo_invariants.rs :255-259 states the order is "expose via apr, drop the bin, drop the entry here", because deleting first removes the capability rather than relocating it. apr-qa-readme-sync's capability is reachable through no apr subcommand (update_readme / generate_table / START_MARKER have zero callers outside it; `apr qa-playbook` has 15 subcommands and none syncs the README). It also does not fabricate -- it exits 1 with a real errno -- so it is not in this issue's defect class. And aprender-qa-certify is publish=false, so deleting the bin changes nothing for the 0.30.0 already on crates.io. - Items 1/2/3/5: closed by the four commits above; the issue text should be updated rather than worked. Two of its three repro commands are not valid invocations of these binaries (`bench sweep` does not exist; `shell fetch <id>` needs -c). - #2495 residuals in neighbouring crates (`convert` and `export` claim success and write nothing; `validate` prints PASS beside a failed check). Same class, already triaged on their own issue, different crates. - The 0.64.0 publish blocker (#2539) -- PR #2540 is open, green and MERGEABLE. Unrelated to this diff, but note it gates the cascade tier that ships these three crates, so the #2519 fixes cannot reach crates.io users until it lands. Refs #2519, #2495, #2477 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two defects the adversarial review found in the first pass, both now closed.
1. THE EQUATION WAS FALSE ON ITS OWN TREE. The contract's
`reported_work_was_done` equation is universally quantified over every command
reachable from `entrenar_shell::commands::execute`:
forall command C: reports_success(C) implies performed(C)
and `execute_export` (commands.rs:435, in the very file this diff edits)
returned `Ok("Exported to {path} in {format} format")` having written nothing.
There is no exporter in this crate to delegate to —
`grep -rn "fn export" crates/aprender-train-shell/src/` is empty — so the
truthful fix is to refuse, not to narrow the equation until the lie fits
inside it. It now returns ConfigValue{field:"export", ...} pointing at
`apr export`.
Its test asserted the fabrication: `assert!(result.contains("Exported"))`
passed *because* of the defect. Same shape as the `wgpu_available = true`
hole found in finetune_tests.rs today — a test that locks a defect in.
Replaced with two: one requiring failure and requiring the message not read
like a success, and a non-vacuity control proving no file is created at the
path it used to claim it had written.
2. THE README COUNT BLOCKED TWO REQUIRED GATES. This branch adds one contract,
so `find contracts/ -name '*.yaml'` moves 1778 -> 1779 while README still
said 1778, failing readme_contract (ci.yml:346) and check_readme_claims.sh
(ci.yml:645), both in gate.needs. Fixed in all THREE places — the prose
claim, the "1778 contracts across" line, and the architecture tree comment at
line 225 that my first two edits missed and the gate caught.
NOTE FOR SEQUENCING: fix/ticket-2532 also adds exactly one contract and also
claims 1779. Whichever lands second needs 1780. They cannot both be right at
once, which is why they should be queued one at a time.
Verified: cargo test -p aprender-train-shell --lib rc=0;
check_readme_claims.sh rc=0 (4 PASS, 0 FAIL); pv validate rc=0.
Refs #2519
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
noahgift
force-pushed
the
fix/ticket-2519
branch
from
August 20, 2026 19:00
a660562 to
dcc4247
Compare
…s test ci / lint Format check was the only real failure on #2548. rustfmt wraps the three-arg assert! added by the previous commit across four lines. Whitespace only -- no logic change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbMTnT8Upx6Ym18i5H11iR
This was referenced Aug 22, 2026
Merged
Contributor
Author
|
Superseded by #2613, the 0.64.0 integration batch. This PR's commits are merged into Why batched rather than landed individually: one Batching also found four defects that were invisible to every individual PR — most Closing now, deliberately: an open PR that merges first moves #2613's base and forces |
This was referenced Aug 22, 2026
Closed
Closed
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
1 participant
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.