From 338e9f2126e4ccd96b01de335f7444cdc1112f31 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 01:53:57 +0000 Subject: [PATCH 1/3] Add a mandatory five-row Impact block to PRs and reviews MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every Direct-lane PR and every review fills Process, Dependencies, Context cost, Behaviour and Runtime. Context cost is pasted from bin/render-routing.sh --check on base and head. RELEASE-PROCESS.md §4a holds the reviewer rule, the A/B requirement tiers and a bad/good example; CONTRIBUTING.md's reviewer list points there. No CI check: check-pr-discipline.sh reads the git diff, not the PR body. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VJgWP5vEoAsNsJYqCDMGNw --- .github/PULL_REQUEST_TEMPLATE.md | 11 +++++++++++ CHANGELOG.md | 1 + CONTRIBUTING.md | 2 ++ docs/RELEASE-PROCESS.md | 27 +++++++++++++++++++++++++++ 4 files changed, 41 insertions(+) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 8e05361c..9f35c428 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -10,6 +10,17 @@ Nothing to fill in — the field-evidence line inside the file is the whole bar. **What changed and why (one paragraph):** +**Impact** — all five rows, every PR. "none" or "NOT RUN: " is legal; a blank row is +not. Reviewers check each row against the diff (`docs/RELEASE-PROCESS.md` §4a). + +| Row | Answer | +|---|---| +| **Process** — human steps or gates added/removed; does anything new block; new stage artifacts | | +| **Dependencies** — shared files touched (`_common.sh`, `exec.sh`, `skill-routing.tsv`, …); order vs other open PRs; new tools/platform assumptions; new artifact reads + their producer | | +| **Context cost** — always-loaded words per role at the affected stage(s), base → head, and baseline total vs 80,000, pasted from `bin/render-routing.sh --check` on both | | +| **Behaviour** — A/B result for agent-read text (`process/preflight-skill-baseline-2026-08-31.md`), or "NOT RUN: ", or "n/a: no agent-read text" | | +| **Runtime** — seconds/resources added or saved per exec, gate or check: measured, or "n/a" | | + **Field evidence** — which real project this ran against / was observed in, and what it measured or fixed there (CLAUDE.md → "Shipping an instrument", rule 4; required for any instrument change): diff --git a/CHANGELOG.md b/CHANGELOG.md index 649e7cbf..f6706976 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ Sections dated before 2026-09-19 predate the cycle and stay as they are. Named m ## Unreleased +- process(pr-template, release-process): **every Direct-lane PR and every review fills a five-row Impact block: Process, Dependencies, Context cost, Behaviour, Runtime.** "none" or "NOT RUN: " is legal, a blank row is not. Context cost is pasted from `bin/render-routing.sh --check` on base and head, never estimated; a missing or estimated row is a review finding. The Behaviour A/B (`process/preflight-skill-baseline-2026-08-31.md`) is required for baseline-routed text and agent stubs, recommended for situational skills, n/a for scripts. `docs/RELEASE-PROCESS.md` §4a holds the rule and a bad/good example; `CONTRIBUTING.md`'s reviewer list points there. No CI check: `bin/check-pr-discipline.sh` reads the git diff, not the PR body. Field evidence: three PRs reviewed on 2026-10-15 with no process cost stated (#375 added a human step per re-baseline, #384 adds 1–11 s per exec), both found by chance. — maintainer request, MendixMau - learn(learned-detection-gaps, testing-shape, ui-loop): **a class on a snippet call never reaches the DOM; a 403 bounce and a widget-pinned session probe both read as sign-outs; app restarts matched a "signs me out" report by timing.** Detection-gaps row: `Class` on a `SNIPPETCALL` is green through check, exec, mxbuild, lint and DESCRIBE, and absent in the browser ([#336](https://github.com/MendixMau/mxcli-project-toolkit/issues/336)). Testing-shape §4 gains two false-red rows: `login.html?messageCode=403` with the session kept ([#338](https://github.com/MendixMau/mxcli-project-toolkit/issues/338)), and instruments pinned to one grid name or a typed menu list (1/7 and 7/34 false FAILs after a redesign, [#339](https://github.com/MendixMau/mxcli-project-toolkit/issues/339)). ui-loop: say so before a restart while a human uses the app ([#337](https://github.com/MendixMau/mxcli-project-toolkit/issues/337)) — three-app field run, 2026-10-14 - log(bug-logs): **upstream draft for `mxcli run --local --watch`: a theme-only rebuild deletes `dist/pages` and still reports a successful reload** ([#284](https://github.com/MendixMau/mxcli-project-toolkit/issues/284)). `bug-logs/pending-github-issues/watch-theme-rebuild-drops-dist-pages.md`, status DRAFT — NOT FILED: the repro (one SCSS edit, every page 404s on `dist/pages/.js`, a restart restores all 135 bundles; twice in one app on mxcli v0.26.0 / Mendix 11.15.0), the other change classes watch serves stale, the timing table (reload 35 builds median 3.8 s, restart 44 builds median 19.2 s, max 55.2 s, 928 s total) and the upstream questions: why a restart is about 5× a reload, and whether a recreated microflow keeps its old compiled version by name. The toolkit side is already on master (exec.sh restart verdicts, `app-restart.sh`, ui-loop.md "What watch picks up"). Field evidence: the 2026-10-12 three-app UX run and its 2026-10-14 recurrence, as recorded in #284. — field retro #284, MendixMau - fix(close-task): **`close-task.sh` resolves `checkpoint.sh` instead of printing it bare.** The script ships at `claude-hooks/bin/checkpoint.sh` and reaches `~/.claude/bin` only via the opt-in `bin/install-claude-hooks.sh --basic`, so a project without the hooks got a `checkpoint.sh --write` step it could not run. `close-task.sh` now looks in PATH, then `~/.claude/bin`, then `$MXTK_ROOT/claude-hooks/bin`, and prints the full path it found, single-quoted so a path with spaces survives copy-paste. If none is found it prints `checkpoint.sh not installed — run /bin/install-claude-hooks.sh --basic, or skip: RESUME.md is the resume source` and carries on; the exit code still means only "writes owed". `close-the-loop.md` step 4 says the same. The `~/.claude/bin` copy (`claude-hooks/bin/close-task.sh`) now prints its sibling `checkpoint.sh` by full quoted path, because `~/.claude/bin` is usually not on PATH. Fixture `tests/wave2/test-close-task-checkpoint.sh` (14 cases, including a space and `'` in HOME and the toolkit path; 11 fail on master). Field evidence: the reporter's existing-app run, where the closing agent had to skip the step with no pointer. — joshuamoesa (#358) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ac0d1cdc..33f51538 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -79,6 +79,8 @@ follow `README.md` → "How to add a new skill" including the routing-table row. `CLAUDE.md` → "Shipping an instrument — field-proof before merge". 3. A `CHANGELOG.md` line is present and credits the source. 4. Scoped fixtures for the touched files were run, and named in the PR. + The PR's Impact block has all five rows filled and each matches the diff + (`docs/RELEASE-PROCESS.md` §4a); a missing or estimated Context cost row is a finding. 5. Squash by default; a merge commit only for a branch whose every commit is already clean — see below. 6. A name found in a PR is rewritten by hand, in the PR, with a placeholder (`ClientX`) — diff --git a/docs/RELEASE-PROCESS.md b/docs/RELEASE-PROCESS.md index ab9d6fd6..5b19e39c 100644 --- a/docs/RELEASE-PROCESS.md +++ b/docs/RELEASE-PROCESS.md @@ -87,6 +87,33 @@ eight moves. A `ready` PR with no activity for 14 days gets `verdict/promote-to- `CHANGELOG.md`. Over the cap: split the PR or stack it (each layer green on its own, split plan stated in the PR body before work starts). `size/*` labels this from the diff. +## 4a. The Impact block — what a change costs, stated before review + +Every Direct-lane PR fills the five Impact rows in `.github/PULL_REQUEST_TEMPLATE.md` +(Process, Dependencies, Context cost, Behaviour, Runtime), and every review re-checks them. +Why: on 2026-10-15 three PRs were reviewed without anyone stating their process cost; #375 +added a human step per re-baseline and #384 added 1–11 s per exec, and both were found only by +chance. + +The reviewer, for each of the 5 rows, reads the diff and confirms or corrects the answer: + +1. **Process** — every new human step, gate, blocking path or stage artifact in the diff is named. +2. **Dependencies** — every shared file in the diff is listed; every new artifact read names + the mandatory step that produces it (CLAUDE.md → "Shipping an instrument", rule 5). +3. **Context cost** — the numbers are pasted from `bin/render-routing.sh --check` run on base + and on head. A missing row, or an estimate instead of the pasted output, is a review finding. +4. **Behaviour** — an A/B per `process/preflight-skill-baseline-2026-08-31.md` is **required** + for a change to always-loaded / baseline-routed text or an agent stub, **recommended** for + situational skill text, and **n/a** for scripts. +5. **Runtime** — measured seconds per exec/gate/check, or "n/a" with nothing on a hot path. + +The review ends with 5 of 5 rows confirmed; "NOT RUN: " counts, a blank does not. + +| | Context cost row | +|---|---| +| ✗ bad | "Small — a few lines in one skill." | +| ✓ good | "`learned-mdl-preflight.md` +42 words; mdl at Stage 5 30,185 → 30,227; baseline 73,522 → 73,564 (budget 80,000). `render-routing.sh --check` on base and head." | + ## 5. Model tiers, by purpose Described by what each tier is for, not by vendor or model name — a session on any harness From ff47b94503c772186ea7082e14b47af1ffd969b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 01:58:23 +0000 Subject: [PATCH 2/3] =?UTF-8?q?process(impact-block):=20review=20fixes=20?= =?UTF-8?q?=E2=80=94=20Context=20"none"=20when=20no=20routed=20file,=20doc?= =?UTF-8?q?s=20read=20by=20agents=20are=20not=20n/a?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Independent review of #388: MERGE with four wording fixes, applied here. - Context cost may be "none: no routed or always-loaded file touched"; paste render-routing output only when a routed file changes. - Behaviour: process docs and templates read by review agents are agent-read text -> NOT RUN with a reason, never n/a; n/a is for scripts only. - CONTRIBUTING: the Impact check is its own numbered merge criterion. - Template says "every Direct-lane PR", matching the CHANGELOG line. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VJgWP5vEoAsNsJYqCDMGNw --- .github/PULL_REQUEST_TEMPLATE.md | 6 +++--- CHANGELOG.md | 2 +- CONTRIBUTING.md | 6 +++--- docs/RELEASE-PROCESS.md | 9 ++++++--- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 9f35c428..4133a4f8 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -10,15 +10,15 @@ Nothing to fill in — the field-evidence line inside the file is the whole bar. **What changed and why (one paragraph):** -**Impact** — all five rows, every PR. "none" or "NOT RUN: " is legal; a blank row is +**Impact** — all five rows, every Direct-lane PR. "none" or "NOT RUN: " is legal; a blank row is not. Reviewers check each row against the diff (`docs/RELEASE-PROCESS.md` §4a). | Row | Answer | |---|---| | **Process** — human steps or gates added/removed; does anything new block; new stage artifacts | | | **Dependencies** — shared files touched (`_common.sh`, `exec.sh`, `skill-routing.tsv`, …); order vs other open PRs; new tools/platform assumptions; new artifact reads + their producer | | -| **Context cost** — always-loaded words per role at the affected stage(s), base → head, and baseline total vs 80,000, pasted from `bin/render-routing.sh --check` on both | | -| **Behaviour** — A/B result for agent-read text (`process/preflight-skill-baseline-2026-08-31.md`), or "NOT RUN: ", or "n/a: no agent-read text" | | +| **Context cost** — always-loaded words per role at the affected stage(s), base → head, and baseline total vs 80,000, pasted from `bin/render-routing.sh --check` on both — or "none: no routed or always-loaded file touched" | | +| **Behaviour** — A/B result for agent-read text (`process/preflight-skill-baseline-2026-08-31.md`), or "NOT RUN: ", or "n/a: scripts only" (process docs that review agents read are agent-read text) | | | **Runtime** — seconds/resources added or saved per exec, gate or check: measured, or "n/a" | | **Field evidence** — which real project this ran against / was observed in, and what it measured diff --git a/CHANGELOG.md b/CHANGELOG.md index f6706976..08db3eda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ Sections dated before 2026-09-19 predate the cycle and stay as they are. Named m ## Unreleased -- process(pr-template, release-process): **every Direct-lane PR and every review fills a five-row Impact block: Process, Dependencies, Context cost, Behaviour, Runtime.** "none" or "NOT RUN: " is legal, a blank row is not. Context cost is pasted from `bin/render-routing.sh --check` on base and head, never estimated; a missing or estimated row is a review finding. The Behaviour A/B (`process/preflight-skill-baseline-2026-08-31.md`) is required for baseline-routed text and agent stubs, recommended for situational skills, n/a for scripts. `docs/RELEASE-PROCESS.md` §4a holds the rule and a bad/good example; `CONTRIBUTING.md`'s reviewer list points there. No CI check: `bin/check-pr-discipline.sh` reads the git diff, not the PR body. Field evidence: three PRs reviewed on 2026-10-15 with no process cost stated (#375 added a human step per re-baseline, #384 adds 1–11 s per exec), both found by chance. — maintainer request, MendixMau +- process(pr-template, release-process): **every Direct-lane PR and every review fills a five-row Impact block: Process, Dependencies, Context cost, Behaviour, Runtime.** "none" or "NOT RUN: " is legal, a blank row is not. Context cost is pasted from `bin/render-routing.sh --check` on base and head, never estimated; a missing or estimated row is a review finding. The Behaviour A/B (`process/preflight-skill-baseline-2026-08-31.md`) is required for baseline-routed text and agent stubs, recommended for situational skills, NOT RUN with a reason for process docs review agents read, n/a for scripts only; Context cost may be "none" when no routed or always-loaded file is touched. `docs/RELEASE-PROCESS.md` §4a holds the rule and a bad/good example; `CONTRIBUTING.md`'s reviewer list points there. No CI check: `bin/check-pr-discipline.sh` reads the git diff, not the PR body. Field evidence: three PRs reviewed on 2026-10-15 with no process cost stated (#375 added a human step per re-baseline, #384 adds 1–11 s per exec), both found by chance. — maintainer request, MendixMau - learn(learned-detection-gaps, testing-shape, ui-loop): **a class on a snippet call never reaches the DOM; a 403 bounce and a widget-pinned session probe both read as sign-outs; app restarts matched a "signs me out" report by timing.** Detection-gaps row: `Class` on a `SNIPPETCALL` is green through check, exec, mxbuild, lint and DESCRIBE, and absent in the browser ([#336](https://github.com/MendixMau/mxcli-project-toolkit/issues/336)). Testing-shape §4 gains two false-red rows: `login.html?messageCode=403` with the session kept ([#338](https://github.com/MendixMau/mxcli-project-toolkit/issues/338)), and instruments pinned to one grid name or a typed menu list (1/7 and 7/34 false FAILs after a redesign, [#339](https://github.com/MendixMau/mxcli-project-toolkit/issues/339)). ui-loop: say so before a restart while a human uses the app ([#337](https://github.com/MendixMau/mxcli-project-toolkit/issues/337)) — three-app field run, 2026-10-14 - log(bug-logs): **upstream draft for `mxcli run --local --watch`: a theme-only rebuild deletes `dist/pages` and still reports a successful reload** ([#284](https://github.com/MendixMau/mxcli-project-toolkit/issues/284)). `bug-logs/pending-github-issues/watch-theme-rebuild-drops-dist-pages.md`, status DRAFT — NOT FILED: the repro (one SCSS edit, every page 404s on `dist/pages/.js`, a restart restores all 135 bundles; twice in one app on mxcli v0.26.0 / Mendix 11.15.0), the other change classes watch serves stale, the timing table (reload 35 builds median 3.8 s, restart 44 builds median 19.2 s, max 55.2 s, 928 s total) and the upstream questions: why a restart is about 5× a reload, and whether a recreated microflow keeps its old compiled version by name. The toolkit side is already on master (exec.sh restart verdicts, `app-restart.sh`, ui-loop.md "What watch picks up"). Field evidence: the 2026-10-12 three-app UX run and its 2026-10-14 recurrence, as recorded in #284. — field retro #284, MendixMau - fix(close-task): **`close-task.sh` resolves `checkpoint.sh` instead of printing it bare.** The script ships at `claude-hooks/bin/checkpoint.sh` and reaches `~/.claude/bin` only via the opt-in `bin/install-claude-hooks.sh --basic`, so a project without the hooks got a `checkpoint.sh --write` step it could not run. `close-task.sh` now looks in PATH, then `~/.claude/bin`, then `$MXTK_ROOT/claude-hooks/bin`, and prints the full path it found, single-quoted so a path with spaces survives copy-paste. If none is found it prints `checkpoint.sh not installed — run /bin/install-claude-hooks.sh --basic, or skip: RESUME.md is the resume source` and carries on; the exit code still means only "writes owed". `close-the-loop.md` step 4 says the same. The `~/.claude/bin` copy (`claude-hooks/bin/close-task.sh`) now prints its sibling `checkpoint.sh` by full quoted path, because `~/.claude/bin` is usually not on PATH. Fixture `tests/wave2/test-close-task-checkpoint.sh` (14 cases, including a space and `'` in HOME and the toolkit path; 11 fail on master). Field evidence: the reporter's existing-app run, where the closing agent had to skip the step with no pointer. — joshuamoesa (#358) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 33f51538..bff3c817 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -79,11 +79,11 @@ follow `README.md` → "How to add a new skill" including the routing-table row. `CLAUDE.md` → "Shipping an instrument — field-proof before merge". 3. A `CHANGELOG.md` line is present and credits the source. 4. Scoped fixtures for the touched files were run, and named in the PR. - The PR's Impact block has all five rows filled and each matches the diff +5. The PR's Impact block has all five rows filled and each matches the diff (`docs/RELEASE-PROCESS.md` §4a); a missing or estimated Context cost row is a finding. -5. Squash by default; a merge commit only for a branch whose every commit is already clean — +6. Squash by default; a merge commit only for a branch whose every commit is already clean — see below. -6. A name found in a PR is rewritten by hand, in the PR, with a placeholder (`ClientX`) — +7. A name found in a PR is rewritten by hand, in the PR, with a placeholder (`ClientX`) — never auto-replaced. Auto-replace breaks paths, fixtures and credit lines, and the real name stays recoverable from history either way. diff --git a/docs/RELEASE-PROCESS.md b/docs/RELEASE-PROCESS.md index 5b19e39c..e6b5cf09 100644 --- a/docs/RELEASE-PROCESS.md +++ b/docs/RELEASE-PROCESS.md @@ -100,11 +100,14 @@ The reviewer, for each of the 5 rows, reads the diff and confirms or corrects th 1. **Process** — every new human step, gate, blocking path or stage artifact in the diff is named. 2. **Dependencies** — every shared file in the diff is listed; every new artifact read names the mandatory step that produces it (CLAUDE.md → "Shipping an instrument", rule 5). -3. **Context cost** — the numbers are pasted from `bin/render-routing.sh --check` run on base - and on head. A missing row, or an estimate instead of the pasted output, is a review finding. +3. **Context cost** — when any routed or always-loaded file changes, the numbers are pasted from + `bin/render-routing.sh --check` run on base and on head; otherwise "none: no routed or + always-loaded file touched" is the answer. A missing row, or an estimate instead of the pasted + output, is a review finding. 4. **Behaviour** — an A/B per `process/preflight-skill-baseline-2026-08-31.md` is **required** for a change to always-loaded / baseline-routed text or an agent stub, **recommended** for - situational skill text, and **n/a** for scripts. + situational skill text, **NOT RUN with a reason** for process docs and templates that review + agents read (they are agent-read text, so never "n/a"), and **n/a** for scripts only. 5. **Runtime** — measured seconds per exec/gate/check, or "n/a" with nothing on a hot path. The review ends with 5 of 5 rows confirmed; "NOT RUN: " counts, a blank does not. From e1f116fa68e3db04c32cf8c6cbfd1a66579d00eb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 02:05:34 +0000 Subject: [PATCH 3/3] process(impact-block): a generated on-demand routing row alone does not require the A/B Applying the block to 11 open PRs marked #273 and #346 "A/B REQUIRED" only because render-routing added an on-demand row to agent stubs, with Context cost 0. Say so: that case is NOT RUN with the reason, not required. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VJgWP5vEoAsNsJYqCDMGNw --- docs/RELEASE-PROCESS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/RELEASE-PROCESS.md b/docs/RELEASE-PROCESS.md index e6b5cf09..28398277 100644 --- a/docs/RELEASE-PROCESS.md +++ b/docs/RELEASE-PROCESS.md @@ -108,6 +108,9 @@ The reviewer, for each of the 5 rows, reads the diff and confirms or corrects th for a change to always-loaded / baseline-routed text or an agent stub, **recommended** for situational skill text, **NOT RUN with a reason** for process docs and templates that review agents read (they are agent-read text, so never "n/a"), and **n/a** for scripts only. + A stub change that is only a generated on-demand routing row (`render-routing.sh` output, + Context cost 0) is not an agent-stub change for this purpose: Behaviour is **NOT RUN: on-demand + row only**, not required. 5. **Runtime** — measured seconds per exec/gate/check, or "n/a" with nothing on a hot path. The review ends with 5 of 5 rows confirmed; "NOT RUN: " counts, a blank does not.