chore: regenerate only changed colab notebooks

[andreatnvidia]

📋 Summary

Regenerates only Colab notebooks whose source files changed, preventing unrelated random cell ID churn from appearing in pull requests. Deleted and renamed sources also remove their stale generated notebooks.

🔗 Related Issue

Closes #413

🔄 Changes

• Pass an optional FILES list from the Make target to the generator's existing --files option.
• Derive changed and deleted notebook sources from the pull request or push commit range.
• Treat renames as delete-plus-add so old generated notebooks do not remain.
• Preserve full regeneration for manual workflow runs.
• Keep the existing random cell ID filter for regenerated notebooks.

🧪 Testing

• .venv/bin/ruff check --fix .
• .venv/bin/ruff format .
• .venv/bin/pytest packages/data-designer-config/tests - 643 passed
• .venv/bin/pytest packages/data-designer-engine/tests - 2,209 passed
• .venv/bin/pytest packages/data-designer/tests - 1,105 passed, 1 skipped
• Selective single-notebook generation and ID-only diff filtering
• Historical changed-source and deleted-source range detection
• Unit tests added/updated - N/A, workflow and Makefile wiring only
• E2E tests added/updated - N/A, no runtime behavior changed

✅ Checklist

• Follows commit message conventions
• Commits are signed off (DCO)
• Architecture docs updated - N/A, no architecture change

[greptile-apps]

Greptile Summary

Selectively regenerates Colab notebooks affected by source changes while preserving full regeneration for manual runs.

• Computes changed and deleted notebook sources from the relevant Git commit range.
• Uses NUL-delimited filename transport to preserve whitespace and removes outputs for deleted or renamed sources.
• Passes the selected files through the Make target and includes newly generated files in the final diff.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
.github/workflows/check-colab-notebooks.yml	Selects changed and deleted sources safely, removes stale generated outputs, and detects newly generated untracked notebooks.
Makefile	Adds optional NUL-delimited selective generation while retaining the existing full-generation behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Workflow starts] --> B{Valid base SHA?}
  B -->|No| C[Regenerate all Colab notebooks]
  B -->|Yes| D[Collect changed and deleted sources]
  D --> E[Remove outputs for deleted sources]
  E --> F[Regenerate changed sources]
  C --> G[Mark untracked outputs intent-to-add]
  F --> G
  G --> H{Meaningful generated diff?}
  H -->|Yes| I[Fail synchronization check]
  H -->|No| J[Pass]

Loading

_{Reviews (5): Last reviewed commit: "Merge branch 'main' into andreatnvidia/c..." | Re-trigger Greptile}

[github-actions]

Thanks for putting this together, @andreatnvidia — this is a clean, well-scoped fix for a genuinely annoying problem (unrelated cell-ID churn drowning real diffs).

Summary

The PR makes the Colab-notebook check regenerate only the notebooks whose docs/notebook_source/*.py sources changed in the PR/push range, and removes generated notebooks for deleted/renamed sources. It wires an optional FILES list through the Makefile into the generator's existing --files flag. The implementation matches the stated intent, and the fallbacks (empty range → full regen) are sensibly conservative.

Findings

Suggestions — Take it or leave it

.github/workflows/check-colab-notebooks.yml:44 — Deleted sources are also placed in the files list

• What: The changed-sources query uses --diff-filter=ACDMT, which includes D. So a deleted source (e.g. 3-foo.py) ends up in both deleted and files, and gets passed to make generate-colab-notebooks FILES="… 3-foo.py".
• Why: It's harmless today — the generator skips paths where source_path.exists() is false ("⚠️ Skipping … file not found"), and the notebook was already rm -f'd — but it's slightly surprising and prints a scary-looking skip line for every deletion.
• Suggestion: Consider --diff-filter=ACMT (drop D) for the files query, since deletions are handled entirely by the separate deleted loop. Purely cosmetic; only worth it if you want the CI log to read cleanly.

.github/workflows/check-colab-notebooks.yml:70 — Generator change won't retrigger regeneration

• What: The workflow's paths: filter is limited to docs/notebook_source/*.py, and regeneration is now selective on changed sources. A change to docs/scripts/generate_colab_notebooks.py (which affects every notebook's output) neither triggers this workflow nor, if triggered another way, regenerates unchanged notebooks.
• Why: This is a pre-existing limitation of the path filter (not introduced here), but the move to selective regeneration widens the blind spot a little — a generator tweak could leave all committed notebooks stale without CI noticing.
• Suggestion: Not blocking for this PR. If you want to close the gap later, adding docs/scripts/generate_colab_notebooks.py to the trigger paths and falling back to full regeneration when the generator itself changed would cover it.

.github/workflows/check-colab-notebooks.yml:33 — Lost the "up-to-date" success log line

• What: The inverted conditional dropped the echo "✅ Colab notebooks are up-to-date …" branch; on success the step now just exits silently.
• Why: Purely a log-readability nit — a green step with no output is marginally less reassuring when scanning CI logs.
• Suggestion: Optional echo in the success path if you like the confirmation; the logic itself is equivalent and correct.

What Looks Good

• fetch-depth: 0 paired with the git diff BASE_SHA GITHUB_SHA range — necessary and correctly added; without full history the range diff would fail on the default shallow checkout.
• Rename handling via --no-renames — treating a rename as delete-old + add-new is exactly right here, so the stale generated notebook gets removed while the new one is generated. Nice.
• Conservative fallbacks — empty/all-zeros BASE_SHA (workflow_dispatch, first push of a branch) falls through to full regeneration rather than silently generating nothing. ifdef FILES in the Makefile also treats an empty FILES="" as "regenerate everything," so the safe direction is over-generation, never under-generation. Verified the ifdef behavior locally.
• ${file%.py}.ipynb path derivation for the deletion loop is correct and the check step still catches an author who deletes a source but forgets to remove the committed .ipynb.

Note: this changeset touches only the workflow YAML and the Makefile — no Python — so the ruff lint/format step and the interface→engine→config import invariants don't apply, and there's no unit-test surface to cover (the PR description confirms this).

Verdict

Ship it (with nits) — the logic is correct and the fallbacks are safe. The findings above are all optional polish; none block merge.

---

This review was generated by an AI assistant.

[nabinchha]

Thanks for putting this together, @andreatnvidia!

Summary

This PR selectively regenerates Colab notebooks from the source files changed in the relevant PR or push range, removes stale generated notebooks for deletions and renames, and preserves full regeneration for manual runs. The implementation matches the stated intent and keeps the existing generator interface small by forwarding the optional FILES list through the Make target.

Findings

I validated the existing unresolved P1 review thread and agree it should be addressed before merge; I am not opening a duplicate finding. I found no additional correctness, design, or maintainability issues beyond that thread.

What Looks Good

• The BASE_SHA fallback and empty-FILES behavior preserve full regeneration for manual dispatches and other ranges without a usable base.
• --no-renames correctly turns a rename into delete-old plus add-new, allowing the stale generated notebook to be removed and its replacement to be generated.
• Keeping deletions in the files output is useful with the current Makefile contract: on a deletion-only change it keeps FILES non-empty and avoids accidentally falling back to a full regeneration. I would not drop D from that query unless the selective/full mode is represented separately.

Verdict

Needs changes — address the existing unresolved P1 so newly generated, untracked notebooks cannot escape the final diff check. Once that is fixed, I do not see another blocker.

---

This review was generated by an AI assistant.

[andreatnvidia]

Thanks, @nabinchha. I addressed the original unresolved P1 in 8b30baf4, so new untracked notebooks now participate in the final diff. I also addressed Greptile’s follow-up whitespace-filename finding in 4d3184e2 using NUL-delimited paths. The regression checks and repository validation pass. Could you please take another look?