lever: answer plan-first's gate question before the paths exist - #345
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_fb4dbe0c-1ece-4cd9-b6b0-4c44fa538e5c) |
2e1e2e9 to
ef94e38
Compare
22bba13 to
8c1e319
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_ef3f56f4-c4a6-4efe-b4a5-2a8a9ebbc73e) |
8c1e319 to
172f130
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_21f49277-1d5b-4ccc-a781-7505af8076d8) |
|
Tick the box to add this pull request to the merge queue (same as
|
- Problem: plan-first Step 3 asks which gates a planned change will trip, and
at what scope. Answering that by hand means reading preflight.py's path
table and every gate's argparse, which nobody does -- so the answer arrives
at publish time as a re-split or a false pass.
- make-pr's preflight.py already does the lookup, but only for a diff that
exists. This asks the same question from a list of paths you intend to
touch, so a re-split costs a line edit instead of a rebase cascade.
- Imports preflight's classify/gates_for rather than restating the unit table
(principle-bind-to-named-inventory). Two copies of that table would drift,
and the drift would be silent.
- Derives ref-awareness by reading each gate's own argparse flags, not from a
list kept in this script. A gate that gains --base starts being reported
without an edit here, which is the same reasoning that made the skill
trigger inventory generated rather than hand-written.
- Exit 1 when the plan as stated would be rejected later: mixed units, a
ref-aware gate planned without refs, or a stale base. It never edits
anything, so the exit code is the whole output contract.
- Reports unknowns unconditionally. A plan with no unknowns has not been
examined, and three of them are outside what a path list can answer.
Run against a realistic plan, the lever reported
`check_codify_has_code.py` as UNSCOPED. Reading
scripts/check_codify_has_code.py:91 confirmed `default="origin/main"`, while
preflight.py:93 invoked it with no refs -- the same vacuous-pass class already
fixed for the coverage gate, in a sibling nobody had checked.
Proved on a throwaway two-slice stack (slice A: code only; slice B: prose
only, adding a rule-shaped line):
codify --base origin/main -> exit=0 vacuous: A's code satisfied B's prose
codify --base <slice A> -> exit=1 the actual violation
Fixed in preflight.py, pinned by two colocated tests.
The lever flagged `check_skill_test_coverage` as "may pass vacuously" for
these paths. preflight.py --base plan-first duly reported `ok skill test
coverage`. Run with explicit refs:
check_skill_test_coverage.py --base plan-first --head HEAD
fail engine/skills/make-pr: changed without a corresponding test change
So this slice had changed preflight.py with no colocated test, and the gate
hid it exactly as predicted. The two tests above are that fix. (The coverage
gate's own scope fix is in flight on #309 and deliberately not duplicated
here.)
- New: scripts/plan_preflight.py (read-only; imports detectors, runs git
rev-parse/rev-list, writes nothing), tests/test_plan_preflight.py.
- Modified: engine/skills/make-pr/scripts/preflight.py passes --base to the
codify gate, so it now reports violations it previously hid on stacked
slices. Expect previously-green stacked slices to surface real gaps.
- Revertable with git revert.
- python3 -m unittest tests.test_plan_preflight -> Ran 19 tests, OK
- python3 -m unittest discover -s engine/skills/make-pr/tests -> Ran 16, OK
- python3 engine/skills/make-pr/scripts/preflight.py --base plan-first
-> ok preflight passed (single unit engine-runtime, 7 gates)
- python3 scripts/check_skill_test_coverage.py --base plan-first --head HEAD
-> ok skill test coverage (after the colocated tests; fail before them,
both outputs above)
- Two-slice codify repro: exit=0 at origin/main, exit=1 at the slice base,
both pasted above.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013sYZswUQ3HEnUEdVWrUbvq Change-Id: Ib96cec6fd81eb67ece42e6d9dfa54545dd1f0c5a
172f130 to
875e9a0
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_9363b2ae-429e-467b-8333-c54694177533) |
Summary
plan-firststep 3 asks which gates a planned change will trip and at what scope. Answering by hand means reading preflight's path table plus every gate's argparse, which nobody does — so the answer arrives at publish time as a re-split or a false pass.This answers it from a list of paths you intend to touch, before they exist. It reuses preflight's own classification rather than restating it, and derives which gates are scope-sensitive by reading their argparse flags.
On its first real run it found a live instance of the defect it was built to detect.
Review Claim
The planning question "what gates will these paths trip, and at what scope" should be answered by a script that reuses preflight's table, not by hand.
Review Lane
behaviorReview Unit
engine-runtimeSafety Invariant
The new script is read-only: it imports detectors, runs
git rev-parse/rev-list, and writes nothing. Its exit code is its whole contract. The one change to existing behavior is thatpreflight.pynow passes--baseto the codify gate, so it reports violations it previously hid on stacked slices — strictly more reporting, never less.Slice Rationale
Separate from the playbook it serves because that is a
product-skillclaim and this isengine-runtime; they cannot mix. Running the playbook on itself produced this split, which is the cheapest possible demonstration that it works.Detail: the defect it found on its first run
The lever reported
check_codify_has_code.pyas UNSCOPED. Readingscripts/check_codify_has_code.py:91confirmeddefault="origin/main", whilepreflight.py:93invoked it with no refs — the same vacuous-pass class already fixed for the coverage gate, sitting in a sibling nobody had checked.Proved on a throwaway two-slice stack (slice A: code only; slice B: prose only, adding a rule-shaped line):
Detail: then it predicted a defect in this slice
The lever flagged
check_skill_test_coverageas "may pass vacuously" for these paths.preflight.py --base plan-firstduly reportedok skill test coverage. With explicit refs:This slice had changed
preflight.pywith no colocated test, and the gate hid it exactly as predicted. The two newtest_codify_gate_*cases are that fix. The coverage gate's own scope fix is in flight on #309 and deliberately not duplicated here.Non-goals
preflight.pyruns them.--baseis reported without editing this script.Test Plan
Test Plan
python3 -m unittest tests.test_plan_preflight→Ran 19 tests ... OKpython3 -m unittest discover -s engine/skills/make-pr/tests→Ran 16 tests ... OKpython3 engine/skills/make-pr/scripts/preflight.py --base plan-first→ok preflight passed(single unitengine-runtime, 7 gates)Coverage gate, fail-before / pass-after at the real slice scope:
Two-slice codify repro output is in the Detail section above.
Revert Plan
Revert Plan
git revert <sha>🤖 Generated with Claude Code
https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
Note
Low Risk
Developer tooling only: stricter preflight reporting for rule prose; no runtime or production behavior change beyond catching more gate violations.
Overview
Adds
scripts/plan_preflight.py, a read-only planner that takes intended--pathsand reports plan-first slices (via importedpreflight.classify), gates with scope labels (scoped / whole-tree / “may pass vacuously”), and base freshness against remote. It derives ref-awareness by scanning each gate script for--base/--headargparse flags and exits 1 on mixed units, unscoped ref-aware gates, or a stale base.preflight.pynow passes--basetocheck_codify_has_code.pywhenever a real slice base exists—the same vacuous-pass class already handled for skill test coverage—so stacked prose-only slices cannot be green-lit by a sibling’s code onorigin/main.Tests cover the new planner (slices, ref derivation, CLI/JSON) and regress
gates_forcodify command wiring with and withoutbase.Reviewed by Cursor Bugbot for commit 875e9a0. Bugbot is set up for automated code reviews on this repo. Configure here.