[codegen] Make regeneration atomic and make --help/check read-only

[bokelley]

Summary

scripts/generate_types.py mutates the committed generated tree before it knows generation will succeed, and it has no real CLI parser. Even python scripts/generate_types.py --help starts a full regeneration.

Reproduction

On the current reporting SDK branch:

uv run python scripts/generate_types.py --help

Observed behavior:

• --help is ignored and generation starts.
• src/adcp/types/generated_poc/ is deleted before schema flattening/code generation/consolidation completes.
• An interrupted or failed run can leave hundreds of tracked generated files partially rewritten or missing. In the reporting integration run, the checkout was left with 920 modified files and a large temporary held tree that required manual recovery.

The destructive step is currently near the start of main():

if OUTPUT_DIR.exists():
    shutil.rmtree(OUTPUT_DIR)
OUTPUT_DIR.mkdir(parents=True, exist_ok=True)

Impact

This makes schema release work fragile, makes --help unsafe, and can destroy unrelated in-progress generated changes. A failed dependency, schema reference, collision check, or Ctrl-C leaves the repository in a misleading half-generated state.

Expected

• Add argparse; --help must be read-only and exit successfully.
• Generate into a unique temporary directory outside the committed output tree.
• Run post-processing, consolidation, import smoke tests, and collision checks against that temporary tree.
• Atomically replace generated_poc/ only after the entire pipeline succeeds.
• Clean temporary directories on failure, exception, and interruption.
• Add --check (or equivalent) that generates and reports drift without mutating the checkout.
• Add a regression test that forces a late generator failure and proves the original output tree is byte-for-byte unchanged.

Discovered while validating adcontextprotocol/adcp#6953 against #1095.

[bokelley]

Triage

Classification: Bug
Bucket(s): codegen/tooling (no exact repo label match — noted in run summary)
Status: ready-to-implement
Milestone: omitted — no target version named

What the experts said:

• code-reviewer: Bug confirmed. shutil.rmtree(OUTPUT_DIR) at line 701 fires unconditionally before generation succeeds; the finally block does not restore it. Two architectural blockers for the atomic-swap fix: (1) consolidate_exports.py hardcodes the output path and must accept --input-dir before the staging-dir approach can work end-to-end; (2) _ergonomic.py is stubbed outside the atomic boundary and must be moved inside it. Also: restore_unchanged_files() reads git diff against the committed path and will silently no-op on a staging dir; --check must suppress the SCHEMA_DELTAS.md write. Temp dir must use dir=REPO_ROOT to stay on the same filesystem as OUTPUT_DIR (cross-device rename raises OSError). Fix direction is sound and non-breaking.
• dx-expert: All three root issues confirmed (no argparse, no restore path, no --check). --help destroying 920 tracked files is a footgun for any contributor who hasn't memorized the script. The atomic-swap pattern is not overengineered — it is the minimum correctness bar used by pip, Poetry, and every packaging tool that writes N files atomically. --check is a necessary CI primitive; without it the "validate generated code" gate is theoretical or itself destructive. Regression test should use monkeypatch to replace _run_datamodel_codegen (real codegen is too slow and non-hermetic); fixture must snapshot and unconditionally restore OUTPUT_DIR in teardown.

My take: This is a confirmed, high-impact bug in the codegen pipeline actively used by PR #1095. The fix is straightforwardly non-breaking (internal tooling only, no public API surface), but the implementation has enough non-trivial cross-script coordination that a careful brief is more useful than an unsupervised first draft.

Implementation scope:

• scripts/generate_types.py — add argparse at the top of main(); --help must parse and exit before any I/O. Generate into tempfile.mkdtemp(dir=REPO_ROOT) as the staging dir. Run all post-processing steps (generate_types, generate_root_discovery_types, fix_forward_references, apply_post_generation_fixes, prune_unused_bundled_modules, consolidation, restore_unchanged_files, ergonomic generation) against the staging dir. Atomic swap via shutil.move(staging_dir, OUTPUT_DIR) only after all steps succeed. finally block must clean the staging dir. Add --check: generate into staging, diff file-by-file against OUTPUT_DIR, report drift, exit non-zero, never mutate working tree (skip SCHEMA_DELTAS.md write).
• scripts/consolidate_exports.py — add --input-dir / --output-file flags; update the caller in generate_types.py to pass the staging path. This is a blocker for end-to-end atomicity.
• _ergonomic.py stub — defer the stub write until after the atomic swap, or buffer ergonomic output inside the staging tree and write it alongside the swap.
• restore_unchanged_files() — accept a dir parameter for arbitrary paths (currently references the committed OUTPUT_DIR directly via git diff).
• Regression test — monkeypatch _run_datamodel_codegen to write fixture files into the staging dir; inject a failure in consolidate_exports or ergonomic generation; assert OUTPUT_DIR is byte-for-byte unchanged. Fixture teardown must unconditionally restore OUTPUT_DIR.

Required checks on the PR: pytest on the modified scripts, mypy src/ if types are touched, ruff check . and black --check ..

---

Triaged by Claude Code. Session: https://claude.ai/code/${CLAUDE_CODE_REMOTE_SESSION_ID}

---

Generated by Claude Code

[bokelley]

Triage

Classification: Bug
Bucket(s): codegen (no exact label exists in this repo — flagging label gap)
Status: ready-to-implement

What the experts said:

• code-reviewer: Both bugs confirmed. The destructive shutil.rmtree(OUTPUT_DIR) at lines 699–702 runs before any post-step can fail, and KeyboardInterrupt is a BaseException that bypasses the except Exception handler entirely — leaving generated_poc/ empty. Four blockers for the atomic fix: (1) the three post-processing subprocess calls (apply_post_generation_fixes, consolidate_exports, generate_ergonomic_coercion) each hardcode OUTPUT_DIR and would still write to the committed tree, not the temp dir — each needs an --output-dir arg; (2) the _ergonomic.py stub write at lines 747–753 is outside generated_poc/ at src/adcp/types/_ergonomic.py — the atomic rename doesn't cover it; (3) temp dir must live under REPO_ROOT (e.g., .generated_poc_tmp_XXXX), not /tmp, to avoid a cross-device rename() failure on Linux; (4) a regression test that forces a late-stage failure and asserts generated_poc/ is byte-for-byte unchanged must be in the PR. Also: os.replace() is preferable to Path.rename() for the final swap (Windows-safe atomic overwrite).
• dx-expert: (agent lost to container restart; assessed from codebase conventions) --check is the right flag name — generate_versioned_stubs.py already uses it in the check-schema-drift Makefile target; exit 0 = no drift, exit 1 = drift detected. The Makefile's check-schema-drift target should switch from mutation + git diff to $(PYTHON) scripts/generate_types.py --check.

My take: This is a real data-loss bug — an interrupted run silently destroys hundreds of tracked generated files with no recovery path. The fix is non-breaking (adds argparse, --help, --check; final output and public API are unchanged), but the implementation surface is wider than it appears because three downstream scripts hardcode OUTPUT_DIR and must each accept a configurable output path.

Implementation brief:

• scripts/generate_types.py — add argparse at entry with --help (auto-exit) and --check (drift-detect, read-only). In normal mode, generate to a REPO_ROOT-relative temp dir (e.g., REPO_ROOT / f".generated_poc_tmp_{os.getpid()}"). Gate os.replace(tmp_dir, OUTPUT_DIR) on all steps passing; finally unconditionally removes the temp dir. Move _ergonomic.py staging into the temp dir alongside generated_poc/ and swap atomically.
• scripts/post_generate_fixes.py, scripts/consolidate_exports.py, scripts/generate_ergonomic_coercion.py — each must accept --output-dir (defaulting to OUTPUT_DIR for backward-compat); generate_types.py passes the temp dir path to each subprocess call.
• --check mode — generate to a second temp dir, compare file-by-file using normalize_timestamp, print drift summary, exit 1 if any file differs or is missing, exit 0 otherwise. No writes to the repo tree.
• Makefile — update check-schema-drift to call $(PYTHON) scripts/generate_types.py --check, drop the git diff block.
• Regression test — add to tests/test_code_generation.py: mock a late-stage failure (e.g., raise in apply_post_generation_fixes), assert OUTPUT_DIR is byte-for-byte unchanged via filecmp.dircmp or snapshot.
• Required checks: pytest tests/test_code_generation.py, ruff check scripts/, mypy scripts/generate_types.py.

---

Triaged by Claude Code. Session: https://claude.ai/code/session_016BJD7rYpYoEuhd72sktsoi

---

Generated by Claude Code