You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Corrected after investigation. The original text of this issue claimed the budget is unenforced and proposed adding a test. That was wrong: tests/test_agent_prompt.py:31 already defines PROMPT_BYTE_BUDGET = 62_000 and asserts against it. It was missed by a grep 62000 — the constant is written 62_000. The real gap is different and is described below.
The gap: green checks that describe a merge which no longer exists
.github/workflows/ci.yml uses actions/checkout@v5 with no ref:, so on a pull_request event it checks out the merge commit. The budget test therefore does measure the merged result — it is the right test, wired the right way. But a merge commit is only recomputed when the PR is updated. A PR that sits while main moves keeps reporting a green result for a merge that no longer reflects reality.
CI ran exactly twice on that branch: once on the original commit, once when the fix was pushed. In between, main grew by 496 B. The over-budget merge was caught by hand with a git merge-tree simulation during review; no automation would have caught it, even though the test exists and works.
This generalises past this one file: any invariant that depends on both sides of a merge (the licence-declaration test from #577 has the same shape) is only as fresh as the last CI run on that branch.
Options
Require branches to be up to date before merging (repo ruleset). Closes the hole exactly. Cost: every PR needs a rebase/update when main moves, which is real friction on a repo with this merge rate.
Merge queue. Same guarantee without the manual rebase, at the cost of setup and slower merges.
Accept and document. Keep it a reviewer responsibility, but say so explicitly in CONTRIBUTING.md so it is a known check rather than folklore — and note that a stale green check is not evidence about the merge.
Option 1 is the honest fix; option 3 is the cheap one. Worth a deliberate decision rather than drifting into 3 by default.
Two smaller things found alongside
CONTRIBUTING.md says "60 KB" in three places (lines 381, 464, 594) while the enforced budget is 62 000 B. The test's own comment records the bump from 60 kB to 62 kB in v0.48.0; the prose was never updated. Worth fixing so the number in the checklist matches the number that fails the build.
The gap: green checks that describe a merge which no longer exists
.github/workflows/ci.ymlusesactions/checkout@v5with noref:, so on apull_requestevent it checks out the merge commit. The budget test therefore does measure the merged result — it is the right test, wired the right way. But a merge commit is only recomputed when the PR is updated. A PR that sits whilemainmoves keeps reporting a green result for a merge that no longer reflects reality.What happened on PR #580
mainkeboola-expert.mdmerged with #58073ded7a(61494 B)7445d76CI ran exactly twice on that branch: once on the original commit, once when the fix was pushed. In between,
maingrew by 496 B. The over-budget merge was caught by hand with agit merge-treesimulation during review; no automation would have caught it, even though the test exists and works.This generalises past this one file: any invariant that depends on both sides of a merge (the licence-declaration test from #577 has the same shape) is only as fresh as the last CI run on that branch.
Options
mainmoves, which is real friction on a repo with this merge rate.Option 1 is the honest fix; option 3 is the cheap one. Worth a deliberate decision rather than drifting into 3 by default.
Two smaller things found alongside