Repository navigation
Wire the checkpoint invariant predicates - #325
Merged
Merged
Conversation
The checkpoint runner never read a case-level invariants block, and its expected and resume handlers skipped any name they did not know. So 17 predicate names across 025, 026, 029, 031, 069 and 070 asserted nothing, among them 031's caller correlation_id claims and the resume correlation checks in 025 and 070. All three positions now reject an unknown name, and each name the runner meets is checked against what the runs recorded: both runs' saves, the saved record, the resume's events and body trace, and the route each save took. Boolean predicates accept only true. 050's executes_step names and 054's batching scope, skipped on purpose until now, are checked as well. 057 joins the runner. It passes a caller invocation_id on the first run and another on the resume, which the engine must ignore. Nothing tested that before. Two handlers could pass vacuously: the instance attempt checks when no recorder saw the instance, and the duplicate checks when the field was not a list. Both now fail instead. The branches that read correlation_id_assertions, invocation_id_differs and correlation_id_preserved are gone, since no fixture declares them. POSITION_BLIND_READS gains resume.caller_invocation_id, which the runner now reads at both case level and under resume. The module docstring now lists every fixture the runner drives. Four docstrings move their spec citations into comments, and the comment on the fixture set maps each fixture to its proposal without the history.
There was a problem hiding this comment.
🔵 Needs a closer look
Predicate interpretations need specification confirmation, and expected-error resumes still bypass invariant validation.
1 open issue
What changed in this PR
Strengthens checkpoint conformance coverage by checking previously ignored predicates against recorded execution evidence.
Changes:
- Adds invariant validation and checks for checkpoint and resume behavior.
- Activates fixture 057 and records checkpoint save routes.
- Registers the nested caller invocation ID directive.
| File | Description |
|---|---|
| tests/conformance/test_checkpoint.py | Implements predicate checks, execution evidence capture, and fixture coverage. |
| tests/conformance/harness/vocabulary.py | Registers resume.caller_invocation_id. |
Review effort: Balanced
Resume invariant names were checked only at the end of the case runner, after the resume.expected_error branch and the branch for a resume with no from_first_run had already returned. A case on either path could declare an unknown resume predicate, or a valid one, and nothing checked it. Both resume blocks are now merged and their names checked up front, next to the case-level guard. Declaring case-level or resume invariants on a path that never completes a resume fails. Two other paths dropped blocks the same way and now reject them: a checkpoint_not_found case rejects expected.invariants, and a seeded-record case rejects case-level and expected invariants. No current fixture declares a block on a path that skips it.
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.

The checkpoint runner never read a case-level
invariants:block, and itsexpectedand resume handlers skipped any predicate name they did not handle. So 17 names across 025, 026, 029, 031, 069 and 070 asserted nothing, among them 031's callercorrelation_idclaims and the resume correlation checks in 025 and 070. The OTel runner's 031 test still covered 031's claim on the span side, but nothing checked it on the saved records.true, so a fixture that sets one tofalsefails instead of switching it off.invocation_idon the first run and another on the resume, which the engine must ignore. No test covered that before.executes_stepnames and 054's batching scope, which the runner skipped on purpose, are now checked.correlation_id_assertions,invocation_id_differsandcorrelation_id_preservedare removed. No fixture declares them.POSITION_BLIND_READSgainsresume.caller_invocation_id, which the runner reads both at case level and underresume.Some predicates leave their meaning open, so these readings are pending a spec ruling. Every fixture that runs today passes under them:
last_saved_at_monotonic_across_savesis non-decreasing in save order, since two saves can land in the same clock tick.completed_positions_step_monotonicmeans steps strictly increase within each save.inner_first_node_not_re_runlooks at the first completed position inside the subgraph, andsubgraph_re_entry_uses_parent_statesmeans the re-enteredstep_tworeceives the saved record'sparent_states.save(), and at least one save goes through the batched path.Testing:
last_saved_at, out-of-order steps and a non-UUID invocation id each fail 026.false, case-level invariants with no resume, and per-instance predicates aimed at the wrong or an unseen instance.parent_statescheck cannot tell an engine that restores the state class's defaults from one that restores the saved state, because 029 saves its outer state at the defaults.Review follow-ups:
from_first_run, as didexpected.invariantson acheckpoint_not_foundcase and case-level orexpectedinvariants on a seeded-record case. Seven edited-fixture probes that passed before this change now fail with the intended message.