BabylonJS / BabylonJS/Babylon-Lite

Bundle-size PR comment goes stale when a PR stops moving bytes

Open
#627 0 comments 0 reactions 1 assignee Claimed by @RaananW View on GitHub
Dominant language
TypeScript
Stars
149
Forks
29
Avg merge
1d 3h
Merged PRs (30d)
113

Description

Filed as a follow-up from #530, which surfaces ceiling headroom in the bundle-size PR comment. This is **pre-existing behaviour of the posting path**, not introduced by that PR, and fixing it needs a change to the pipeline's posting step rather than to the reporter — so it was deliberately kept out of a change scoped to reporting only.

**Repro**
- Open a PR that grows a scene enough to trip the bundle-size comment gate. CI posts a `## Bundle Size Changes` comment.
- Push a commit that removes the growth, so the PR now moves zero bundle bytes.
- Let CI run again.

- **Expected result:** the previously-posted comment is updated (or retracted) to reflect that the PR no longer moves any bundle bytes.
- **Current result:** the original comment remains on the PR verbatim and indefinitely. It asserts a present state it no longer measures.

**Why it happens**

`scripts/report-bundle-size-deltas.ts` gates on three disjuncts — a whole-KB delta, a scene this PR moved into the tight band, or an inherited ceiling breach. When none fires it sets `POST_BUNDLE_COMMENT=false` and returns, and the pipeline skips the posting step entirely.

That gate is correct and should stay: it is what keeps the comment off the many PRs that move no bytes. The gap is that *not posting* and *clearing a previous post* are the same action today, so silence leaves whatever was posted earlier standing.

**Live instance**

PR #530 currently displays the comment posted at `2026-08-25T22:59:16Z` by build 58448 on `826ab18a` — 16,529 characters, 94 rows in the "scenes this PR moved" block. Every one of those rows describes a scene that *shrank*; the block was warning about ceiling pressure using a sign-blind predicate. All of that was fixed later in the same PR, but the comment predates the fix and has never been refreshed, because the branch now moves zero bytes and the gate correctly declines to post.

The result is that the most visible artifact on that PR demonstrates the bugs rather than the fix. A reviewer opening it would reasonably conclude the feature is broken.

**Suggested fix**

**Corrected after inspecting the pipeline — this is structural, not a missing gate branch.**

My original suggestion here was to give the posting step an "update if present" mode. No such mode exists. `azure-pipelines.yml` posts via **`GitHubComment@0`**, whose only inputs are `gitHubConnection`, `repositoryName`, `id` and `comment` — it is **create-only**, with no marker lookup, no edit and no upsert.

So there is no `else` branch to add to the gate. Even a gate that fired correctly on *"something was said before, nothing to say now"* could only **append a second comment**, leaving the stale one above it — arguably worse than the current behaviour, since the PR would then show two bundle-size comments disagreeing with each other.

The actual fix is to **replace the task with the sticky-comment pattern**: embed a hidden marker in the body, look up the prior bot comment by that marker, and `PATCH` it when present rather than `POST`ing a new one. That makes every run idempotent and removes the whole class — the comment then reflects the build that last ran, not the last build that happened to have something to say.

That is unambiguously a change to posting behaviour rather than to report content, which is why it is filed here rather than folded into #530.

**Provenance:** the task was introduced by `a706c896` (2026-05-11, *"Add bundle size delta PR comment (#88)"*), so this predates both #526 and #530 and is repo-wide rather than owned by either. Note `azure-pipelines.yml` contains two `GitHubComment@0` tasks — the API-report one has the same limitation and may be worth fixing together.

Worth noting the failure mode is the same shape as a stale instrument read: the comment's content is a snapshot whose freshness is a property of when it was written, not of the field it renders. It is arguably worse than silence, because stale content asserting a current state reads exactly like fresh content.

**Screenshots**
N/A

**Desktop / Smartphone**
N/A — this is CI reporting infrastructure, not runtime behaviour.

**Additional context**
- Reporter: `scripts/report-bundle-size-deltas.ts` (gate at the end of `main()`).
- Posting step: `azure-pipelines.yml`, `Post bundle-size changes to PR`, conditioned on `POST_BUNDLE_COMMENT`.
- No ceiling values (`maxRawKB` in `scene-config.json`) are involved in or affected by this issue.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.