Skip to content

feat(querygen): add realization-batch checkpoint artifact#241

Open
henrycgbaker wants to merge 1 commit into
feat/querygen-resumable/05-selected-blueprints-artifactfrom
feat/querygen-resumable/06-realization-batch-artifact
Open

feat(querygen): add realization-batch checkpoint artifact#241
henrycgbaker wants to merge 1 commit into
feat/querygen-resumable/05-selected-blueprints-artifactfrom
feat/querygen-resumable/06-realization-batch-artifact

Conversation

@henrycgbaker

@henrycgbaker henrycgbaker commented May 29, 2026

Copy link
Copy Markdown
Collaborator

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_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 (5/6): Third and final vertical artifact slice. Adds per-batch checkpointing for Stage 2 realization. Stage 2 batches are independent (no chained state), so each batch resumes on its own merits — the candidate_ids header field binds each checkpoint to its exact frozen-result chunk.


What

Why

Stage 2 has no chained state between batches, so each realization batch is checkpointed independently — a rerun replays only the batches that didn't complete. Binding the checkpoint to its candidate_ids chunk means a re-chunked frozen result can't silently reuse a stale realization.

Test plan

  • tests/unit/core/querygen/test_realization_batches.py — round-trip, header-mismatch per field, version-mismatch, malformed JSON, length-validator.

@github-actions github-actions Bot added feature Adds or expands user-facing functionality size: M 200-499 LOC labels May 29, 2026
@henrycgbaker henrycgbaker added contracts Changes schemas, type contracts, or validation logic for inputs, outputs, and persisted artifacts querygen Changes affecting the synthetic query generation tool labels May 29, 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.
@saschagobel saschagobel self-requested a review June 5, 2026 13:28
@saschagobel saschagobel marked this pull request as ready for review June 5, 2026 13:28

@saschagobel saschagobel left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The realization-batch artifact boundary makes sense to me for crash recovery. Since Stage 2 batches do not carry chained state, checkpointing them independently is a good fit, and binding each artifact to its candidate_ids chunk is the right safeguard against accidentally applying a checkpoint to the wrong blueprint slice.

My concern is mainly with how this fits into the wider stack. I would avoid carrying the per-stage reuse model into this artifact contract. For v1, realization checkpoints should be validated against the same strict same-run/same-effective-settings compatibility model as the rest of the checkpoints. If settings that affect the run differ, I’d rather fail clearly and ask the user to start a new run or delete/recreate the existing one, rather than enabling partial reuse/recompute semantics through stage-specific drift handling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contracts Changes schemas, type contracts, or validation logic for inputs, outputs, and persisted artifacts feature Adds or expands user-facing functionality querygen Changes affecting the synthetic query generation tool size: M 200-499 LOC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants