Skip to content

Size diff comment noise - #11817

Merged
sensei-hacker merged 6 commits into
release/9.1from
size-diff-comment-noise
Aug 24, 2026
Merged

Size diff comment noise#11817
sensei-hacker merged 6 commits into
release/9.1from
size-diff-comment-noise

Conversation

@sensei-hacker

Copy link
Copy Markdown
Member

Reduces the minimum threshold

…f-comment-9.1

Fix arm-none-eabi-size not found on real CI runners
…f-comment-9.1

Point baseline publishing at Build pre-release, not the dead ci.yml p…
Increased the noise threshold from 32 to 256 bytes to filter out minor changes.
@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

@sensei-hacker
sensei-hacker merged commit 848e35c into release/9.1 Aug 24, 2026
6 checks passed
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Reduce CI size-diff comment noise by raising delta threshold

⚙️ Configuration changes ✨ Enhancement 🕐 Less than 5 minutes

Grey Divider

AI Description

• Increase size-delta noise threshold to suppress toolchain jitter in PR comments.
• Reduce false-positive ⚠️ markers for tiny RAM/flash fluctuations.
• Keep size reporting focused on meaningful regressions.
Diagram

graph TD
  CI["CI size job"] --> PRJSON[("PR size-report.json")] --> SCRIPT["size-diff-comment.js"] --> COMMENT["PR size diff comment"]
  CI --> BASEJSON[("Baseline size-report.json")] --> SCRIPT
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Percent-based threshold (relative delta)
  • ➕ Scales sensitivity with target size (e.g., 0.1% flash) across boards
  • ➕ Avoids a one-size-fits-all byte threshold
  • ➖ Harder to reason about for reviewers at-a-glance
  • ➖ May still need a small absolute floor to avoid rounding jitter
2. Per-metric/per-target thresholds
  • ➕ Can tune RAM vs flash separately and account for target-specific variance
  • ➕ Further reduces false positives on particularly noisy targets
  • ➖ More configuration and ongoing maintenance
  • ➖ Risk of inconsistency across targets

Recommendation: The PR’s approach (raising a single absolute noise threshold to 256 bytes) is the simplest effective fix for comment noise and aligns with the goal of suppressing toolchain jitter without changing the reporting model. Consider a percent-based threshold only if reviewers still see frequent false positives across different size tiers.

Files changed (1) +1 / -1

Other (1) +1 / -1
size-diff-comment.jsRaise notable-delta threshold from 32B to 256B +1/-1

Raise notable-delta threshold from 32B to 256B

• Increases NOISE_THRESHOLD_BYTES to 256 so small RAM/flash deltas are treated as noise. This reduces unnecessary ⚠️ markers and PR comment churn caused by rounding/toolchain variability.

.github/scripts/size-diff-comment.js

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

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@github-actions

Copy link
Copy Markdown

RAM / Flash usage vs. base branch — commit 64c6a3a

Target Flash Δ RAM Δ
MATEKF405 ±0 B (±0.00%) ±0 B (±0.00%)
MATEKF722 ±0 B (±0.00%) ±0 B (±0.00%)
MATEKF765 ±0 B (±0.00%) ±0 B (±0.00%)
MATEKH743 ±0 B (±0.00%) ±0 B (±0.00%)

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

@github-actions

Copy link
Copy Markdown

Test firmware build ready — commit 64c6a3a

Download firmware for PR #11817

245 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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant