feat(querygen): add checkpoint run-dir paths and per-stage fingerprints#238
Open
henrycgbaker wants to merge 3 commits into
Open
feat(querygen): add checkpoint run-dir paths and per-stage fingerprints#238henrycgbaker wants to merge 3 commits into
henrycgbaker wants to merge 3 commits into
Conversation
…n export Extracts the tmpfile+rename atomic-write idiom into core/atomic_io.py and routes querygen's planning-summary export through it (indented JSON). Annotation (record_builder/export_runner) should adopt atomic_write_text later; left out here to keep this PR's dependency surface to querygen only.
github-actions
Bot
added
feature
henrycgbaker
added
paths
henrycgbaker
force-pushed
the
feat/querygen-resumable/02-atomic-write-helper
branch
from
7e1155f to
e973f6a
Compare
This was referenced Jun 4, 2026
saschagobel
pushed a commit
that referenced
this pull request
Jun 4, 2026
…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.
Base automatically changed from
feat/querygen-resumable/02-atomic-write-helper
to
main
June 4, 2026 10:28
saschagobel
reviewed
Jun 5, 2026
saschagobel
left a comment
saschagobel
left a comment
Collaborator
There was a problem hiding this comment.
I think the checkpoint path additions here are directionally fine and seem like the right filesystem shape for crash recovery.
My concern is with the per-stage fingerprint helpers, especially fingerprint_planning_llm_settings() and fingerprint_realization_llm_settings(). Those push the design toward partial reuse under changed settings, e.g. reusing Stage 1 when only the realization model changes. For the first version, I think we should scope this to resumability only: same run_id, same effective settings, recover after crash/abort. If settings that affect the run differ, I’d prefer one strict compatibility check that fails clearly and asks the user to start a new run or delete/recreate the existing one.
So I’d suggest removing the per-stage fingerprints in this stack and replacing them with a single same-run compatibility checkpoint identity.
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
2 participants
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.
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 (2/6): Foundation. Adds the checkpoint path bundle, the per-stage fingerprint helpers used to detect config drift, and the embedding-model constant — all dormant at merge time (consumed by the later artifacts and orchestration).
What
querygen_paths.py: nests resumable state under<run_dir>/checkpoints/(checkpoints_dir,planning_batches_dir,selected_blueprints_json,realization_batches_dir). Separates the resumable working state from the deliverable CSV.planning_summary.py:_sha256_jsonshared helper;fingerprint_planning_llm_settings+fingerprint_realization_llm_settings(per-stage). A realization-model change re-runs only Stage 2; the frozen Stage 1 result and planning checkpoints stay valid.deduplication.py:EMBEDDING_MODEL_CHECKPOINTconstant exposed for the dedup artifact's provenance field.Why
Foundation for the checkpoint/resume work that lands in #239–242. The path bundle, fingerprint helpers, and embedding-model constant land first so the later artifact and orchestration PRs are pure consumers — nothing here is wired into a live code path at merge.
Test plan
tests/unit/core/paths/test_querygen_paths.py— checkpoints/ nesting expectations.