feat(querygen): add planning-batch checkpoint artifact and read engine#239
Conversation
github-actions
Bot
added
feature
henrycgbaker
added
contracts
…n planning-summary export (#237) **Stack (6 open PRs):** #237 → #238 → #239 → #240 → #241 → #242 **Chain goal:** Add per-batch checkpoint/resume to querygen so a re-run with the same `run_id` skips already-completed work. Nests resumable state under `<run_dir>/checkpoints/`, writes a frozen Stage 1 result (planning + dedup) plus per-batch Stage 1 and Stage 2 artifacts, and orchestrates resume behind a fail-fast config-drift gate (`--force` overrides) so checkpoints are never silently reused under changed settings. **This PR (1/6):** Extracts the tmpfile + atomic-rename idiom into a shared `core/atomic_io.py` helper and adopts it for querygen's planning-summary write — the atomic-write primitive every later checkpoint artifact (#239–242) writes through. --- ## What - `core/atomic_io.py` — `atomic_write_text` context manager: PID+uuid-uniquified tempfile, `Path.replace` for the atomic rename, no fsync (matches house style). - Querygen `export.py` adopts it for `export_planning_summary` (indented JSON for human-scannable persisted artifacts). - **Note:** annotation `record_builder`/`export_runner` should adopt `atomic_write_text` in a follow-up — kept out here to keep this PR's dependency surface to querygen only. ## Why Extracts the tmpfile + atomic-rename idiom (previously duplicated inline in two annotation sites — `record_builder.write_partition_manifest` and `export_runner.write_export_csv`) into one shared core helper, then adopts it for querygen's planning-summary write. Reaches the rule-of-3 with the upcoming querygen artifact writes in #239–242. ## Test plan - [x] `tests/unit/core/test_atomic_io.py` — round-trip + temp-cleanup-on-exception.
saschagobel
left a comment
saschagobel
left a comment
There was a problem hiding this comment.
The PlanningBatchArtifact boundary itself makes sense to me for Stage 1 crash recovery, and the assemble/export/read split is broadly in line with the existing artifact pattern.
My concern is with the generic read/drift engine introduced here. DriftedField and read_checkpoint_artifact() feel heavier than needed for a first crash-resumability pass, especially because the broader stack currently moves toward config-drift handling and partial reuse under changed settings. For v1, I think we can keep this much simpler: checkpoint absent → run; checkpoint present and compatible with the same run_id / same effective settings → reuse; checkpoint incompatible → fail clearly. That likely only needs a small internal checkpoint read helper, not a reusable drift framework returning (artifact, list[DriftedField]).
So I’d suggest simplifying here and keeping the planning-batch reader focused on strict same-run compatibility.
2 participants
Stack (6 open PRs): #236 → #237 → #238 → #239 → #240 → #241 → #242
Chain goal: Add per-batch checkpoint/resume to querygen so a re-run with the same
run_idskips already-completed work. Nests resumable state under<run_dir>/checkpoints/, writes a frozen Stage 1 result (planning + dedup) plus per-batch Stage 1 and Stage 2 artifacts, and orchestrates resume behind a fail-fast config-drift gate (--forceoverrides) so checkpoints are never silently reused under changed settings.This PR (3/6): First of three vertical artifact slices. Adds per-batch checkpointing for Stage 1 planning, plus the generic read engine all three artifact readers (this PR's, #240's, #241's) delegate to.
What
PlanningBatchArtifactschema (querygen_output.py) withextra="forbid"+1:1 candidate_ids/blueprintsmodel_validator.assemble_planning_batch_artifactstampspragmata_version+created_atinternally.export_planning_batch_artifact— atomic JSON write via the helper from refactor(core): add shared atomic_write_text helper, adopt in querygen planning-summary export #237.read_checkpoint_artifact(engine): returns(artifact, list[DriftedField])—(artifact, [])reusable,(None, [])absent,(None, drifted)config-drifted; raises on damaged file (caller self-heals).DriftedField—@dataclass(frozen=True, slots=True)withfield/stored/expected, in line with core's frozen-dataclass container convention.read_planning_batch_artifact— thin typed wrapper over the engine (matches the existingread_planning_summary_artifactper-artifact typed-reader house pattern).Why
The read engine is the reusable core of the whole stack: every checkpoint read (#240's frozen result, #241's realization batches, #242's orchestration) goes through
read_checkpoint_artifact, so the drift/absent/damaged trichotomy is defined once here and the later PRs only add typed wrappers.Test plan
tests/unit/core/querygen/test_planning_batches.py— schema, round-trip, header-mismatch (drift) per field, version-mismatch, malformed/extra-field, length-validator.