Context
Related to the bug in issue.md (sync-merge commits misclassified as "Direct commits").
The immediate fix there (matching a PR's real commit set via pull.get_commits()) resolves the
symptom, but it does not address the underlying design gap: commit mining pulls in the entire
commit graph of the default branch, not just the commits that actually landed on it directly.
The gap, as technical debt
data.commits is built from a full walk of the default branch's history — repo.get_commits()
in since-time mode, repo.compare() in tag-compare mode (see
release_notes_generator/data/miner.py). Both APIs return
every commit reachable from the branch tip, including commits that only exist because they were
absorbed into a PR's branch (regular work commits, and any commit created by merging the base
branch back into the PR branch, e.g. "Merge branch 'develop' into feature/XYZ").
The tool then has to work backwards from that oversized set, trying to recognize and exclude
"commits that really belong to a PR" via merge_commit_sha matching and/or commit-message
pattern matching. This is inherently reactive: every new shape of commit that can appear inside a
PR branch (sync-merges today, but potentially cherry-picks, git merge --no-ff from other
branches, bot commits, etc.) is a new case the exclusion logic has to learn about individually.
This is technical debt because:
- The definition of "direct commit" (commits that landed on the default branch outside of any PR)
is being approximated after the fact from an oversized data set, instead of being true by
construction.
- Every future edge case in how a PR's branch was built shows up as a new bug report against the
same symptom (a commit incorrectly landing in — or being excluded from — the "Direct commits ⚠️"
chapter), rather than being handled once, structurally.
- The near-term fix (
pull.get_commits() per PR) adds an API call per PR to compensate for
over-fetching in the first place, rather than not over-fetching.
Final solution: mine only first-parent commits of the default branch
Change commit mining so data.commits only ever contains commits reachable via the first-parent
chain of the default branch (equivalent to git log --first-parent), instead of the full commit
graph.
On a repository merging PRs with "Create a merge commit," the first-parent chain of develop
consists of: the commits made directly on develop (true direct commits) and the PR merge
commits themselves — never the commits that only exist inside a PR's branch (regular work commits
or sync-merges). "Direct commit" then becomes structurally correct by definition of the walk,
instead of something inferred by exclusion rules.
Impact
- Correctness: eliminates this entire class of misclassification at the source, for any current or
future shape of commit that can appear inside a PR branch — not just the sync-merge case. No
dependency on commit message wording or per-PR API calls to compensate.
- Simplifies mining:
merge_commit_sha matching and the compare-mode regex (_PR_NUMBER_RE)
become unnecessary as exclusion mechanisms — the "direct commit" set is correct as mined, and a
PR's own commits (if still needed for a PR's record) can be fetched deliberately via
pull.get_commits() only for PRs the tool is already processing, not as a workaround.
- Cost: PyGithub's
get_commits() / repo.compare() REST calls don't expose a first-parent
filter directly. Achieving a true first-parent walk requires either reconstructing the chain
client-side (starting at the branch tip, always following a commit's first listed parent SHA
via the Git Commits API) or shelling out to a local shallow clone and running
git log --first-parent. Both are more invasive than the current REST-only mining approach and
need design/testing (pagination of parents, rate-limit impact of walking commit-by-commit vs.
bulk listing).
- Behavioral change to double-check against intent: repositories that allow genuine direct pushes
to develop which are themselves merge commits (e.g., merging a hotfix branch without a PR)
would still show those as direct commits — which matches the "Direct commits ⚠️" chapter's stated
purpose (commits on the default branch without a PR) and does not regress it.
Relationship to the near-term fix
The pull.get_commits() fix in issue.md should ship first since it's low-risk and
resolves the reported symptom. This first-parent mining change is the structural fix that removes
the need for that kind of compensating exclusion logic altogether, and should be scheduled as
follow-up work rather than blocking the immediate bug fix.
Context
Related to the bug in issue.md (sync-merge commits misclassified as "Direct commits").
The immediate fix there (matching a PR's real commit set via
pull.get_commits()) resolves thesymptom, but it does not address the underlying design gap: commit mining pulls in the entire
commit graph of the default branch, not just the commits that actually landed on it directly.
The gap, as technical debt
data.commitsis built from a full walk of the default branch's history —repo.get_commits()in since-time mode,
repo.compare()in tag-compare mode (seerelease_notes_generator/data/miner.py). Both APIs return
every commit reachable from the branch tip, including commits that only exist because they were
absorbed into a PR's branch (regular work commits, and any commit created by merging the base
branch back into the PR branch, e.g.
"Merge branch 'develop' into feature/XYZ").The tool then has to work backwards from that oversized set, trying to recognize and exclude
"commits that really belong to a PR" via
merge_commit_shamatching and/or commit-messagepattern matching. This is inherently reactive: every new shape of commit that can appear inside a
PR branch (sync-merges today, but potentially cherry-picks,
git merge --no-fffrom otherbranches, bot commits, etc.) is a new case the exclusion logic has to learn about individually.
This is technical debt because:
is being approximated after the fact from an oversized data set, instead of being true by
construction.
same symptom (a commit incorrectly landing in — or being excluded from — the "Direct commits
chapter), rather than being handled once, structurally.
pull.get_commits()per PR) adds an API call per PR to compensate forover-fetching in the first place, rather than not over-fetching.
Final solution: mine only first-parent commits of the default branch
Change commit mining so
data.commitsonly ever contains commits reachable via the first-parentchain of the default branch (equivalent to
git log --first-parent), instead of the full commitgraph.
On a repository merging PRs with "Create a merge commit," the first-parent chain of
developconsists of: the commits made directly on
develop(true direct commits) and the PR mergecommits themselves — never the commits that only exist inside a PR's branch (regular work commits
or sync-merges). "Direct commit" then becomes structurally correct by definition of the walk,
instead of something inferred by exclusion rules.
Impact
future shape of commit that can appear inside a PR branch — not just the sync-merge case. No
dependency on commit message wording or per-PR API calls to compensate.
merge_commit_shamatching and the compare-mode regex (_PR_NUMBER_RE)become unnecessary as exclusion mechanisms — the "direct commit" set is correct as mined, and a
PR's own commits (if still needed for a PR's record) can be fetched deliberately via
pull.get_commits()only for PRs the tool is already processing, not as a workaround.get_commits()/repo.compare()REST calls don't expose a first-parentfilter directly. Achieving a true first-parent walk requires either reconstructing the chain
client-side (starting at the branch tip, always following a commit's first listed parent SHA
via the Git Commits API) or shelling out to a local shallow clone and running
git log --first-parent. Both are more invasive than the current REST-only mining approach andneed design/testing (pagination of parents, rate-limit impact of walking commit-by-commit vs.
bulk listing).
to
developwhich are themselves merge commits (e.g., merging a hotfix branch without a PR)would still show those as direct commits — which matches the "Direct commits
purpose (commits on the default branch without a PR) and does not regress it.
Relationship to the near-term fix
The
pull.get_commits()fix in issue.md should ship first since it's low-risk andresolves the reported symptom. This first-parent mining change is the structural fix that removes
the need for that kind of compensating exclusion logic altogether, and should be scheduled as
follow-up work rather than blocking the immediate bug fix.