Conversation
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. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| **A budget breach blocks — at any commit, first or fiftieth.** The gate | ||
| measures the cumulative diff against the caps and the file-set against the | ||
| allowlist at PR open and every push. Budgets are contracts, not suggestions: | ||
| disclosure-and-continue is a violation, not a state, and numbers in a PR body | ||
| are not measurements — they go stale; the diff is the only witness. |
There was a problem hiding this comment.
🔍 Budget gate needs verification
The playbook promises automatic budget and allowlist checks on every push. No repository workflow or script provides them, leaving enforcement dependent on undocumented infrastructure.
Was this helpful? React with 👍 or 👎 to provide feedback.
![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: dbd9a6507b
ℹ️ 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".
| - Time is a budget alongside size: a slice should reach its human checkpoint within ~90 minutes of starting (implementation + bot gate + CI). If its PR sits ~2 hours without merging, blocking, or reaching a checkpoint, the slice is stalled — report status and either re-plan or block; never leave a PR sitting idle. Waiting at the human checkpoint does not count against the slice. | ||
| - Every slice carries a **merge contract** in its task/issue description: one line naming what the PR may and may not touch (e.g. "additive dead code, no call-site changes"), plus separate prod and test line budgets (the ~300-per-PR limit is prod lines; tests ride their slice under their own cap). If the diff exceeds the budget or starts mixing phases, stop and report the overrun — in Space-managed work set the task to `blocked`; otherwise flag it in the PR — budgets are contracts, not suggestions. | ||
| - Reuse existing pipelines/gates where they fit; do not rebuild routing or decision logic a sibling already owns. | ||
| A task with an incomplete contract is not dispatchable; no task, no PR. |
There was a problem hiding this comment.
When an agent decomposes work, the repository directs it to load CLAUDE.md, but this assertion does not identify what makes a contract complete or direct the agent to docs/decomposition-playbook.md, where the required budgets, allowlist, rung, split trigger, ladder, and stop rules now live. Consequently, an agent following the repository entry-point guidance can dispatch a task that violates the newly declared binding contract; retain a direct instruction here to read the playbook before decomposition.
AGENTS.md reference: AGENTS.md:L3-L3
Useful? React with 👍 / 👎.
| CLAUDE.md carries the tenets as the always-loaded summary; this file is the | ||
| manual; the gate — cumulative diff vs caps and file-set vs allowlist, | ||
| checked at PR open and every push — is the enforcement that makes both | ||
| credible. |
There was a problem hiding this comment.
For every PR opened or pushed under this policy, the promised budget and file-allowlist enforcement does not run: inspection of .github/workflows/main.yml, the other workflow/action files, package.json, and a repo-wide search of scripts/ found no check that reads task caps or a per-task file allowlist; PR and push CI only invokes the existing make check chain. As written, an over-budget or out-of-allowlist diff can pass all repository checks despite the document saying the gate blocks it, so add the gate to those events or describe this enforcement as unavailable until it exists.
Useful? React with 👍 / 👎.
| - **Budgets with receipts** — separate prod and test caps, each from a count | ||
| *(tenet 1)*. |
There was a problem hiding this comment.
When the task is an Extract slice, this required-contract checklist can be satisfied with only a net production cap even though lines 35–36 separately require gross moves to be capped. A move of thousands of lines can therefore report a near-zero net budget and still have a formally “complete” contract, leaving the promised gate with no gross threshold to enforce and defeating the review-size constraint; make the separate gross-move cap a required contract field for Extract slices.
Useful? React with 👍 / 👎.
…d; add bricks rule
…ge test suites as separate slices
…ion research Closes the gap surfaced by the 2026-08-29 fleet retrospective (108 PRs, 8 lanes, 13 split chains, 23 in-budget controls): the playbook file now exists on dev and is the law; CLAUDE.md carries the eight tenets as the always-loaded summary. What changed: - The slice ladder, bricks rule, and measure-first stay. - New: the task contract (budgets with receipts, file allowlist, named bricks, pre-declared split trigger, one deliverable). A task with an incomplete contract is not dispatchable; no task, no PR. - New: stop rules with ruling menu. Budget breach blocks at any commit; review growth trips a breaker (machinery-adding finding, 3rd non-convergent cycle with owner re-invokes counted, or >30% growth); menu is bounded absorb / spin to slice / re-cut / kill, and only the owner rules. Peer self-adjudication is removed. - New: deferrals create priced successors at deferral time. - New: re-cuts obey the axis law. Fresh measurement plus new boundary axis; a second same-axis re-cut forces epic re-decomposition. - New: ops failures (restart run loss, never-spawned dispatch) are not decomposition events; never read zombie churn as re-cut data. - CLAUDE.md: the long pointer paragraph is replaced with the eight tenets block (essence only, no duplication of the manual). Numbers and constants are tunable and stated once in the ruling menu: ~300 prod cap, ~10 prod / ~18 test per entry, ~105-140 harness lines, <=~100 line bounded absorb, 3-cycle trigger, 30% growth threshold. CLAUDE.md no longer contains a 'read docs/decomposition-playbook.md' line. The file is the source of truth, not a pointer from CLAUDE.md. Reference implementation: external-events delivery redesign (#3013-3027) and capability-dispatcher measured re-cuts (CD 08). Supersedes the original #3229 branch intent (file extraction only); PR #3229 was closed before this commit.
dbd9a65 to
f735a00
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f735a006e6
ℹ️ 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".
| A large ported or characterization test suite (>~300 test lines) may be | ||
| its own test-only slice riding after the wiring. |
There was a problem hiding this comment.
When a characterization suite exceeds roughly 300 lines, this exception explicitly schedules it after the wiring PR, even though characterization tests are supposed to pin existing behavior before that behavior is rewired. This allows the integration change to merge without the regression coverage needed to establish equivalence; split an oversized characterization suite into multiple pre-wiring pin slices instead.
Useful? React with 👍 / 👎.
| - Time is a budget: ~90 minutes to the human checkpoint; a PR idle ~2 hours | ||
| is stalled — report and either re-plan or block. |
There was a problem hiding this comment.
When a PR is awaiting human review, the preceding no-polling rule correctly tells the agent to go idle, but this unconditional two-hour rule then classifies that same wait as a stall and directs the agent to re-plan or block. Restore the human-checkpoint exception so normal reviewer latency does not create false blocked tasks or unnecessary replanning.
Useful? React with 👍 / 👎.
1 participant
Composes the original #3229 branch content with the 2026-08-29 fleet retrospective (108 PRs, 8 lanes, 13 split chains, 23 in-budget controls).
What changed vs the original #3229 branch:
Numbers and constants are tunable and stated once in the ruling menu: ~300 prod cap, ~10 prod / ~18 test per entry, ~105-140 harness lines, <=~100 line bounded absorb, 3-cycle trigger, 30% growth threshold.
CLAUDE.md no longer contains a "read docs/decomposition-playbook.md" line. The file is the source of truth, not a pointer from CLAUDE.md.
Reference implementation: external-events delivery redesign (#3013-3027) and capability-dispatcher measured re-cuts (CD 08).
The original PR #3229 was closed before this commit (file extraction only, no rule additions beyond bricks).