What actually proves the recovery works

NVIDIA-NeMo/RL PR #3480 — the test evidence behind the recovery ledger, independently re-run for this review. Links pinned to bca9a721. Part 3 of 3 · the ledger · the atomic cut

The standing worry with checkpoint tests is mocks: a "round trip" where the buffer or the sampler is a stub proves the wiring, not the recovery. This page is the audit — which pieces of the PR's test pyramid run the real code, what each layer proves, what we re-ran locally for this review, and the one assertion still missing.

Three layers, and what is real in each

functional: two real training runs, 2 GPUs a hook parks one rollout before its TQ commit; a timeout checkpoint cuts it deterministically; run 2 must show THAT group id redispatched exactly once, committed exactly once, and absent from the next checkpoint's ledger everything real: TransferQueue backend, vLLM, trainer — registered run_test fast (runs in both CI lanes) race suite: the real controller, cut at exact instants (17 tests) drives the real _save_checkpoint, _rollout_pump, redispatch and restore methods; blocks a save mid-write, fires a real commit against it, asserts exactly one owner lands on disk; covers the cursor-skew cuts, corrupt/missing sidecar, capacity, redispatch ordering real: controller, barrier, buffer, samplers, ledger · stubbed: trainer, generation, rollout manager round-trip matrix: 4 samplers × 6 buffer states = 56 tests save to disk, then a genuine restart: brand-new data-plane client loads the files, brand-new buffer, brand-new sampler, brand-new ledger; asserts finished groups return without re-writing tensor rows, unfinished groups re-commit through the ledger, evicted groups stay gone real: TQReplayBuffer, all 4 samplers via create_sampler, barrier, ledger, disk round trip stubbed: the tensor converter (fixed tensors) · absent: trainer/generation — neither is on the save/restore path scenarios: all-complete · partly generated · two-batches-in-flight (lag 2) · evicted group · trained-out-of-order hole · stale-only (weight gap 0 vs 7)
Each layer trades scope for realism. The matrix proves state survives the disk; the race suite proves the cut is atomic in the real controller; the functional test proves a real interrupted run recovers a specific parked prompt.

Two details worth trust. The functional hook is injected through a test-only entrypoint — no environment-variable hooks in production code — and its assertions are about a specific group id, not "the run finished". And the matrix restart is honest: the restored side shares no objects with the saving side, down to a fresh data-plane client that must load everything from the files.

Re-run for this review, and the one missing assertion

All in-scope suites were re-run locally on CPU for this review (macOS, hand-built venv):

test_checkpoint_recovery_matrix + test_checkpoint_dispatch_races
  + test_rollout_recovery + test_checkpointing            147 passed, 14.45s
test_rollout_pump + test_tq_replay_buffer
  + test_sampler_interface                                 140 passed, 12.90s
                                                           0 failed, 0 skipped

The gap: the matrix proves restored groups are present — it checks the buffer's group ids and ready flags after restore — but nothing ever asks a restored sampler to select them. The harness even says so: ready/pending counts are "reported separately for diagnosis only; nothing asserts on them" (_checkpoint_scenarios.py:263). Selection is per-sampler logic — one keys on the stamped step, the others on weight windows — so presence does not imply selectability, and only in_order gets an end-to-end restore-then-train proof from the functional test.

To check whether this hides a bug, we wrote the missing test against the PR's own harness and ran it: save the all-complete scenario, restore into a fresh buffer, real sampler.select(), assert the three restored groups come back. All four samplers pass. So the product code is fine — this is a missing assertion, and the review asks the author to fold it into the matrix (the harness already builds the restored sampler; the assertion is a few lines).

Also asked for, not yet present: a lag=0 row in the matrix, and any overhead number for the wider exclusive checkpoint window (part 2). Neither blocks the feature; both keep the next regression from hiding.
So what. This is the rare checkpoint feature where the tests genuinely restart the world: real buffer, real samplers, real barrier, real files, plus a race suite that cuts the real controller mid-write and a functional run that recovers a deliberately parked prompt. With the select() assertion added, the round trip is covered end to end for all four samplers.