Repository navigation
Conversation
…core (Refs #5803) Refs #5804 PMAT-Ticket: PMAT-5803 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> PMAT-Batch: BATCH-aprender-n-receipt (cherry picked from commit dad2551) (cherry picked from commit 9ed0876) (cherry picked from commit 2f3c05a) (cherry picked from commit 63a2d8c) (cherry picked from commit 1d4dc37) (cherry picked from commit 24c119c) (cherry picked from commit 1fa5b48) (cherry picked from commit 615369e) (cherry picked from commit daa9be8) (cherry picked from commit 672eba5)
noahgift
left a comment
There was a problem hiding this comment.
Seat 1 (aprender-w5018, non-author, claude-sonnet-5-5): APPROVE @5a3518e04371d474f0197ea9b231893a1d997dad. One commit (PMAT-5803, one PMAT-Ticket, one Co-Authored-By, zero merges). Read the Cargo.toml, Cargo.lock, guards-nightly, manifest and explicit-test-command changes in full. At this head: check_tuner_test_move.sh exits 0 (cargo lists the 11 moved tests once each, none ignored, compute carries zero aprender-core edges) and its --self-test reads 23 passed, 0 failed; aprender-core --test tuner_training_data_quality 7 passed; aprender-compute --test tuner_falsification 109 passed. The new guard step runs only in guards-nightly as a LAB step with its one ticket label (C324 respected, nothing added to a PR path); the ml-tuner feature stays as an empty feature so removal is semver-safe. patch-id 1fed13499e1d (single commit, equal to the PR diff vs merge-base). Observation: the same commit is the 7dc5641 layer inside #6100, so the two PRs overlap; whichever lands second needs a rebase onto the first.
|
§13.11 rung 1 — quorum shadow verdict Shadow mode: this records a verdict and merges nothing. A refusal |
noahgift
left a comment
There was a problem hiding this comment.
Seat 2 (aprender-w5032, non-author) @ 5a3518e: not agreed yet, REQUEST_CHANGES.
The test move is sound: all 11 tests are kept (7 moved, 4 stay), fragments 295/298 run both targets, the Cargo.lock change only drops the dev-dep, and the nightly step is schedule-only. Fix before agreement:
- crates/aprender-compute/book/src/ecosystem/aprender.md:15 says the ml-tuner feature was removed, but crates/aprender-compute/Cargo.toml:139 still declares ml-tuner = []. Say it is a deprecated no-op, as ml-tuner.md does.
- scripts/check_tuner_test_move.sh:130 labels case L6 'CI step off / mod removed', but the script never reads ci/explicit-test-commands.d. Deleting fragment 295 leaves the moved tests unrun while the guard stays green. Check that 295 and 298 exist with the right commands, or fix the label.
- check_tuner_test_move.sh:14 says the moved bodies equal the merge-base bodies '(proved in the PR body)', but the PR body has no such proof. Quote it or drop the claim.
- Minor: the static checks at lines 91-92 treat grep rc>=2 as no match and the script has only set -u. Use the g() rc>=2-fails-closed pattern.
I read the diff only; workspace-test and ci / gate were pending when I looked.
|
Superseded by #6148 (linear rebuild fixing seat-2 findings; NOAH 10:32Z rule). |
Re-cut of the tuner-move commit from #6100 (EMF-001: one ticket per PR). Single commit, cherry-picked off main with no conflicts; check_tuner_test_move.sh --self-test 23/23 green locally.
Next: PMAT-6125 (F042-F044 contract fix + guard pin refresh) lands as its own PR on top once this merges.
🤖 Generated with Claude Code