feat(stamphog): bound the whole PR by the most generous ceiling in play - #92116
Merged
Conversation
A folder that granted one size_gate key was given a fresh copy of the other key's global ceiling, which created a second budget nobody delegated: a lines-only grant let a PR touch 30 folder files plus 30 global files, and a files-only grant let it add 800 folder lines plus 800 global lines. Each ceiling now resolves on its own chain walk. A file counts against the nearest folder granting that ceiling and against the global pool when the chain grants it nowhere, so a one-key grant leaves its files in the pool they were already in for the other key. Generated-By: PostHog Desktop Task-Id: dd27894e-233a-469b-ada8-c80d13d67405
Generated-By: PostHog Desktop Task-Id: dd27894e-233a-469b-ada8-c80d13d67405
The size-gate helpers returned same-typed pairs, which the tuple-return-prefer-dataclass rule blocks: callers can swap the two scope tuples, or the two counts, without a type error. _size_budgets now returns the EffectivePolicy the scopes already live on, and the per-scope file filter returns the file list so substantive_size stays the one place that unpacks a count pair. Generated-By: PostHog Desktop Task-Id: dd27894e-233a-469b-ada8-c80d13d67405
Per-scope size budgets are checked independently and scope count is unbounded. A PR touching products/desktop/ plus anything else can reach 1000 substantive lines in desktop plus 800 in the global pool, and every folder that adopts a grant adds another full budget on top. The ~800 threshold documented in the agent README was derived from PR size, not from per-folder size, so after delegation nothing in the gate measured what that number describes. Files have the same shape. That part is pre-existing and latent, because only products/visual_review/ grants max_files today. Each ceiling now also carries a roof over the whole PR: the most generous ceiling in play for that key. It needs no new key in policy.yml, since every grant is already validated at or under the contract ceiling and the global pool is always a scope. With no grant in play the roof equals the global default, so the gate reduces to the single global total it applied before the ceilings became delegable. Per-scope budgets still hold, so the headroom a grant unlocks stays spendable only inside the folder that granted it. This does tighten the pre-existing files case. A PR with 50 visual_review files plus more elsewhere now refuses where it passes today. Claude-Session: https://claude.ai/code/session_01R6HeFa3hsShpfQM4gGWTZv
Contributor
🤖 CI report✅ Trunk lane — non-backend laneThis PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes. |
webjunkie
marked this pull request as ready for review
September 1, 2026 07:00
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
😎 Stack merged successfully - details. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Stamphog PR size gate so per-scope budgets (global + folder grants) cannot add up to an unbounded whole-PR allowance. It introduces a derived “roof” for both substantive lines and files, defined as the most generous ceiling in play across scopes, and updates tests and docs to match.
Changes:
- Add whole-PR “roof” limits for substantive lines/files, derived from the max scope ceiling for each key.
- Update size-gate refusal messaging to report whole-PR roof overruns (in addition to per-scope overruns).
- Add new policy-resolution and pipeline tests covering roof behavior, plus documentation updates.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| products/stamphog/packages/pr-approval-agent/review_pr.py | Enforces PR-wide roofs for lines/files after per-scope checks and adds corresponding refusal messages. |
| products/stamphog/packages/pr-approval-agent/policy.py | Adds line_roof/file_roof derived properties on EffectivePolicy to compute the PR-wide roofs. |
| products/stamphog/packages/pr-approval-agent/test_policy.py | Refactors size-gate setup helper and adds tests validating roof behavior and “most generous ceiling” semantics. |
| products/stamphog/packages/pr-approval-agent/README.md | Documents that per-scope budgets do not sum because a PR-wide roof still applies. |
| .stamphog/README.md | Documents the new roof concept and how it relates to delegation ceilings and global defaults. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…gate-roof # Conflicts: # .stamphog/README.md # products/stamphog/packages/pr-approval-agent/README.md # products/stamphog/packages/pr-approval-agent/policy.py # products/stamphog/packages/pr-approval-agent/review_pr.py # products/stamphog/packages/pr-approval-agent/test_policy.py
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0e00baa25
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review feedback on the roof docs. The contract-ceiling sentence read as if every PR gets that total, when a PR whose files reach no grant keeps the lower global roof. The agent README bullet ended mid-sentence. Claude-Session: https://claude.ai/code/session_01R6HeFa3hsShpfQM4gGWTZv
A repo overlay can replace size_gate while inheriting the overrides contract. That allowed a global max_lines above the contract ceiling, and the roof then followed the global pool past the ceiling the docs call the upper bound. The loader now fails closed on that pairing. It is incoherent on its own terms too: under the global default a ceiling bounds nothing, and delegation could only ever lower a folder's allowance. Claude-Session: https://claude.ai/code/session_01R6HeFa3hsShpfQM4gGWTZv
dmarticus
added a commit
that referenced
this pull request
Sep 4, 2026
…ay (#92116) Co-authored-by: Charles Vien <charles.v@posthog.com> Co-authored-by: Charles Vien <me@charlesvien.com>
dmarticus
added a commit
that referenced
this pull request
Sep 4, 2026
…ay (#92116) Co-authored-by: Charles Vien <charles.v@posthog.com> Co-authored-by: Charles Vien <me@charlesvien.com>
dmarticus
added a commit
that referenced
this pull request
Sep 4, 2026
…ay (#92116) Co-authored-by: Charles Vien <charles.v@posthog.com> Co-authored-by: Charles Vien <me@charlesvien.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Stamphog can auto-review a PR of 1800 substantive lines once #91348 lands. The size gate was calibrated to escalate past ~800.
size_gate.max_linesdelegable and checks every budget per scope. It removes the whole-PR line total.products/desktop/gets 1000 lines there, plus 800 in the global pool. Every folder that adopts a grant adds another full budget.products/visual_review/grantsmax_filestoday.Changes
Each ceiling now bounds the whole PR, at the most generous ceiling in play for that key.
products/desktop/onlyproducts/desktop/plus globalproducts/desktop/plus four more granting folderstoo large for auto-review (1394L, 49F substantive across the whole PR — roof is 1000L).policy.yml. Every grant is validated at or under the contract ceiling, and the global pool is always a scope.products/visual_review/files plus more elsewhere now refuses where it passes on master.size_gatewhile inheriting theoverridescontract, and that pairing let the global pool carry the roof past the ceiling.EffectivePolicy, two comparisons in_check_size, one loader check, and both READMEs.Nothing in the PostHog app looks different. The only surface a person reads is the stamphog refusal message quoted above.
How did you test this code?
test_size_gate_roof_bounds_the_whole_prcovers a PR where every scope fits its own budget but the total does not. No existing test had two scopes that both fit.test_roof_is_the_most_generous_ceiling_in_playcovers a roof that ignores the global pool. A folder may grant under the global default, and the roof must not follow it down._ceiling_under_global_defaultjoins the malformed-policy parametrization. It catches a contract ceiling under its own global default, which no existing case covered.pr-approval-agentsuite andproducts/stamphog/backend/tests/test_policy_defaults.pylocally, plushogli ci:preflight --strict.Automatic notifications
Docs update
.stamphog/README.mdandproducts/stamphog/packages/pr-approval-agent/README.mdare updated here.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Written by Claude Code (Opus 5) at @webjunkie's direction while reviewing #91348. The review flagged the missing whole-PR total, and this layer implements the fix it proposed. Stacking it keeps the delegation work mergeable if a reviewer rejects the roof.
Skills invoked:
/posting-pr-reviews,/merging-prs,/stacking-prs,/simplify,/writing-pr-descriptions.The roof derives from
max(scope.ceiling ...)rather than a newpolicy.ymlkey. A configured roof is a second number that can drift from theoverridescontract. The derived one cannot exceed the contract ceiling.Codex found the loader gap during review, and Copilot found two doc statements that overstated or trailed off. Those are fixed here. Copilot also flagged the em dash in the refusal messages. Rejected: the three sibling messages in the same function and the whole package use it, so changing two strings would make the gate's output inconsistent with itself.
Considered and rejected: judging the whole PR as one budget at the highest grant on any touched chain. That would let one file in a granting folder raise the ceiling for unrelated code in the same PR. Per-scope budgeting exists to prevent that.