Skip to content

Bring size-diff baseline fixes to maintenance-10.x - #11805

Merged
sensei-hacker merged 2 commits into
iNavFlight:maintenance-10.xfrom
sensei-hacker:feature-ci-ram-flash-diff-comment-10x
Aug 21, 2026
Merged

Bring size-diff baseline fixes to maintenance-10.x#11805
sensei-hacker merged 2 commits into
iNavFlight:maintenance-10.xfrom
sensei-hacker:feature-ci-ram-flash-diff-comment-10x

Conversation

@sensei-hacker

Copy link
Copy Markdown
Member

Summary

maintenance-10.x never got two fixes to the RAM/flash size-diff PR
comment feature that already landed on master and release/9.1. As a
result, pushes to maintenance-10.x (e.g. #11753, merged a few hours ago)
never produced a size baseline — nightly-build.yml's own copy on this
branch didn't even list maintenance-10.x as a trigger branch, so "Build
pre-release" never fired for that push at all.

Root Cause

push-triggered workflows use the copy of the workflow file present on
the branch actually being pushed to, not master's copy. maintenance-10.x's
copy of nightly-build.yml still only listed master/maintenance-8.x.x/
maintenance-9.x — the addition of maintenance-10.x and release/9.1
(from an earlier fix) had only reached master and release/9.1 themselves,
never maintenance-10.x.

Changes

Cherry-picks two commits already reviewed and merged elsewhere, no new code:

  • nightly-build.yml: add maintenance-10.x/release/9.1 to the push
    branch list.
  • ci.yml: fix "Save branch name" step's condition
    (github.event_name == 'push'!= 'pull_request') — it was never
    true when ci.yml runs via nightly-build.yml's workflow_call,
    since github.event_name reports 'workflow_call' there, not 'push'.
  • ci-size-report.yml: publish-baseline listens to "Build pre-release"
    instead of the dead ci.yml push trigger, and no longer gates on the
    aggregate workflow_run.conclusion (a separate, unrelated Release job
    failure was dragging that down even when the build itself succeeded —
    see Fix publish-baseline never firing due to unrelated Release job failures #11801 for the full diagnosis on release/9.1).
  • README.md: corresponding doc corrections.

Testing

  • Both commits cherry-picked cleanly onto maintenance-10.x with no
    conflicts.
  • YAML syntax validated for all three workflow files.
  • shellcheck clean on the size-extraction scripts (unchanged by this PR,
    verified as a sanity check).
  • Can't fully exercise a workflow_run-triggered workflow locally; will
    confirm live once merged and a subsequent push to maintenance-10.x
    triggers "Build pre-release".

Code Review

Both commits were already reviewed with the inav-code-review agent when
first merged to master/release/9.1 (see #11801). No new code in this PR.

Related

Part of the size-diff PR comment feature (#11791/#11794/#11795/#11796/#11797/#11801).

…ush trigger

ci.yml's own push trigger is a no-op (a branches: list with only a negative
pattern matches nothing per GitHub's docs; confirmed zero push-triggered
runs exist in this repo's history). nightly-build.yml ("Build pre-release")
is push-triggered correctly and already invokes ci.yml's jobs via
workflow_call to build nightly releases -- listen to it instead, at no
extra build cost. Add maintenance-10.x and release/9.1 to its branch list
so baselines exist for those bases too.

Also fix ci.yml's "Save branch name" step: it was gated on
github.event_name == 'push', but that reports as 'workflow_call' when
invoked from nightly-build.yml, not 'push' -- so branch.txt would never
have been produced on the path this now depends on.
The Release job (nightly upload to iNavFlight/inav-nightly) can fail for
reasons unrelated to the build itself and drags the whole run's
conclusion to failure even when the build succeeded and produced the
size-report/branch-name artifacts. Confirmed live: the 2026-08-20 push
to release/9.1 had every build job succeed but Release fail with "Bad
credentials" (NIGHTLY_TOKEN), so publish-baseline never ran and no
baseline has ever been published — which is why PR iNavFlight#11800 still shows
"No size baseline is available yet".

Check the specific build/upload-artifacts job's conclusion instead of
the aggregate run conclusion.
@sensei-hacker sensei-hacker added this to the 10.0 milestone Aug 21, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Fix size-diff baseline publishing for maintenance-10.x workflows

🐞 Bug fix ⚙️ Configuration changes 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Ensure pushes to maintenance-10.x trigger "Build pre-release" and baseline generation.
• Publish baselines from nightly-build workflow runs, not ci.yml’s broken push trigger.
• Avoid false baseline skips by checking the build job outcome, not overall run conclusion.
Diagram

graph TD
  A["Push to branch"] --> B["Build pre-release (nightly-build.yml)"] --> C["Build firmware (ci.yml)"] --> D("Artifacts: size-report + branch-name") --> E["CI Size Report (ci-size-report.yml)"] --> F[("pr-test-builds baseline asset")]
  G["Pull request"] --> C --> D --> E --> H["PR size-diff comment"]
  subgraph Legend
    direction LR
    _wf["Workflow"] ~~~ _art("Artifact") ~~~ _store[("Release asset")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Fix ci.yml push trigger and publish baselines from ci.yml runs
  • ➕ More direct mental model: baseline publishing always follows ci.yml regardless of caller
  • ➕ Avoids coupling to nightly-build.yml naming/structure
  • ➖ Risk of enabling unintended push CI on branches (extra compute / policy change)
  • ➖ Still requires handling workflow_call vs push semantics for branch artifacts
2. Keep workflow_run conclusion gating, but split nightly-build release into separate workflow
  • ➕ Removes need for GH API job-conclusion probing
  • ➕ Cleaner separation: build success is not affected by release upload failures
  • ➖ Larger CI restructuring for a maintenance backport
  • ➖ More files and migration risk than the current cherry-pick fix

Recommendation: Keep the PR’s approach for maintenance-10.x: it’s a minimal-risk backport that restores baseline generation without changing CI policy/compute. The added GH API job-level conclusion check is a pragmatic guard against unrelated Release-job failures blocking baseline publishing, while preserving existing PR-comment behavior.

Files changed (4) +99 / -28

Bug fix (2) +68 / -19
ci-size-report.ymlPublish baselines from Build pre-release runs and gate on build job success +62/-17

Publish baselines from Build pre-release runs and gate on build job success

• Extends workflow_run triggers to include "Build pre-release" and routes baseline publishing to that upstream workflow for push events. Replaces overall run-conclusion gating with a GH API query that checks the specific build/artifact-producing job’s conclusion before downloading artifacts and publishing the baseline.

.github/workflows/ci-size-report.yml

ci.ymlEnsure branch-name artifact is produced for workflow_call runs +6/-2

Ensure branch-name artifact is produced for workflow_call runs

• Changes the branch-name steps to run for any non-pull_request event, covering both direct pushes and workflow_call invocations. This ensures downstream baseline publishing can reliably resolve the head branch name.

.github/workflows/ci.yml

Documentation (1) +29 / -9
README.mdDocument actual triggers and baseline flow for size-diff feature +29/-9

Document actual triggers and baseline flow for size-diff feature

• Updates workflow documentation to reflect that ci.yml push triggering is effectively broken and that per-push builds/baselines come from nightly-build.yml. Clarifies that ci-size-report listens to both PR builds (Build firmware) and push builds (Build pre-release).

.github/workflows/README.md

Other (1) +2 / -0
nightly-build.ymlTrigger Build pre-release on maintenance-10.x and release/9.1 pushes +2/-0

Trigger Build pre-release on maintenance-10.x and release/9.1 pushes

• Adds maintenance-10.x and release/9.1 to the push branch allowlist so the reusable build workflow runs on those branches. This restores the artifact-producing build path needed for baseline publishing on maintenance-10.x.

.github/workflows/nightly-build.yml

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Brittle job-name gate 🐞 Bug ☼ Reliability
Description
ci-size-report.yml gates baseline publishing on finding a job named exactly `"build /
upload-artifacts"` in the upstream run; any caller job rename (nightly-build) or callee job rename
(ci.yml) will cause proceed=false and baseline publishing will be skipped.
Code

.github/workflows/ci-size-report.yml[R60-61]

+          CONCLUSION=$(gh api "repos/${{ github.repository }}/actions/runs/${RUN_ID}/jobs" --paginate \
+            --jq '.jobs[] | select(.name == "build / upload-artifacts") | .conclusion')
Evidence
The new logic queries the upstream run’s job list and only proceeds when a job with the exact name
build / upload-artifacts is found and concluded successfully; that exact string depends on both
the caller job name (build) and the callee job id (upload-artifacts).

.github/workflows/ci-size-report.yml[54-76]
.github/workflows/nightly-build.yml[22-26]
.github/workflows/ci.yml[179-183]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`publish-baseline` checks upstream success by querying workflow-run jobs and selecting a single hard-coded job name (`build / upload-artifacts`). This is fragile because GitHub job display names can change when either the caller job name (in `nightly-build.yml`) or the callee job id/name (in `ci.yml`) changes, causing baseline publishing to be skipped.

### Issue Context
- Caller workflow `nightly-build.yml` defines the reusable-workflow caller job name as `build`.
- Callee workflow `ci.yml` contains a job id `upload-artifacts`.
- The current jq filter requires an exact combined display name string.

### Fix Focus Areas
- .github/workflows/ci-size-report.yml[54-76]

### Suggested change
Update the jq selection to be resilient, e.g. match by suffix:
- select(.name | endswith("/ upload-artifacts") or endswith(" / upload-artifacts") or .name == "upload-artifacts")

Optionally:
- assert exactly one match and emit an error/warning if multiple matches occur
- include the matched job name in logs for easier debugging

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

2. Branch coverage comment wrong 🐞 Bug ⚙ Maintainability
Description
ci-size-report.yml’s header comment claims baseline publishing only covers master,
maintenance-9.x, maintenance-10.x, and release/9.1, but nightly-build.yml also triggers
“Build pre-release” on maintenance-8.x.x, so baselines will also be published for that branch.
Code

.github/workflows/ci-size-report.yml[R17-20]

+#    rebuild the base branch to get a comparison point. Only fires for
+#    branches nightly-build.yml's own push trigger covers (currently
+#    master, maintenance-9.x, maintenance-10.x, release/9.1 — see that
+#    file). Stale baselines for since-deleted branches aren't cleaned up
Evidence
The workflow comment enumerates covered branches but omits maintenance-8.x.x, while the push
trigger list in nightly-build.yml explicitly includes it, so the actual behavior includes it.

.github/workflows/ci-size-report.yml[17-20]
.github/workflows/nightly-build.yml[5-13]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The comment describing which branches are covered by `nightly-build.yml` is inconsistent with the actual `nightly-build.yml` trigger list, which includes `maintenance-8.x.x`.

### Issue Context
This is a documentation mismatch inside the workflow file itself; it can mislead future maintainers about which branches will have baselines published.

### Fix Focus Areas
- .github/workflows/ci-size-report.yml[17-20]
- .github/workflows/nightly-build.yml[6-13]

### Suggested change
Update the comment to include `maintenance-8.x.x` (or, if unintended, remove that branch from the `nightly-build.yml` trigger list).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment on lines +60 to +61
CONCLUSION=$(gh api "repos/${{ github.repository }}/actions/runs/${RUN_ID}/jobs" --paginate \
--jq '.jobs[] | select(.name == "build / upload-artifacts") | .conclusion')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Brittle job-name gate 🐞 Bug ☼ Reliability

ci-size-report.yml gates baseline publishing on finding a job named exactly `"build /
upload-artifacts"` in the upstream run; any caller job rename (nightly-build) or callee job rename
(ci.yml) will cause proceed=false and baseline publishing will be skipped.
Agent Prompt
### Issue description
`publish-baseline` checks upstream success by querying workflow-run jobs and selecting a single hard-coded job name (`build / upload-artifacts`). This is fragile because GitHub job display names can change when either the caller job name (in `nightly-build.yml`) or the callee job id/name (in `ci.yml`) changes, causing baseline publishing to be skipped.

### Issue Context
- Caller workflow `nightly-build.yml` defines the reusable-workflow caller job name as `build`.
- Callee workflow `ci.yml` contains a job id `upload-artifacts`.
- The current jq filter requires an exact combined display name string.

### Fix Focus Areas
- .github/workflows/ci-size-report.yml[54-76]

### Suggested change
Update the jq selection to be resilient, e.g. match by suffix:
- select(.name | endswith("/ upload-artifacts") or endswith(" / upload-artifacts") or .name == "upload-artifacts")

Optionally:
- assert exactly one match and emit an error/warning if multiple matches occur
- include the matched job name in logs for easier debugging

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +17 to +20
# rebuild the base branch to get a comparison point. Only fires for
# branches nightly-build.yml's own push trigger covers (currently
# master, maintenance-9.x, maintenance-10.x, release/9.1 — see that
# file). Stale baselines for since-deleted branches aren't cleaned up

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Informational

2. Branch coverage comment wrong 🐞 Bug ⚙ Maintainability

ci-size-report.yml’s header comment claims baseline publishing only covers master,
maintenance-9.x, maintenance-10.x, and release/9.1, but nightly-build.yml also triggers
“Build pre-release” on maintenance-8.x.x, so baselines will also be published for that branch.
Agent Prompt
### Issue description
The comment describing which branches are covered by `nightly-build.yml` is inconsistent with the actual `nightly-build.yml` trigger list, which includes `maintenance-8.x.x`.

### Issue Context
This is a documentation mismatch inside the workflow file itself; it can mislead future maintainers about which branches will have baselines published.

### Fix Focus Areas
- .github/workflows/ci-size-report.yml[17-20]
- .github/workflows/nightly-build.yml[6-13]

### Suggested change
Update the comment to include `maintenance-8.x.x` (or, if unintended, remove that branch from the `nightly-build.yml` trigger list).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@sensei-hacker
sensei-hacker merged commit c7ab572 into iNavFlight:maintenance-10.x Aug 21, 2026
22 checks passed
@github-actions

Copy link
Copy Markdown

RAM / Flash usage vs. base branch — commit 0bfde9b

No size baseline is available yet for this PR's base branch (first run after this feature shipped, or a new branch). This comment will show deltas once a baseline exists.

Target Flash Δ RAM Δ
MATEKF405 660355 B (no baseline) 143640 B (no baseline)
MATEKF722 462207 B (no baseline) 125012 B (no baseline)
MATEKF765 684703 B (no baseline) 149948 B (no baseline)
MATEKH743 717127 B (no baseline) 153380 B (no baseline)

See RAM/flash optimization guide for techniques to reduce usage.

@github-actions

Copy link
Copy Markdown

Test firmware build ready — commit 0bfde9b

Download firmware for PR #11805

244 targets built. Find your board's .hex file by name on that page (e.g. MATEKF405SE.hex). Files are individually downloadable — no GitHub login required.

Development build for testing only. Use Full Chip Erase when flashing.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant