feat(review): add review:explain-change with a digest policy and an interactive view - #6018
Conversation
…nteractive view Rename review:pr-explainer to review:explain-change and leave a one-release stub. The markdown digest is the record; build-digest.mjs builds the view from a checked-in template plus escaped JSON through view-builder's interactive profile, outside the working tree. digest-policy.mjs resolves the new review-digest cascade concern (off, offer, always; offer triggers on files, changed lines, blast radius, risk paths, and an opt-in label) and the rendered-views medium key. The skill never posts to the pull request and never sets a check status. Refs #1217 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
build-digest.mjs takes no --out: each page goes to a fresh mkdtemp directory under the realpath of the OS temp dir, written with flag wx, so a diff-steered caller cannot aim K2 text at rules, shell, or settings files. digest-policy.mjs reads the team layer and the rendered-views team file from the pull request's base ref (baseRefName), applies an overlay only when untracked and gitignored, and always fires risk-path for a change to any digest config file, so a checked-out PR cannot silence its own digest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On a case-insensitive filesystem a PR could track .claude/Review-Digest.local.json and have it read as the untracked overlay. The tracked check now lists files with an :(icase) pathspec, and the always-on config paths match case-insensitively against normalised PR paths (backslashes, ./, duplicate slashes). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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. |
|
Claude finished @kyle-sexton's task in 5m 41s —— View job Review complete
ScopePer What I checkedI traced the two new scripts line by line against their stated contracts and the cascade/view-builder machinery they depend on:
I found no correctness issues meeting this lane's high-signal bar (something a careful senior reviewer would block or flag), so there are no inline comments to post. Not independently verifiedI could not execute shell/node commands in this sandbox (tool-use approval was denied for running the test suite, e.g. |
|
Claude finished @kyle-sexton's task in 4m 55s —— View job Security review (
|
![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: 24eedc6b88
ℹ️ 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".
|
claude-security-review has reviewed this pull request through 7003422; a later push is reviewed from there. |
|
claude-review has reviewed this pull request through 7003422; a later push is reviewed from there. |
…ge test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 7s —— View job Code review complete
ScopePer What's new since the last review at this headThe only commit since FindingOne inline comment posted on The new Not re-litigatedCodex's P1 note (digest policy has no caller yet for unrequested PR events) and P2 note (base-ref resolution for forked PRs) are not raised here as findings: the former is explicitly disclosed in the PR's own Related section ("Not wired here... belongs to source-control"), and the latter doesn't hold up under this repo's CI checkout model, where |
|
Claude finished @kyle-sexton's task in 1m 39s —— View job Security review (
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 11s —— View job Code review complete
Skip gate applies
Standing note (not new)The doc/code contradiction flagged in the prior reviews is still open: the |
|
Claude finished @kyle-sexton's task in 2m 4s —— View job Security review (
|
…uard the overlay digest-policy.mjs reads the team layer and the rendered-views team file at the PR's baseRefOid, which names the base repository's commit even for a fork; a missing or malformed oid skips the team layer with a warning. An untracked overlay that is not gitignored now applies with a warning, as the config-cascade convention says, and an overlay reached through a symlinked .claude or overlay file, or resolving outside .claude, is refused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 3m 39s —— View job Code review complete
ScopePer What changed since
|
|
Claude finished @kyle-sexton's task in 2m 13s —— View job Security review (
|
# Conflicts: # docs/conventions/rendered-views/CHANGELOG.md Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
A PR could make .claude a submodule holding the overlay files, which ls-files on the parent repo never sees. The overlay is now refused when .claude itself is a tracked entry (gitlink, file, or symlink) in any case, or when .claude/.git exists, and .gitmodules joins the config paths that always fire risk-path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lodic-software/claude-code-plugins into feat/1217-review-explain-change
kyle-sexton
added
do-not-merge
Keep review 0.38.0 above main's 0.37.2 and both changelog entries. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 3m 30s —— View job Code review complete
ScopePer What's new since the last review at
|
|
Claude finished @kyle-sexton's task in 2m 51s —— View job Security review (
|
Keep review 0.38.0 above main's 0.37.2 and both changelog entries. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 1s —— View job Security review (
|
|
Claude finished @kyle-sexton's task in 1m 6s —— View job Code review complete
Skip gate applies
That commit lands in two files this PR also touches, Standing items (not re-raised, already resolved on this head)The doc/code contradiction on the overlay's gitignore behavior ( |
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 4s —— View job Code review complete
ScopePer What changed in this incrementBoth edits close the doc-completeness note this lane raised at
No findingsBoth changes are accurate, narrowly-scoped doc fixes with no code behavior change in this increment. No correctness, design, or convention issues found. Not re-litigatedCodex's P1 note (no caller wires |
Keep review 0.38.0 above main's 0.37.2 and both changelog entries. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 5s —— View job Security review (
|
|
Claude finished @kyle-sexton's task in 2m 11s —— View job Code review complete
Skip gate applies
That merge lands in four files this PR also touches (shared aggregator docs that regenerate across Standing items (not re-raised, already resolved on this head)The doc/code contradiction on the overlay's gitignore behavior |
|
Claude finished @kyle-sexton's task in 1m 10s —— View job Security review (
|
|
Claude finished @kyle-sexton's task in 39s —— View job Code review complete
Skip gate applies
Those merges touch a few files this PR also carries (shared aggregator docs that regenerate Standing items (not re-raised, already resolved on this head)The doc/code contradiction on the overlay's gitignore behavior |
…ection main released discovery review (feat(review): add review:explain-change with a digest policy and an interactive view (#6018); fix(discovery): run research gates as one plain command); this branch's entries for those plugins move one patch above. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
No related issue: wave 3 PR 2 of the CI performance program (precise test selection). Refs #3932, #6021. ## Summary `scripts/affected-tests.sh` now selects the suites a change runs or reads, in the change's own language, instead of every suite that mentions a file name anywhere, every shell suite of a touched plugin, and three always-run suites. Over the 895 pull requests merged to main in the 7 days to 9671ece it selects 9,534 suites where main's selector selects 34,365, within 5% of the design model's 9,095, and both real main breaks from the window are still selected. Every Node suite it selects also runs: a Node suite that CI runs through a sibling `.test.sh` brings that wrapper (R9). ## Fix Rules (full text in the script header): - **Same-language edges (R3).** A file in the changed file's language that names it on a code line is a dependent, transitively. - **Another language counts only where the line runs or loads the file (R4):** an interpreter or process API on the line, matched as a word (the `.sh` of `x.sh` is not one), or a path to the file. A chain takes at most one such transition. A changed data file (C#, Markdown, YAML, ...) reaches code of any language that names it without spending the transition. - **Comment lines never count**, in suites and in code. `# shellcheck source=` and JSDoc `@import` / `import()` still count. - **Manifests select no suite through a mention.** plugin.json, marketplace.json, hooks.json, settings.json, package.json, package-lock.json, CHANGELOG.md and LICENSE reach a suite only through R1, R2 or a declared scope; their gates own them. - **Ambiguous names count only when the mention resolves.** This covers basenames two or more files carry, plus README.md, SKILL.md, AGENTS.md, CLAUDE.md and index.md. A mention resolves when it comes from the file's own directory; when it is a bare name from a directory above the file with no other file of that name below; when it ends in the file's shortest unique path suffix of two or more components; or when it is a path relative to a directory below the root that holds both files and has a directory in it (`$PLUGIN_DIR/skills/interview/SKILL.md`). A name-only path (`$SKILL_DIR/SKILL.md`, `$T/README.md`) or a root-relative one (`$ROOT/.github/workflows/ci.yml`) does not resolve, because tests build those paths under temporary directories; the suites the strace saw reading such files declare them. - **No Python import rule.** `import foo` does not name `foo.py`, as in design rules S1-S9. A module that only an import reaches is unmapped and falls back to the Python corpus (S9). - **Declared scopes replace R8's whole-plugin rule and the always list (S7).** A suite that reads files it never names declares them in its leading comment block, before any code or docstring: `# test-scope: <glob> [<glob>...]`, one or more lines. 87 suites carry 132 globs, seeded from an strace of every suite. `scripts/affected-tests.test.sh` also declares the live files its LIVE cases find by glob (the github `advise` and planning `interview` skill bodies, the autonomy reference docs), so renaming or deleting one runs it; its reference-YAML case runs in a fixture on probe files, so that YAML stays unmapped. A changed suite whose glob matches no file fails the run. `scripts/affected-tests-always.txt` is deleted, and `--with-always` is accepted and does nothing. - **A wrapped Node suite brings its wrapper (R9).** A selected `<stem>.test.js` or `<stem>.test.mjs` whose directory holds `<stem>.test.sh` selects that wrapper too, after every other rule and before `--shard`. CI runs such a suite only through the wrapper (`scripts/run-outside-node-suites.sh` reports it `OWNED` and runs nothing), and the walk stops at a reached suite, so a change reaching `exec-bash.resolver.test.mjs` through `lib/exec-bash.mjs` used to select the suite, not the wrapper, and CI ran neither. - **An unmapped file runs only its own language's suites.** `--unmapped-corpus` keeps the report, adds that language's corpus and exits 4. Wiring it into `ci.yml` is PR 3's job. - **No-suite list.** `plugins/*/evals/*` replaces the eval fixture entries. The Python module entries main listed (`plugin_cache_versions.py`, `discover.py`, `docs_crosscheck.py`, the `session_bridge.py` copies) are removed, so a change to one is unmapped and falls back to the Python corpus instead of selecting nothing. `plugins/performance/lib/spawn_noise.py` (a copy nothing imports) is added. - **`--replay <range> [--against <ref>]`** reruns the selector on each first-parent commit against its parent in a scratch clone, with this tree's no-suite list and declared scopes (handed to older commits through `AFFECTED_TESTS_SCOPES`), and, with `--against`, prints only the suites the two selectors disagree on, `<ref>` using its own headers. - **Releases.** The 30 plugins whose suites gained a header get a patch bump and a CHANGELOG entry naming those suites; nothing they run changed. ## Verification ### Replay: 7 days of merged pull requests Every first-parent commit on main from 2026-09-26 to 9671ece (900; 895 change a file), selected against its parent three ways: main's selector at 9671ece, this PR's selector, and the design's model (`selmodel.py`, the script that produced the design's section 3 figures) re-run on the same commits. | measure (895 PRs) | main | this PR | design model | |---|---|---|---| | suites per PR, p50 / p90 / p95 / max | 22 / 83 / 142 / 523 | 5 / 24 / 33 / 281 | 4 / 23 / 31 / 273 | | suites selected, total | 34,365 | 9,534 | 9,095 | | total with the language-scoped unmapped fallback (S9, PR 3) | 38,094 | 15,290 | 27,360 | | total as CI runs it today (unmapped: whole shell corpus) | 43,852 | 21,686 | 32,445 | | shell suites selected | 32,420 | 7,946 | 7,352 | | PRs with an unmapped file | 22 | 26 | 48 | | PRs with no shell change that start shell suites | 491 of 491 (10,269) | 351 of 491 (1,560) | 312 of 491 (1,361) | | PRs changing no code that start any suite | 356 of 356 (7,181) | 233 of 356 (974) | 203 of 357 (856) | | PRs selecting no suite | 0 | 127 | 157 | The design's section 3 figures (p50 3, p95 28, 7,348 total, 13,371 with the fallback) came from a different window, 826 PRs to 2026-10-02 20:52Z. The same model gives the last column on this window, so that column is the target the 5% bar applies to. ### Design targets vs measured | | this PR | design model | difference | |---|---|---|---| | suites selected, total | 9,534 | 9,095 | +439 (+4.8%) | | p50 / p90 | 5 / 24 | 4 / 23 | +1 / +1 | | p95 | 33 | 31 | +2 (+6.5%) | | max | 281 | 273 | +8 (+2.9%) | The total is within the 5% bar. p50 is 1 suite over the model and p95 2, and the declared scopes account for both: selections only this PR makes are 713 through a declared scope, 41 wrappers (R9), 31 through a mention and 16 siblings; selections only the model makes are 362 (its guessed plugin scans 190, its interpreter test matching the `.sh` of a file name 116, siblings 56). Without the 713 declared-scope selections, p95 would be 30. The declared scopes are what the strace saw the suites read, plus the live files `scripts/affected-tests.test.sh` reads by glob (18 of the 713: edits to the github `advise` and planning `interview` skill bodies and the autonomy reference docs). What dropping the Python import rule cost, against the previous head: 903 (PR, suite) selections over 75 suites, 264 of which main also made; the strace saw the suite read a changed file in 60 of them. Where nothing else maps the module (`discover.py`, `docs_crosscheck.py`, `plugin_cache_versions.py`), the change is unmapped and the S9 fallback runs the Python corpus. Where the module maps through a sibling or a mention (`hygiene.py`, `destructive_guard.py`), the suites that only import it are not selected; the design accepts that and catches it with the twice-daily full run and the trace audit (PR 4). ### The C# fixture case `plugins/code-metrics/scripts/fixtures/sources/CmSample.cs` goes from 14 suites to 7. `dispatch.test.sh`, `audit-complexity.test.sh` and `audit-type-debt.test.sh` name the file. `audit-coverage.test.sh`, `audit-duplication.test.sh` and `audit-size.test.sh` declare `plugins/code-metrics/scripts/fixtures/*`, which the strace saw them read. `setup-check.test.sh` sits beside `setup-check.sh`, which copies the plugin's `scripts/`. The design predicted 4; the three declared scopes make the difference. ### The two real main breaks are still selected - 88dd7e1 selects `plugins/github/github.test.sh` (its header declares `plugins/github/*`) and `plugins/planning/tests/interview-defenses.test.sh`. - e544012 selects `plugins/planning/tests/interview-defenses.test.sh`: `$PLUGIN_DIR/skills/interview/SKILL.md` resolves to the changed file. The suite also pins both against the live tree. ### Trace audit of every drop Every suite ran under strace to record its file reads. Of 25,352 (PR, suite) pairs this PR drops against main, over 700 suites, the strace saw the suite read a changed file in 1,004 pairs over 53 suites: - **Manifest identity reads:** `install_state`, `sync-run`, the planning `surface`/`watch` suites and the `*-format` hook suites read a plugin's name or version from `plugin.json`, or Node reads the root `package.json` while resolving modules. The root package files are pins that PR 3 routes to the Node lanes (S8). - **Live gates CI also runs as whole-tree gate steps:** `check-loop-lane-floor-drift`, `check-fixture-git-isolation`, `check-summary-reader-parity`. - **`scripts/affected-tests.test.sh`:** its LIVE cases run the selector, whose `git grep` reads every file. - **Plugin copies and walks:** `abort-boundary`, `check-guardrails-ps-differential` and `test_kill_switch_probe.py` read a README or changelog, which does not change what they test. - **Python imports:** the 60 pairs above, where a test imports a changed module that something else maps. - **Relations only today's tree has**, since the trace ran on today's tree and each PR replays on its own: `interview-defenses.test.sh` began naming `context/surface.md` after 7 of its PRs. `check-prerequisite-probes.test.sh` has been deleted since. The full list, each suite with its PR count, its trace verdict and main's reasons for selecting it, is in [this comment](#6059 (comment)) (57 KB, too large for this body). ### Every selected Node suite runs (R9) The figures above are the previous head's per-PR selections with R9 applied. A real replay of this head's selector over the same 900 commits, with the same inputs as the previous head's replay, removes nothing and adds exactly those 41 (PR, suite) selections over 33 PRs, each the `.test.sh` wrapper of a Node suite already selected: `lib/exec-bash.resolver.test.sh` 31, `plugins/guardrails/hooks/exec-bash.resolver.test.sh` 7, `plugins/autonomy/skills/setup/scripts/resolve-prerequisites.fixtures.test.sh` 3. No PR's exit code or unmapped set changes; three `--unmapped-corpus` runs also gain the wrappers of the Node suites their corpus adds. Selected Node suites that CI runs nowhere (outside the registered packages and the four sub-projects, with a wrapper that is not selected): 41 (PR, suite) pairs over 33 PRs at the previous head, 5 of which main covered by selecting the wrapper; 0 at this head. On today's tree, `lib/exec-bash.mjs` plus `plugins/guardrails/hooks/exec-bash.mjs` selects both `exec-bash.resolver.test.mjs` suites and both wrappers, and `run-outside-node-suites.sh --paths` on the two Node suites (CI's exit-3 branch) reports each `OWNED` by its wrapper and exits 0. The workflow scripts of review, planning, testing, discovery and multi-agent and the autonomy `fixture-harness.mjs` and `resolve-prerequisites.mjs` select 9 wrapped Node suites, each with its wrapper. ### Local runs WSL (Ubuntu 26.04) at the head, main merged: - `scripts/affected-tests.test.sh`: PASS=134 FAIL=0 (the 4 Python-import cases are gone; the R8 cases now build suites with headers, including a declaration below code that declares nothing and a stale glob that fails only the run changing its suite; the R9 case, a Node suite reached through a mention, fails on the previous head's selector, 133/1) - `scripts/lib/gate-entry.test.sh`: 33/0 - Headers read back from the 87 suites equal the former list entry for entry, plus the three globs `scripts/affected-tests.test.sh` adds for the live files it reads. - The selector on `plugins/github/skills/advise/SKILL.md`, `plugins/planning/skills/interview/SKILL.md` and an autonomy reference doc selects `scripts/affected-tests.test.sh` through its header; the reference YAML (`plugins/toolchain/reference/ecosystems/go.yaml`, `docs/conventions/ecosystem-commands/examples/go.yaml`) stays UNMAPPED at exit 1. - shellcheck clean on the selector and the 81 changed shell suites; the pinned ruff check passes on the 6 changed Python suites. - `check-changelog-parity.sh` `--check`, `--check-bump`, `--check-preserved` and `--check-order` pass against origin/main. Where main released a plugin this PR also releases (planning in #6063; animation in #6081; code-metrics, harness-ops, repo-hygiene, session-flow and source-control in #6065; actionlint, animation, autonomy, context-guard, guardrails, harness-ops, source-control, speech and testing in #6064; speech in #6082; review in #6018; discovery in #6088), this PR's entry sits one patch above main's. - Main's #6064 deleted `scripts/lib/sync-cluster.sh`, the only file the `scripts/lib/sync-*.sh` glob in `scripts/affected-tests.test.sh`'s header matched; the glob is dropped, since the selector fails (exit 2) on any diff that changes a suite declaring a glob that matches nothing, as it did on the merge commit alone. ## Related - #3932: the CI performance program. - #6021 (wave 3 PR 1, `ci.yml` job rename) has landed. It edited `scripts/affected-tests-always.txt`, and this PR keeps that file deleted. - PR 3 should: - run `--unmapped-corpus` in place of the whole-shell-corpus fallback, which gives the 15,290 figure above; - route the pins (root `package*.json`, `.node-version`, Python pins) to their lanes (S8); - drop `--with-always` from `ci.yml`; - update the `ci.yml` comment that names `affected-tests-always.txt`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
… a recording link, and publish as an Artifact by default (#6102) Closes #5856 ## Summary Completes the `review:explain-change` digest on top of the core from #6018: - **Fresh-context risk-map check.** Step 3 of the skill dispatches one subagent with a fixed brief that carries only the pull request number and repository. It never gets the record, the builder's risk rows, or its reasoning. The skill then marks each row `agreed`, `disputed` (the row and its level stay, with the checker's level and reason), `added` (an area only the checker named), or `unchecked` (no check ran). Rows are never dropped. The page shows a Check column. - **Optional quiz.** `--quiz`, or a reader's request, adds three to five questions to the record and the page. The reader ticks choices. The copied or saved reply carries only builder ids (`quiz-1-questions-<q>-choices-<c>`). With no request, neither the record nor the page has the section. - **run-e2e recording link.** When `/testing:run-e2e` recorded the pull request's head (`headRefOid`), the record links the recording and the page shows its path as text. - **Artifact by default.** `digest-policy.mjs` resolves `medium` to `artifact` when no layer sets it. `medium: file` in `~/.claude/rendered-views.md` keeps the page local. The rendered-views README and the review-digest convention now state the `artifact` default. ## Fix - `build-digest.mjs` `shapeDigest` adds `risks[].check` (limited to the four values, anything else becomes `unchecked`), `risks[].checker`, and turns `recording` and `quiz` into lists of zero or one section. A list with no rows renders nothing, so an absent quiz or recording leaves no heading. The runtime (`lib/view-runtime.js`) and `lib/view-builder.mjs` are unchanged. - `templates/digest.html` adds the Check column and puts the recording and quiz sections inside `data-rv-each` containers. It contains no data-driven attribute and no non-fragment `href`: the recording path is shown as text, never as a link. - Security behavior from #6018 is unchanged: no `--out`, `mkdtemp` plus `wx` output, refusal of a temp dir inside a working tree, the team layer read at `baseRefOid`, the overlay guards, `CONFIG_PATHS`, and output only through the interactive profile. The digest never posts to the pull request and never gates merge. - `review` 0.38.0 to 0.39.0 with a CHANGELOG entry. There is also a rendered-views convention CHANGELOG entry. ## Verification - `node --test plugins/review/tests/explain-change.test.mjs`: 54 pass, 0 fail. New tests: quiz, recording, and check fields reach the page only through the data block, including hostile strings in each one; the check value is limited to the four values; quiz and recording come out as empty lists when the input lacks them, and an empty page still validates; quiz and recording headings sit inside their list containers; the checker brief's only placeholders are `<n>` and `<owner/repo>`; the default `medium` is `artifact` and a user-global `medium: file` overrides it. - `bash plugins/review/tests/explain-change-chrome.test.sh`: all tokens ok. - Rendered both pages in Chromium through playwright-cli from a local HTTP server. Full digest: the Check column, recording, and quiz all render, and ticking a choice then saving produced `picked: quiz-1-questions-2-choices-2`. Bare digest: no quiz or recording heading, and both optional containers have `display: none`. - `skill-quality` `check-skill.sh` over `plugins/review/skills`: explain-change PASS with 0 warnings. `check-evals-quality.sh` PASS. `scripts/check-changed-skills.sh origin/main` PASS. `check-skill-description-voice.sh origin/main` PASS. `check-changelog-parity.sh --check-bump origin/main` PASS. `check-purged-em-dashes.sh` PASS. markdownlint-cli2 with the repo config is clean on the six changed markdown files. - Not run: `scripts/check-html-assets.sh`, because htmlhint is not installed in this worktree (`npm ci` not run). ## Related - Refs #5835, the parent spec container (wave C1). - Builds on #6018 (#1217), the explain-change core. - Not changed in this PR: `~/.claude/rendered-views.md` (`medium: file` for the owner's personal layer) is user-scope config for the owner to set. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2 participants
Closes #1217
Summary
Slice 21 of #5835: the
review:explain-changecore. It renamesreview:pr-explainertoreview:explain-changeand leaves a one-release stub. It adds adigest_policy(off,offerby default,always) that resolves from a newreview-digestcascade concern, and it builds an interactive digest page only throughlib/view-builder.mjs. The risk-map check, quiz, recording link, and artifact publish default stay with #5856.Fix
plugins/review/skills/explain-change/SKILL.md: the markdown digest (why, before and after, risk map, where to focus, annotated hunks) is the record. The page is a view of it.templates/digest.html: the checked-in interactive template. It usesdata-rv-*bindings only, filters files, collapses hunks, and has a "Reviewed" pick per file, a note, and copy and save buttons.scripts/build-digest.mjs: copies the input down to the fields the template binds, all as strings. It builds with view-builder's interactive profile, so the data goes into the JSON data block and the hash-pinned runtime renders it as text. It writes to the OS temp directory and refuses any--outinside a working tree, so the view never sits beside the record.--checkvalidates a page.scripts/digest-policy.mjs: readsgh pr view --json files,additions,deletions,labelson stdin and decidesskip,offer, orbuild. The offer triggers are more than 5 files, more than 200 changed lines, a HIGH or CRITICAL blast radius, a risk-path glob, and theexplain-changelabel.alwaysbuilds at--event ready, and--requested(a direct ask) always builds. It also resolves the rendered-viewsmediumkey. This lane shipsfile; theartifactdefault is review:explain-change: fresh-context risk-map check, quiz section, run-e2e recording link and artifact publish default #5856's.allowed-toolsgrant onlygh pr view,gh pr diff, and the two scripts, and neither script callsgh.plugins/review/skills/pr-explainer/SKILL.md: a one-release stub naming/review:explain-change, withdisable-model-invocation: true. The old report builderbuild-explainer.mjsis removed.docs/conventions/review-digest.md: the owner doc for the concern. Itsjson configblock is this repository's team layer and holds the shipped defaults. A test keeps it equal to the script'sDEFAULTS.docs/conventions/config-cascade/README.md: adds an Implementers row and a root-rule row forreview-digest, and regenerates the semantics table. The row declares one deviation: an untracked team layer is reported and treated as absent instead of stopping the run, and an overlay that is not gitignored is reported but still applied. The reason is that every key only decides when a reader is offered a view.docs/conventions/rendered-views/README.mdand its CHANGELOG now name the new lane.lib/html-escape.test.shhad used the removed pr-explainer builder as its exemplar page. It now builds that page with view-builder's report profile. Three things went with the old builder: the title-attribute case (the report profile does not allow slots inside tags), the builder-source interpolation scan, and the builder CLI cases.0.36.4to0.37.0, with a CHANGELOG entry. README, catalog, and cheat sheet are regenerated.Verification
bash plugins/review/tests/explain-change.test.sh: 40 of 40 pass. It covers one test per offer trigger (files, changed lines, HIGH, CRITICAL, two risk paths, label) and that exact thresholds do not fire. It covers off, always at ready, always before ready, and a direct request. Through the CLI it covers the cascade (user-global, then the tracked team docs block, then the overlay, key by key; the argument beats every layer; a malformed layer, an invalid value, or an unknown key degrades soft; the docs block beats.claude/review-digest.jsonwith a warning; two blocks are invalid) andmediumresolution. For the builder it checks that hostile data stays in the JSON data block, that unbound fields never reach the page, that the page passes the interactive profile, and that a path inside the working tree is refused. It also checks the read-only boundary.bash plugins/review/tests/explain-change-chrome.test.sh,bash lib/html-escape.test.sh(28 cases), andbash lib/view-builder.test.shpass.scripts/affected-tests.sh --run --jobs 6: every selected shell suite passes exceptcheck-script-contract.test.sh, which failed only becausehtmlhintwas missing. Afternpm ciit passes 48 of 48, andscripts/check-html-assets.shis clean.htmlhinton the new template is clean.check-changelog-parity.sh(--check,--check-bump,--check-preserved),generate-catalog.mjs --check,generate-cheatsheet.mjs --check,sync-config-cascade-semantics.py --check,check-skill-leaf-names.sh,check-docs-naming.sh,sync-shared-copies.sh --check,check-contract-clause-coverage.py,check-orphaned-fixtures.sh,check-spoke-plugin-root.sh,sync-plugin-options-docs.py --check, andcheck-purged-em-dashes.sh. markdownlint finds no issues in the changed markdown, and the ai-slop detector finds nothing in the new prose.check-skill.sh:explain-changepasses with 0 warnings.pr-explainerpasses with 5 warnings, all of them the deliberate move of trigger phrases toexplain-change.check-evals-quality.shpasses on the new evals.files-1-hunks-2), and the<img>and</script>payloads appeared as text.Related
lib/view-builder.mjs), rendered-views: convention v2 with tiers and content classes, plus the record-bundle convention #5851, and feat(lib): view-builder.mjs and view-runtime.js with report and interactive profiles #5852.docs/native-surfaces/records.jsonstill records operator verdicts againstreview:pr-explainer. I left them unchanged because each one is a human ruling./source-control:pull-request readywith--event ready. That change belongs to source-control.🤖 Generated with Claude Code