evidence: re-seal the Apple route ledger; give promotion_rules a consumer - #673
Merged
Merged
Conversation
…umer The red lane first. `test_strict_retune_ledger_admits_on_its_exact_live_apple_host` had been failing on this Mac and I reported it as "pre-existing" more than once. It was right to fail: the ledger was rejected on four independent axes -- os_version (26.5.2 -> 26.6.2), compiler_fingerprint, runtime_fingerprint (this session edited apple_gpu_runtime.mm), and stale_evidence (expired 2026-08-20, twelve days before). The gate was working; the evidence was dead. Re-measured, not re-stamped: benchmark_legacy_retune.py --profile extended, 2 runs x 24 rows, sealed through the existing sealer. Zero rejections against the live context, 16 decisions and 8 ineligible, identical key sets to July -- nothing lost, nothing invented. Near-miss: the first re-run used --profile core, which admitted cleanly with 8 decisions instead of 16. `rejected == ()` would have read as success while halving the Apple route evidence. Diffing decision keys against the committed ledger caught it; the admission result did not. One real route change. retune_mla_decode end-to-end flips explicit -> absorbed at both shapes, and the cause is not the obvious one: `absorbed` was never slower. In July it was 54-57% faster with 100% paired wins in both runs, held back solely because its cross-run speedup spread (5.97%) missed the ledger's own 5% cap by ~1 point. The re-run measures 37-39% faster, again 100% paired wins, spread 2.3%. The flip is "the measurement became consistent enough to conclude", not "the kernel got faster". Diagnostic, not a route claim: absolute times moved the wrong way (absorbed 518us -> 836us at 128-token, 482us -> 557us at 64; explicit held at 1.15x / 0.96x). Larger shape is outside the harness's own drift, smaller is not. Cause is confounded and is NOT attributed. The structural finding: `promotion_rules` was a declaration with no consumer. The aggregator computes and applies the thresholds, the sealer copies them into the ledger for audit, and across twelve committed ledgers nothing ever read them back. load_strict_route_ledger checks provenance exhaustively -- schema, scope, exact context, freshness, source digests, native provenance, correctness, domain, device, duplicates -- and the promotion criteria not at all, so status: "promote_candidate" was self-certifying. A row naming a route that lost every paired trial would have been served. Decision #29, in the evidence layer rather than the IR. promotion_rule_violations() re-derives each promotion from the retained evidence, and the LOADER now rejects a decision its own ledger's rules refuse -- a production gate, not an audit script. Across all 12 committed strict ledgers: 59 promotions checked, 0 violations, so it confirms the aggregator rather than accusing it. Mutation-verified against seven forged rows; a missing rules block fails CLOSED, because without a threshold the honest verdict is "unverifiable", not "fine". Four backends assessed under ROUTE-LEDGER-RULES-UNCONSUMED-2026-09-01: Apple parity validated (measured here, the only host that can); NVIDIA follow-up required (noise_policy is asserted equal to a constant and never compared to any row's margin); ROCm follow-up required and furthest from checkable (its promotion_gate is prose); x86 not applicable (no route ledger -- its baselines are comparisons, not promotions). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The promotion-rules gate turned four tests red, and they were right to go red: both files hand-build ledgers with `status: "promote_candidate"`, no `route_evidence`, and no `promotion_rules` block at all. The old loader accepted them because it validated provenance exhaustively and the promotion criteria not at all -- so the suite's own fixtures were exactly the forgery the new check exists to catch. Fixed the fixtures, not the checker. Relaxing it to skip validation when evidence is absent would reinstate the fail-open hole: unverifiable is not the same as fine. No sealed ledger can lack route_evidence -- the aggregator sets it unconditionally and the sealer copies it forward -- which is why all 12 committed ledgers still load unchanged. Two things beyond making the red go away: - A negative test the fixtures previously could not express: six forged violations (lost every trial / speedup under minimum / spread over cap / no numerical proof / no resource evidence / no paired trials), plus evidence-absent, plus rules-block-absent, plus a `retain_incumbent` control proving a non-promotion is not held to promotion thresholds. Without it, fixing a fixture is just moving the goalposts. - The shared fixture lives in tests/_support/apple.py rather than being imported test-module-from-test-module. Two files build these ledgers and a third checks the committed ones; three copies of a threshold set is how they drift apart. Apple sweep: 3452 passed, 1 skipped, 0 failed (4:03). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eal gap I wrote that commit_mpsgraph_and_wait_with_timeout's namesake guarantee "doesn't hold" -- that waitUntilSignaledValue:timeoutMS: was ignoring its 30s deadline during the 70-minute sweep hang. That is false, and this records the check rather than the guess. A standalone probe built against the on-machine SDK (Decision #27) measures the API honouring its deadline to the millisecond: 251ms for 250, 1001 for 1000, 3000.2 for 3000, and -- the exact shape of the failure -- a committed buffer awaiting an unreachable value returns NO after 1001ms. Sampling the probe mid-wait yields the identical frame at the identical address as the hung process (IOSurfaceSharedEvent waitUntilSignaledValue:timeoutMS: + 72 [0x1988de184]), so it provably exercises the same path rather than a lookalike. What remains unestablished is which of two things took 70 minutes: one uninterruptible wait (a driver wedge defeating the kernel-side deadline) or ~140 sequential 30s timeouts. `sample` aggregates by stack so it cannot separate them, and I killed the process before forward progress could be checked. No GPU fault in the log, no hang report -- so the wedge hypothesis has no positive evidence either. Both stay open; it has not reproduced. What IS established is a defect under either hypothesis: there is no circuit breaker. Seven call sites (6x30s, 1x60s); on timeout the caller correctly falls back to the host path, but nothing records that the device is unusable, so every later dispatch pays the full timeout again. And commit_mpsgraph_and_wait_with_timeout clears the dispatch telemetry ON ENTRY, erasing the previous timeout's evidence before the next attempt. g_last_gpu_error_kind is a thread_local last-error for reporting, not accumulating state, and nothing consults it. A device that wedges early turns a 4-minute suite into an unbounded one and leaves no trace of why. Not fixing it here, and the reason is a hard ordering constraint worth recording: AppleRouteContext.runtime_fingerprint is sha256(apple_gpu_runtime.mm), so editing that file invalidates the strict route ledger this branch just re-sealed. The fix has to land together with a benchmark re-run and re-seal, not before or after. The probe is kept at tools/apple_probes/ so the next person can re-run it instead of rebuilding the argument. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17ec7e6168
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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.
Started as the one red lane on this Mac and turned into three findings.
1. The red lane was right to be red
test_strict_retune_ledger_admits_on_its_exact_live_apple_hosthad been failinghere, and I reported it as "pre-existing" more than once without looking. The
ledger was rejected on four independent axes:
os_version(26.5.2 →26.6.2),
compiler_fingerprint,runtime_fingerprint(this session editedapple_gpu_runtime.mm), andstale_evidence— expired 2026-08-20, twelve daysprior. The gate was working; the evidence was dead.
Re-measured, not re-stamped:
benchmark_legacy_retune.py --profile extended, 2 runs × 24 rows, through the existing sealer. Zero rejectionsagainst the live context; 16 decisions and 8 ineligible, identical key sets to
July — nothing lost, nothing invented.
2. One real route change, and not for the obvious reason
retune_mla_decodeend-to-end flipsexplicit→absorbedat both shapes.absorbedwas never the slower route: in July it was 54–57% faster and won100% of paired trials in both runs, held back solely because its cross-run
speedup spread (5.97%) missed the ledger's own 5% cap by ~1 point. The re-run:
37–39% faster, again 100% paired wins, spread 2.3%. The flip is "the
measurement became consistent enough to conclude", not "the kernel got
faster".
Diagnostic and deliberately not a route claim
(
absolute_time_drift_is_diagnostic_only): absolute times moved the other way —absorbed518µs → 836µs at 128-token and 482µs → 557µs at 64, whileexplicitheld (1.15× / 0.96×). The larger shape's regression is outside this harness's
own run drift; the smaller is not. Cause is confounded across OS, runtime edits
and compiler fingerprint, and is not attributed.
3. The structural finding:
promotion_ruleshad no consumerThe aggregator computes and applies the thresholds; the sealer copies them into
the ledger for audit; and across twelve committed ledgers nothing ever read
them back.
load_strict_route_ledgerchecks provenance exhaustively — schema,scope, exact context, freshness, source digests, native provenance, correctness,
timing domain, device, duplicates — and the promotion criteria not at all, so
status: "promote_candidate"was self-certifying. A row naming a route thatlost every paired trial would have been admitted and served. Decision #29, in
the evidence layer rather than the IR.
The loader now rejects a decision its own ledger's rules refuse — a
production gate, not an audit script. Across all 12 committed strict ledgers:
59 promotions checked, 0 violations, so it confirms the aggregator rather
than accusing it. A missing rules block fails closed: without a threshold
there is nothing to hold a promotion to, and the honest verdict is
"unverifiable", not "fine".
The suite's own fixtures were forging promotions
Turning the gate on made four tests red, and they were right to go red: both
fixture files build ledgers with
promote_candidate, noroute_evidenceandno rules block at all — exactly the forgery the check exists to catch. Fixed
the fixtures, not the checker; relaxing it to skip validation when evidence is
absent would reinstate the fail-open hole. No sealed ledger can lack
route_evidence(the aggregator sets it unconditionally), which is why all 12committed ledgers still load unchanged.
Beyond making the red go away: a negative test the fixtures previously could
not express — six forged violations plus evidence-absent, rules-absent, and a
retain_incumbentcontrol — and the shared fixture moved totests/_support/apple.py, since two files build these ledgers and a thirdchecks the committed ones.
Also investigated, deliberately not fixed here
APPLE-DISPATCH-WEDGE-1(Apple todo). A sweep hung 70 minutes insidecommit_mpsgraph_and_wait_with_timeout. I first wrote that its 30s timeoutwas being ignored; that is false. A probe built against the on-machine SDK
(Decision #27) shows the API honouring its deadline to the millisecond —
including the exact failure shape, a committed buffer awaiting an unreachable
value, returning
NOat 1001ms — and sampling the probe mid-wait yields theidentical frame at the identical address as the hung process, so it provably
exercises the same path.
What remains unestablished is whether 70 minutes was one uninterruptible wait or
~140 sequential 30s timeouts;
samplecannot separate them and the processwas killed first. What is established under either hypothesis: no circuit
breaker. Seven call sites (6×30s, 1×60s); on timeout the caller falls back to
the host path correctly, but nothing records the device as unusable, and the
function clears the dispatch telemetry on entry, erasing the prior timeout's
evidence. A device that wedges early turns a 4-minute suite into an unbounded
one and leaves no trace of why.
Not fixed here because of a hard ordering constraint:
runtime_fingerprintissha256(apple_gpu_runtime.mm), so editing that file invalidates the ledger thisPR seals. The fix must land with a re-run and re-seal. Probe kept at
tools/apple_probes/.Validation
Full unit sweep on the Mac: 16988 passed, 3259 skipped, 0 failed (19:30) —
the first fully clean full sweep of this session; the previously red lane is
green because it was re-measured. Apple subset: 3452 passed, 1 skipped, 0
failed (4:03). mypy 483 files clean; ruff clean on every touched file (the
377 repo-wide errors are pre-existing and identical on
main); generated docsin sync.
Measured on the Mac (M1 Max, macOS 26.6.2) — the only host that can produce
this evidence.
Cross-backend (AGENTS.md:81-85) —
ROUTE-LEDGER-RULES-UNCONSUMED-2026-09-01noise_policy(0.03/0.04) is asserted equal to a constant and never compared to any row's margin, so a promotion inside the noise band passes every check. Its ratchet is otherwise well built (cross-checks the count against the rows, guards the vacuous-pass case, requires timing-domain consensus)."correct oracle plus shape-specific repeated-median gate". Closing it means first deciding that phrase's numeric form.🤖 Generated with Claude Code