Repository navigation
Conversation
CI builds refs/pull/<n>/merge, the base branch as it stood when the run started plus the PR, but the size report compared that build with the merge-base, so every change merged to the base since the PR forked was reported as the PR's own. ci.yml now records the merge's first parent in the base-ref artifact, only when HEAD is that merge, and the size report uses it; builds made before that fall back to the merge-base as before. When only a nearer ancestor has a baseline, the comment names both commits, so the reader knows what the delta also covers.
Pruning sorted the per-commit baselines by created_at, which GitHub sets to the date of the commit a release's tag points to. Every tag in pr-test-builds points to the same commit, so all the baselines share one created_at and the sort fell back to the tag, that is to the SHA. With maintenance-10.x at its 50, a new baseline whose SHA sorted below the lowest kept one was deleted right after being published. Sort by published_at, and test the order with the created_at GitHub reports.
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
PR Summary by QodoCompare size reports against the built base and retain newest baselines
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
Code Review by Qodo
1. Reviewers can see a false size comparison
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
|
RAM / Flash usage vs. base branch — commit
See RAM/flash optimization guide for techniques to reduce usage. |
|
Test firmware build ready — commit Download firmware for PR #12088 251 targets built. Find your board's
|
1 participant
The RAM/flash comment can report changes a PR didn't make. Two separate causes, one commit each.
1. The build contains more than the baseline
The size report compares a PR's build with the baseline of the PR's merge-base (#11835). But CI doesn't build the PR head:
actions/checkouton apull_requestrun checks outrefs/pull/<n>/merge, the base branch as it stood when the run started plus the PR. So every change merged to the base since the PR forked shows up as the PR's own.#12075 shows it cleanly. It changes only
src/utils/settings.rb, which doesn't change the firmware of the base branch, yet the report says:maintenance-10.x,f1cd5c6→3931fcdd8(#11969)f1cd5c6is #12075's merge-base. #11969 was merged tomaintenance-10.xbefore the run for10407e8started, so the merge ref it built contained #11969 and the baseline didn't. The right-hand column is those two commits built locally.The change:
ci.ymlwrites the merge commit's first parent, the base commit the build contains, as a second line ofbase_ref.txt, and only when HEAD really is that merge (two parents, the second the PR head).git cat-filerather thanrev-parse HEAD^1, which fails in the depth-1 checkout.ci-size-report.ymlreads it, validated like the ref, and fetches the baseline for that commit, with the same nearest-ancestor fallback. A build without the line, made before this change or where HEAD was not that merge, falls back to the merge-base as now.This keeps what #11835 fixed: the baseline never comes from a later branch tip, and it is now the exact base the build was made from.
2. The newest baselines are the ones pruned
publish-size-baseline.shkeeps the newest 50 per-commit baselines per branch, sorting bycreated_at. For a release GitHub setscreated_atto the date of the commit its tag points to, and every tag inpr-test-buildspoints to the same commit: on 29 September all 76 baselines there hadcreated_at2026-03-01T20:49:54Z, and 76 differentpublished_at. So the sort falls back to the tag, that is to the SHA, and the 50 kept formaintenance-10.xare the 50 highest SHAs: the lowest kept one starts with6338, and the oldest dates from 30 August.A new commit whose SHA sorts below that is deleted as soon as it is published.
3931fcdd8is one: its publish run logged the release at 10:04:21 on 29 September, and it no longer exists. So even with the first commit, #12075's report would have found no baseline for3931fcdd8and walked back tof1cd5c6, the same numbers as now.The change: sort by
published_at. The test now also runsprune()on three baselines with thecreated_atGitHub really reports and checks that the first one published is the one dropped; before the change it drops the last one.Checked
3931fcdd8(a merge),git rev-parse HEAD^1fails and the newci.ymllines appendf1cd5c6f6, its first parent, with the right PR head, and nothing with a wrong one; exit status 0 both ways.base_ref.txton its own with the ref and the SHA, with CRLF endings, with one line (an older build) and with a malformed second line: the SHA is taken in the first two cases, the merge-base path in the other two.node --test .github/scripts/size-diff-comment.test.js: 31 pass (29 before, 2 new).bash .github/scripts/publish-size-baseline.test.sh: pass, and the new check fails without the second commit.Related
#11930 (on
release/9.1) changes whenci-size-report.ymlruns, this one what it compares against. They are independent and merge without conflicts.Still possible
A PR pushed right after a merge can finish its build before the base's own build has published a baseline for that commit: for #12075 the base's build took 67 minutes and the PR's 73. The comment then uses the nearest older baseline and now says so, naming both commits; re-running "CI Size Report" once the baseline exists, within the day the artifacts are kept, gives the exact delta.