Skip to content

Tech debt: commit mining walks full history instead of first-parent-only #336

Description

@miroslavpojer

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions