Skip to content

CI - FEATURE - Support stacked pull requests in the test and harness-stamp checks - #537

Merged
matt-pharr merged 2 commits into
developfrom
feature/stacked-pr-ci
Oct 19, 2026
Merged

matt-pharr merged 2 commits into
developfrom
feature/stacked-pr-ci

Conversation

@matt-pharr

@matt-pharr matt-pharr commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Release note

  • Audience: developers
  • Numerical impact: none
  • Migration: none

Pull requests can now be stacked on another pull request's branch: tests run on them, and their harness stamp survives updates to the parent.

Regression report

Not run: no changes under src/.

Notes for reviewers

  • test.yaml: dropped the main/develop base filter, so stacked PRs (e.g. ForceFreeStates - BUGFIX! - Trigger the Gaussian reduction on the conditioning of U₁ and factor U₁ in the plasma response matrix #520) get a Tests run.
  • Harness stamp: while a PR targets a feature branch, only its own src/ changes are compared (git diff -U0 merge-base..X -- src/ | git patch-id, at the stamp vs. HEAD), so merging the parent in doesn't make it stale. PRs targeting develop/main keep the existing strict check. I tested this against a simulated stack.
  • Docs: a "Stacked Pull Requests" section in git-workflow.md, covering merging instead of rebasing, no squash-merging a parent, and running the harness against the base branch.
  • Cost: every push to a stacked layer runs the full 2×~40 min matrix.

🤖 Generated with Claude Code

…stamp checks

Run the test suite on pull requests of any base, and check a stacked pull
request's harness stamp against its own src/ changes rather than the whole
tree, so merging an updated parent in does not invalidate it. Pull requests
targeting develop or main keep the strict check. Document the stacked
workflow and the base-branch harness baseline.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@matt-pharr matt-pharr self-assigned this Oct 19, 2026
@matt-pharr
matt-pharr requested a review from logan-nc October 19, 2026 09:00
@github-actions github-actions Bot added the feature New capability label Oct 19, 2026
@matt-pharr

Copy link
Copy Markdown
Collaborator Author

@logan-nc this is a very minor change that should enable us to use PR stacks which should help push through some of these changes for GPEC 2.0.0 faster without having to rerun regression harnesses a bunch of times.

@logan-nc

Copy link
Copy Markdown
Collaborator

@claude review this pr

@claude

claude Bot commented Oct 19, 2026

Copy link
Copy Markdown

Review of #537

No blocking issues. The stamp logic is sound: pr-conventions.yaml checks out with fetch-depth: 0, so git merge-base works on both the stamp and HEAD. -U0 plus patch-id --stable ignores context-only shifts from merging the parent. The edited trigger re-runs the check when the base is changed, so retargeting to develop applies the strict check again.

Points to consider:

  1. Loosened check. Any base other than develop/main (e.g. a hotfix/ or release branch, or a throwaway branch) now gets the weaker check. That is the intent for stacks, but a PR retargeted to a scratch branch skips the strict check until it is retargeted back. The strict check on the final retarget to develop covers this, and the PR text could say so. Only these two branch names are special-cased, so any future long-lived branch needs adding to that list.
  2. Interaction blind spot. The own-patch comparison can't detect a parent change that alters numerics for this PR's code without touching the PR's own src/ diff. The docs already say to run the harness against the base branch. Mentioning the limit in git-workflow.md would help reviewers.
  3. CI cost. Removing the branches: filter in test.yaml runs the full ~2×40 min matrix on every push to any PR, including drafts and docs-only stacked layers. Cancel-in-progress limits the waste. Consider a paths-ignore or a draft skip if the cost becomes a problem.
  4. Testing. The PR says this was tested against a simulated stack, but there is no committed test for the shell logic (own_src_patch). Moving it into ci/conventions/ with a small test would keep it from regressing. This is optional for a CI-only change.
  5. Docs. The git-workflow.md section is clear and useful. The PULL_REQUEST_TEMPLATE.md addition is a good place to point authors at the stacked baseline.

The regression harness was not run. That is reasonable because nothing under src/ changes.


⚠️ HUMAN REVIEW IS REQUIRED BEFORE MERGE: NON-NEGOTIABLE ⚠️

This PR must not be merged into develop without approval from a third-party human reviewer. logan-nc is requested and matt-pharr is assigned. Neither has approved yet.

🤖 Generated with Claude Code

@matt-pharr
matt-pharr merged commit 12dcb4c into develop Oct 19, 2026
7 checks passed
@matt-pharr
matt-pharr deleted the feature/stacked-pr-ci branch October 19, 2026 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New capability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants