Skip to content

Support ChangeLog for the new branch model [skip ci] - #13687

Merged
NvTimLiu merged 3 commits into
NVIDIA:branch-25.12from
NvTimLiu:changelog-new-branch-model
Oct 31, 2025
Merged

Support ChangeLog for the new branch model [skip ci]#13687
NvTimLiu merged 3 commits into
NVIDIA:branch-25.12from
NvTimLiu:changelog-new-branch-model

Conversation

@NvTimLiu

Copy link
Copy Markdown
Collaborator

1, Retrieve commit hashes for a release after enabling the new branch model.

2, Create a query to fetch pull request (PR) information from GitHub using commit hashes.

3, Support retrieving PR ChangeLogs for both the old and new branch models.

1, Retrieve commit hashes for a release after enabling the new branch model.

2, Create a query to fetch pull request (PR) information from GitHub using commit hashes.

3, Support retrieving PR changelogs for both the old and new branch models.

Signed-off-by: timl <timl@nvidia.com>
@NvTimLiu NvTimLiu self-assigned this Oct 29, 2025
Copilot AI review requested due to automatic review settings October 29, 2025 03:17
@NvTimLiu
NvTimLiu requested a review from a team as a code owner October 29, 2025 03:17
@NvTimLiu NvTimLiu added the build Related to CI / CD or cleanly building label Oct 29, 2025
@NvTimLiu
NvTimLiu marked this pull request as draft October 29, 2025 03:17

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

This PR modernizes the changelog generation script to accommodate the repository's branch naming transition from branch-YY.MM to release/YY.MM, starting at version 25.12. The implementation adds a commit-based PR lookup path using git log and GitHub's GraphQL API, while preserving backward compatibility with the legacy branch-name query method. The logic pivots on a hardcoded cutoff (FROM_RELEASE = '25.12') and handles three scenarios: both releases after the cutoff (commit-based), mixed mode at the cutoff, and both before the cutoff (branch-based). The script now sorts releases in descending order to ensure git range syntax works correctly, since get_commits() expects the newest release first for constructing from_branch..to_branch ranges.

Important Files Changed

Filename Score Overview
scripts/generate-changelog 3/5 Adds commit-based PR retrieval for new branch model, with hybrid logic handling three release scenarios and git log parsing.

Confidence score: 3/5

  • This PR introduces significant control-flow branching and external process calls (git, GraphQL), increasing the surface area for runtime errors and edge cases.
  • Score reflects three concerns: line 325 incorrectly treats a sorted list as a set (will always take first element), line 250 silently takes only the first associated PR per commit (losing multi-PR commits), and line 255uses a bare except that swallows all exceptions including keyboard interrupts and network failures.
  • Pay close attention to the process_pr() function (lines 323-345) and get_pr_via_commits() (lines 244-258), as these contain the problematic set/list confusion, silent exception handling, and assumptions about commit-to-PR cardinality.

Sequence Diagram

sequenceDiagram
    participant User
    participant main
    participant process_changelog
    participant process_pr
    participant get_commits
    participant get_pr_via_commits
    participant fetch
    participant post
    participant GitHub API
    participant form_changelog

    User->>main: "Execute script with --releases, --token, --path"
    main->>main: "Parse arguments and validate GITHUB_TOKEN"
    main->>process_changelog: "Process PRs for releases"
    process_changelog->>process_pr: "Get PRs based on releases"
    
    alt Current version > FROM_RELEASE (25.12)
        process_pr->>get_commits: "Get commit hashes between release tags"
        get_commits->>get_commits: "Execute git log for each release pair"
        get_commits-->>process_pr: "Return ver_commits dict"
        process_pr->>get_pr_via_commits: "Get PRs from commit hashes"
        loop For each commit SHA
            get_pr_via_commits->>post: "Query PR by commit SHA"
            post->>GitHub API: "POST query_pr_by_commit with SHA"
            GitHub API-->>post: "Return PR data"
            post-->>get_pr_via_commits: "Return response"
            get_pr_via_commits->>get_pr_via_commits: "Add PR to list if merged and unique"
        end
        get_pr_via_commits-->>process_pr: "Return PR list"
    else Current version == FROM_RELEASE
        process_pr->>get_commits: "Get commits for FROM_RELEASE"
        get_commits-->>process_pr: "Return ver_commits"
        process_pr->>get_pr_via_commits: "Get PRs from commits"
        get_pr_via_commits-->>process_pr: "Return PR list"
        process_pr->>fetch: "Fetch PRs for previous branch"
        fetch->>post: "Query PRs with baseRefName"
        post->>GitHub API: "POST query_pr"
        GitHub API-->>post: "Return PR data"
        post-->>fetch: "Return response"
        fetch-->>process_pr: "Return additional PRs"
    else Current version < FROM_RELEASE
        loop For each release
            process_pr->>fetch: "Fetch PRs for branch-{release}"
            fetch->>post: "Query PRs with baseRefName"
            post->>GitHub API: "POST query_pr"
            GitHub API-->>post: "Return PR data"
            post-->>fetch: "Return response"
            fetch-->>process_pr: "Extend PR list"
        end
    end
    
    process_pr-->>process_changelog: "Return all PRs"
    process_changelog->>process_changelog: "Categorize PRs by project and labels"
    process_changelog-->>main: "Update changelog dict with PRs"
    
    main->>process_changelog: "Process Issues for releases"
    process_changelog->>process_issue: "Get issues from last 3*N months"
    process_issue->>fetch: "Fetch issues with since timestamp"
    loop While hasNextPage
        fetch->>post: "Query issues"
        post->>GitHub API: "POST query_issue"
        GitHub API-->>post: "Return issue data"
        post-->>fetch: "Return response"
        fetch->>fetch: "Append issues and update cursor"
    end
    fetch-->>process_issue: "Return all issues"
    process_issue-->>process_changelog: "Return issues"
    process_changelog->>process_changelog: "Categorize issues by project and labels"
    process_changelog-->>main: "Update changelog dict with issues"
    
    main->>form_changelog: "Generate markdown from changelog dict"
    form_changelog->>form_changelog: "Sort projects, format sections"
    form_changelog->>form_changelog: "Write to file"
    form_changelog-->>main: "Changelog file created"
    main-->>User: "Done"
Loading

1 file reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

Comment thread scripts/generate-changelog
Comment thread scripts/generate-changelog
Comment thread scripts/generate-changelog Outdated
@NvTimLiu

Copy link
Copy Markdown
Collaborator Author

Build PASS:

image image

@NvTimLiu NvTimLiu changed the title Support changelog for the new branch model: Support ChangeLog for the new branch model [skip ci] Oct 29, 2025

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR modifies the changelog generation script to support a transition from branch-based releases to release-branch-based releases starting with version 25.12. The script now uses git commit history to find PRs for releases after 25.12, while continuing to use the GitHub API for earlier releases.

Key changes:

  • Added logic to fetch PRs via git commit history for releases >= 25.12
  • Implemented version-based branching strategy detection (branch-YY.MM vs release/YY.MM)
  • Maintained backward compatibility for releases before 25.12

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/generate-changelog Outdated
Comment thread scripts/generate-changelog
Comment thread scripts/generate-changelog
Comment thread scripts/generate-changelog Outdated
Comment thread scripts/generate-changelog
Comment thread scripts/generate-changelog
Signed-off-by: timl <timl@nvidia.com>
pxLi
pxLi previously approved these changes Oct 31, 2025
@pxLi

pxLi commented Oct 31, 2025

Copy link
Copy Markdown
Member

some follow updates could be taken care later:

  1. Add NOTE for compatibility support
  2. Concurrently send requests to query PR by commit
  3. Dynamically detect local git remote ref

@NvTimLiu
NvTimLiu marked this pull request as ready for review October 31, 2025 07:04
@NvTimLiu

Copy link
Copy Markdown
Collaborator Author

build

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

Extended changelog generation to support the new branch model (release/YY.MM) starting from version 25.12, while maintaining backward compatibility with the old model (branch-YY.MM). Adds commit-based PR retrieval using GraphQL queries, and implements logic to determine which branch naming convention to use based on release versions.

Key changes:

  • Added get_commits() to retrieve commit hashes between release branches using git log
  • Added get_pr_via_commits() to fetch PR metadata from GitHub API using commit SHAs
  • Modified process_pr() to route to old vs new branch model based on FROM_RELEASE threshold (25.12)
  • Added get_prev_release_version() utility to calculate previous release version

Critical issues found:

  • Logic bug in process_pr() line 330: assumes all releases use new model when highest version > FROM_RELEASE, but list can contain older versions requiring old model
  • Bug in get_commits() line 214: always uses origin/release/{to_rel} prefix, even for releases before FROM_RELEASE
  • Type hint mismatches between function signatures and actual parameters
  • Cherry-picked commit handling may select wrong PR (existing comment)

Confidence Score: 2/5

  • This PR has critical logic bugs that will cause failures when generating changelogs for mixed old/new release versions
  • Score reflects two critical logic bugs: (1) process_pr() incorrectly routes all releases to new branch model when only the highest version exceeds FROM_RELEASE, causing git failures for older releases, and (2) get_commits() always uses release/ prefix for to_branch regardless of version. These bugs will break changelog generation for common scenarios like releases=['26.02', '25.10']. Additionally, there are type hint inconsistencies and the existing cherry-pick issue remains unresolved.
  • Critical attention needed for scripts/generate-changelog lines 214, 330-332 to fix branch model routing logic

Important Files Changed

File Analysis

Filename Score Overview
scripts/generate-changelog 2/5 Added support for new release branch model (release/YY.MM), but has critical logic bug in process_pr that incorrectly assumes all releases use new model when highest version > FROM_RELEASE, and get_commits always uses release/ prefix for to_branch

Sequence Diagram

sequenceDiagram
    participant User
    participant main
    participant process_pr
    participant get_commits
    participant get_pr_via_commits
    participant GitHub API

    User->>main: --releases=26.02,25.12
    main->>main: Sort releases descending → [26.02, 25.12]
    main->>process_pr: releases=[26.02, 25.12]
    
    alt current_ver > FROM_RELEASE (26.02 > 25.12)
        process_pr->>get_commits: All releases
        loop For each release
            get_commits->>get_commits: Determine from/to branches
            Note over get_commits: to_branch = origin/release/{ver}<br/>from_branch = origin/release/ or branch-
            get_commits->>get_commits: git log from_branch..to_branch
        end
        get_commits-->>process_pr: ver_commits dict
        process_pr->>get_pr_via_commits: ver_commits
        loop For each commit
            get_pr_via_commits->>GitHub API: query_pr_by_commit(sha)
            GitHub API-->>get_pr_via_commits: PR info
            get_pr_via_commits->>get_pr_via_commits: Deduplicate & filter
        end
        get_pr_via_commits-->>process_pr: PR list
    else current_ver == FROM_RELEASE (25.12)
        process_pr->>get_commits: {FROM_RELEASE}
        process_pr->>get_pr_via_commits: commits for 25.12
        process_pr->>GitHub API: fetch PRs for branch-{prev_ver}
        GitHub API-->>process_pr: Old model PRs
    else current_ver < FROM_RELEASE
        loop For each release
            process_pr->>GitHub API: fetch(baseRefName=branch-{rel})
            GitHub API-->>process_pr: PRs
        end
    end
    
    process_pr-->>main: Combined PR list
    main->>main: Generate changelog markdown
Loading

1 file reviewed, 5 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +330 to +332
if current_ver_float > based_rel:
ver_commits = get_commits(releases)
pr = get_pr_via_commits(ver_commits, token)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Logic assumes all releases use new branch model when current_ver > FROM_RELEASE, but releases list can contain versions ≤ FROM_RELEASE. Example: if releases=['26.02', '25.10'], it calls get_commits which tries origin/release/25.10 (doesn't exist, should be origin/branch-25.10). Consider filtering releases or checking each version individually.

ver_commits = {}
count = len(rel_list) # descending version order assured
for i, to_rel in enumerate(rel_list):
to_branch = f"origin/release/{to_rel}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Always uses origin/release/{to_rel} even for releases before FROM_RELEASE. Should check if float(to_rel) >= float(FROM_RELEASE) before deciding branch name, similar to from_branch logic on lines 221-224.

# Get the commit hashes between two branches or release tags.
# param releases: set of release versions, e.g. {'YY.MM2', 'YY.MM1'}
# return: dict of commit hashes, e.g. {YY.MM2: [sha1, sha2, ...], YY.MM1: [shaX, shaY, ...]}
def get_commits(releases: set):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Type hint says set but receives sorted list from line 435. Update to def get_commits(releases: list): or keep as set and handle sorting internally.

# param ver_commits, e.g. {v1: [sha1, sha2, ...], v2: [shaX, shaY, ...]}
# param token: the token for the API
# return: list of PRs associated with the commit hashes, e.g. [{PR1 info}, {PR2 info}, ...]
def get_pr_via_commits(ver_commits: set, token: str):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: Type hint says set but receives dict from get_commits. Should be def get_pr_via_commits(ver_commits: dict, token: str):

]

# Use check=True to raise exception if git fails, making errors explicit
result = subprocess.run(git_log_args, capture_output=True, text=True, check=True)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Git command will fail silently returning empty commits if branches don't exist. Consider checking branch existence first or handling the specific error case separately from other exceptions.

… the changelog

Signed-off-by: timl <timl@nvidia.com>
@NvTimLiu

Copy link
Copy Markdown
Collaborator Author

build

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

Extends changelog generation to support the new branch model where release branches transition from branch-YY.MM to release/YY.MM starting from version 25.12.

Key Changes:

  • Adds commit-hash-based PR retrieval using get_commits() and get_pr_via_commits() for releases ≥ 25.12
  • Implements hybrid approach in process_pr() to handle old model (baseRefName queries), new model (commit-based), and transition release (25.12)
  • Adds FROM_RELEASE constant (25.12) to mark the transition point
  • Introduces get_prev_release_version() to calculate 2-month-prior releases

Critical Issues Found:

  • Line 214: to_branch always uses origin/release/{to_rel} format, which will fail for mixed-version lists containing releases < 25.12
  • Line 210: Converting sorted list back to unsorted list loses the descending order required by loop logic
  • Line 325: Assumes releases (typed as set) is ordered, but works only due to line 436's conversion to sorted list
  • Type inconsistencies between function signatures (expecting set) and actual usage (passing sorted list)

Confidence Score: 2/5

  • PR has critical logic errors that will cause failures with mixed-version release lists
  • The implementation has fundamental issues: (1) to_branch determination on line 214 doesn't check if to_rel uses old/new model, causing git commands to fail for pre-25.12 releases, (2) type inconsistencies where functions expect sets but receive lists, (3) ordering dependencies that break when converting sorted lists back to unordered structures. These will cause runtime failures when generating changelogs for mixed-version scenarios
  • scripts/generate-changelog requires fixes to branch name logic, type consistency, and ordering preservation

Important Files Changed

File Analysis

Filename Score Overview
scripts/generate-changelog 2/5 Adds new branch model support with commit-based PR retrieval, but has critical logic errors in branch name determination, type inconsistencies, and ordering assumptions

Sequence Diagram

sequenceDiagram
    participant User
    participant main
    participant process_pr
    participant get_commits
    participant get_pr_via_commits
    participant GitHub API

    User->>main: Call with releases (e.g., "26.02,25.12")
    main->>main: Parse and sort releases descending
    main->>process_pr: Pass sorted releases
    
    alt current_ver > FROM_RELEASE (25.12)
        process_pr->>get_commits: Get commits for all releases
        loop For each release
            get_commits->>get_commits: Determine to_branch (origin/release/YY.MM)
            get_commits->>get_commits: Determine from_branch (check < FROM_RELEASE)
            get_commits->>GitHub API: git log from_branch..to_branch
            GitHub API-->>get_commits: Return commit hashes
        end
        get_commits-->>process_pr: Return ver_commits dict
        process_pr->>get_pr_via_commits: Pass ver_commits and token
        loop For each commit in each version
            get_pr_via_commits->>GitHub API: Query PR by commit SHA
            GitHub API-->>get_pr_via_commits: Return associated PRs
            get_pr_via_commits->>get_pr_via_commits: Filter and deduplicate
        end
        get_pr_via_commits-->>process_pr: Return PR list
    else current_ver == FROM_RELEASE
        process_pr->>get_commits: Get commits for FROM_RELEASE only
        process_pr->>get_pr_via_commits: Get PRs via commits
        process_pr->>GitHub API: Fetch PRs for previous release (old model)
        GitHub API-->>process_pr: Return PRs
    else current_ver < FROM_RELEASE
        loop For each release
            process_pr->>GitHub API: Fetch PRs by baseRefName (branch-YY.MM)
            GitHub API-->>process_pr: Return PRs
        end
    end
    
    process_pr-->>main: Return all PRs
    main->>main: Process and generate changelog
Loading

1 file reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

ver_commits = {}
count = len(rel_list) # descending version order assured
for i, to_rel in enumerate(rel_list):
to_branch = f"origin/release/{to_rel}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: to_branch assumes new branch model for all releases in the list, but mixed release versions (e.g., ['26.02', '25.10']) will fail for older releases

Suggested change
to_branch = f"origin/release/{to_rel}"
to_rel_float = float(to_rel)
if to_rel_float < float(FROM_RELEASE):
to_branch = f"origin/branch-{to_rel}"
else:
to_branch = f"origin/release/{to_rel}"

# param releases: set of release versions, e.g. {'YY.MM2', 'YY.MM1'}
# return: dict of commit hashes, e.g. {YY.MM2: [sha1, sha2, ...], YY.MM1: [shaX, shaY, ...]}
def get_commits(releases: set):
rel_list = list(releases)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Converting sorted list to unsorted list loses the ordering that line 436 established. Ordering matters for the loop logic on lines 216-219

Suggested change
rel_list = list(releases)
rel_list = releases # Already sorted in descending order

def process_pr(releases: set, token: str):
pr = []
for rel in releases:
current_ver = list(releases)[0]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: list(releases)[0] assumes ordering but releases is typed as set (unordered). Works due to line 436 conversion but type hint is incorrect

Suggested change
current_ver = list(releases)[0]
current_ver = releases[0] # releases is already a sorted list

@NvTimLiu
NvTimLiu merged commit f34ab9c into NVIDIA:branch-25.12 Oct 31, 2025
60 checks passed
@pxLi

pxLi commented Oct 31, 2025

Copy link
Copy Markdown
Member

bad base ref

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Related to CI / CD or cleanly building

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants