Skip to content

ci: redesign LKG to per-cell freeze files on orphan branch#1041

Draft
kbattocchi wants to merge 6 commits into
mainfrom
lkg-branch-redesign
Draft

ci: redesign LKG to per-cell freeze files on orphan branch#1041
kbattocchi wants to merge 6 commits into
mainfrom
lkg-branch-redesign

Conversation

@kbattocchi

@kbattocchi kbattocchi commented Jun 12, 2026

Copy link
Copy Markdown
Member

Do not merge yet. This PR is for review only. Before merging, the following remains:

  1. (Optional) Seed the lkg branch from a recent green nightly using .github/workflows/seed_lkg_branch.py. If skipped, the first post-merge nightly will populate it automatically.
  2. Run through the verification recipe in the new Maintainer guide section of the README (manual workflow_dispatch smoke tests).

What this PR does

Replaces the single merged lkg.txt / lkg-notebook.txt flow (generated by a marker-merging Python script and prone to two known bugs around partial reruns and cross-extras pinning) with per-matrix-cell freeze files committed to a long-lived orphan lkg branch. Each CI cell reads its own freeze directly — no merging anywhere.

Coverage extended to build_sdist and create_docs cells (which also benefit from pinning); build_wheels deliberately untouched since cibuildwheel manages its own envs.

Commits (review in order)

  1. ci: add push-lkg job — new producer; commits per-cell freezes to lkg from nightly / dispatch-from-main runs. Gated to github.repository == 'py-why/EconML' so forks don't pollute their own lkg branches.
  2. ci: install from per-cell lkg branch files — swaps the install path in tests, notebooks, build_sdist, create_docs to read from a sparse checkout of lkg, with a bootstrap fallback. Also deletes the old root-level lkg.txt / lkg-notebook.txt.
  3. ci: remove dead LKG-generation jobs — drops store-reqs-per-env, generate-lkg, merge-lkg, and generate_lkg.py.
  4. ci: add seed_lkg_branch.py and lint-job test step — manual seed/recovery tool with pytest coverage; new lint-job step runs the tests.
  5. docs: add Maintainer guide section — README section covering the design, file naming, trigger gating, one-time setup (ruleset recipe), seeding, and operational notes.
  6. docs: --no-verify in lkg seed recipe — a fixup of Make initial SDK public #4/Fix tests for Python 3.5 #5 left standalone so the gotcha (pre-commit blocks commits in the orphan worktree because there's no .pre-commit-config.yaml) is visible in history. Squash-friendly if you Rebase and merge.

Testing done locally

  • ruff check clean across all changed files.
  • pytest .github/workflows/tests/ — 13/13 pass.
  • python -c "import yaml; yaml.safe_load(...)" on all three modified workflow files.
  • Doctests in seed_lkg_branch.py pass.

What can't be tested without merging

The push-lkg job's gating restricts it to schedule or workflow_dispatch from main with use_lkg=false — so it only runs after the redesign is on main. Validation requires the manual verification recipe in the README.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

@kbattocchi kbattocchi force-pushed the lkg-branch-redesign branch from 04eb2e8 to c9ce5a9 Compare June 12, 2026 19:01
kbattocchi and others added 6 commits June 12, 2026 15:07
@kbattocchi
ci: add push-lkg job that commits per-cell freezes to lkg branch
71a18f2 
Adds a new push-lkg job that runs after the tests, notebooks, build, and
docs jobs on the nightly schedule (or workflow_dispatch with
use_lkg=false dispatched from main without an inputs.ref override). It
downloads the per-cell requirements-* artifacts those jobs upload,
checks out the long-lived lkg branch into a worktree, copies the freezes
in as lkg-tests-<os>-<py>-<kind>.txt / lkg-notebooks-<kind>-<py>.txt
(and, after the install-side rewrite lands, lkg-build-<os>-<py>.txt and
lkg-docs-<os>-<py>.txt), and commits + pushes with a small fetch/reset
retry loop so concurrent runs don't race.

The trigger condition is intentionally restrictive on the dispatch path:
github.ref must be refs/heads/main and inputs.ref must be empty (or
main), so manual runs that test against a future/breaking dep version on
a feature branch with use_lkg=false do NOT enshrine that version into
LKG before the breaking change has been reviewed and merged. The
schedule path is unrestricted because nightly only runs on the default
branch.

The job is serialized via an lkg-push concurrency group with
cancel-in-progress: false, gated by !cancelled() so a single failing
test cell doesn't block updating the other cells' freezes (the freeze
artifact is uploaded before pytest runs, so it captures install state
not test state), and runs with contents: write permission so it can push
to the protected lkg branch.

build_wheels is intentionally not included in the needs list because it
does not use LKG today: cibuildwheel manages its own per-Python build
envs and the use_lkg input only affects which Python versions get built.

This is the first half of the LKG redesign. The install-side rewrite
that consumes these per-cell files (and adds the matching upload steps
to build_sdist / create_docs) lands in a follow-up commit, and the old
merged-lkg.txt machinery (store-reqs-per-env / merge-artifacts /
generate-lkg / merge-lkg / generate_lkg.py) is removed after that. The
lkg branch itself must be created and seeded manually by the user before
this job will succeed; until then it will fail loudly on the checkout
step, which is the intended bootstrap signal.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Keith Battocchi <kebatt@microsoft.com>
@kbattocchi
ci: install from per-cell lkg branch files instead of merged lkg.txt
6678dd2 
Rewrites the install step in the tests, notebooks, build_sdist, and
create_docs jobs to check out the long-lived lkg branch into a
lkg-cache/ subdir and read its own per-cell freeze, falling back to a
floating install when the file is missing. The per-cell file naming
matches what push-lkg writes:

  tests cells       -> lkg-tests-<os>-<py>-<kind>.txt
  notebooks cells   -> lkg-notebooks-<kind>-<py>.txt
  build_sdist       -> lkg-build-ubuntu-latest-3.12.txt
  create_docs       -> lkg-docs-ubuntu-latest-3.12.txt

The lkg checkout uses continue-on-error: true so the bootstrap case
(branch not yet created, or this matrix cell newly added with no entry
on the branch yet) silently falls through to a floating install. A new
cell will install floating once, the per-cell freeze it uploads will be
picked up by push-lkg on the next nightly, and subsequent PR runs will
then install from that freeze.

For build_sdist and create_docs (which were previously using the merged
lkg.txt directly) this also adds the pip freeze + upload-artifact steps
so their per-cell freezes flow into push-lkg the same way tests' and
notebooks' do. build_wheels is intentionally left untouched because
cibuildwheel manages its own per-Python build envs and never read
lkg.txt.

For the notebooks job specifically, the Install notebook requirements
step (jupyter, jupyter-client, nbconvert, nbformat, seaborn, xgboost,
tqdm) is folded into the same shell block: when the per-cell freeze
exists it already pins those packages, and when it doesn't we install
them as before. This collapses two steps into one and removes the
awkward !env.use_lkg conditional.

The old root-level lkg.txt and lkg-notebook.txt are removed in this same
commit; the install fallback handles the brief window between this PR
merging and the next nightly populating the lkg branch. The push-lkg
job will fail to find files until the lkg branch is created, which is
the intended bootstrap signal.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Keith Battocchi <kebatt@microsoft.com>
@kbattocchi
ci: remove dead LKG-generation jobs and generate_lkg.py
00a2157 
The push-lkg job + per-cell install rewrite in the preceding commits make
the merged-lkg.txt machinery obsolete. Removed:

  * store-reqs-per-env  (jobs/ci.yml) - replaced by the freeze upload
    steps already present in tests, notebooks, build_sdist, create_docs.
  * generate-lkg        (jobs/ci.yml) - no merged lkg.txt to generate.
  * merge-lkg           (jobs/ci.yml) - no merged lkg artifact to bundle.
  * generate_lkg.py     (.github/workflows/) - the marker-merging Python
    script that backed generate-lkg.

merge-artifacts survives but is trimmed: it now bundles only the
non-LKG categories (requirements, tests, coverage, notebooks) for
PR-browsing UX, drops store-reqs-per-env from its needs list, and adds
build + docs so the merged requirements bundle picks up the per-cell
freezes from build_sdist and create_docs.

This removes both prior bugs by construction:

  Bug 1 (partial re-run breaks merge): there is no longer a merge step
  in the LKG path; push-lkg sees only the per-cell artifacts uploaded
  during this attempt's run.

  Bug 2 (marker-based merge over-shares versions across kinds): each
  matrix cell now reads its own freeze; no version is ever pinned in a
  cell that didn't produce that version.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Keith Battocchi <kebatt@microsoft.com>
@kbattocchi
ci: add seed_lkg_branch.py and lint-job test step
1155f7f 
This adds a manual seed/recovery tool that walks `gh run download` output
from a green nightly and copies each cell's requirements file onto the
`lkg` branch worktree under its renamed name.

Why a script:
- The normal flow is the `push-lkg` job auto-committing freezes after
  every nightly. The script exists for bootstrap (the very first seed of
  the `lkg` branch) and for manual recovery (e.g., if the user wants
  to roll the branch back to a specific run).
- The manual recipe is in the module docstring so it's discoverable from
  the file itself.

Tests:
- New `.github/workflows/tests/test_seed_lkg_branch.py` covers
  `rename_artifact_file` mapping, `collect_freezes` directory walking,
  and `main` success/failure paths.
- New `lint` job step runs them. `-o addopts=""` neutralizes the
  package-wide pytest addopts in pyproject.toml (which need pytest-xdist,
  pytest-cov, and the econml package itself — none of which are
  installed in the lint job).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Keith Battocchi <kebatt@microsoft.com>
@kbattocchi
docs: add Maintainer guide section with LKG branch flow
026027a 
Adds a new top-level `Maintainer guide` section to README.md (and a
TOC entry) covering the per-cell `lkg` branch design:

- How the `push-lkg` job and PR install path work together.
- The file naming convention shared by the push job, install steps,
  and seed_lkg_branch.py.
- The trigger gating, including why workflow_dispatch from a feature
  branch with use_lkg=false is excluded.
- One-time setup (orphan branch + branch protection).
- The manual seed/recovery recipe (also in the script's docstring).
- What to do when adding a new matrix cell or recovering from
  wedged pins on a single cell.

Section is separate from `For Developers` because that subsection is
contributor-facing (running tests, pre-commit) while this is strictly
maintainer-only (only people with push access to `lkg` care).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Keith Battocchi <kebatt@microsoft.com>
@kbattocchi
docs: use --no-verify in lkg seed recipe to skip pre-commit hook
51326a4 
The orphan `lkg` branch has no `.pre-commit-config.yaml`. If a
contributor with `pre-commit install` already run on the repo
follows the seed recipe from the README or seed_lkg_branch.py
docstring, `git commit` will abort with a confusing "No
.pre-commit-config.yaml found" error inside the lkg worktree.

`--no-verify` skips that hook for these specific commits, which is
appropriate since:

- The lkg branch contains only pip-freeze text files, not source code.
- The repo's pre-commit hook only runs ruff on python/pyi/jupyter
  files anyway, so it has nothing meaningful to check here.

Also drops the DCO sign-off (`-s`) from the seed commit example —
the lkg branch's commits represent pip freeze output, not human
authorship, so the DCO attestation doesn't conceptually apply.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Keith Battocchi <kebatt@microsoft.com>
@kbattocchi kbattocchi force-pushed the lkg-branch-redesign branch from c9ce5a9 to 51326a4 Compare June 12, 2026 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@kbattocchi