review-change
Reviews a change for code health: structural problems in the changed functions and duplication that touches the change. Use when the operator asks to review a diff, branch, commit range or pull reques
SKILL.mdplugin/skills/review-change/SKILL.md
GitHubReview Change
This skill reviews one change for code health with the act-lite MCP tools. It reports structural problems in the functions the change touches and the duplication the change takes part in. It ends in a report and never edits a file.
Adapt
An operator instruction wins over every default below: the base to compare against, the files to review, whether tests are reviewed.
The tools are the act-lite MCP server's. The same results come from the
act-lite analyze command (act-lite analyze patterns …), so a session
without the MCP server runs the commands instead.
act-lite does not run git; you do. Run only the read-only git commands
below. Never stash, reset, check out or commit.
Inputs
- Base: a git ref. Default: the merge base of
HEADwith the default branch (git merge-base HEAD origin/HEAD, elsemain, elsemaster). The change is the working tree against the base. - Or a file list: the files to review. Every line of each listed file counts as changed, and step 1 is skipped.
include_tests:false, unless the operator asks to review tests too. Pass it to both tools only then.
Procedure
Changed files and changed lines. Skip this step for a file list.
- Files:
git diff --name-only --diff-filter=AMR <base>, plus the untracked files fromgit ls-files --others --exclude-standard. Every line of an untracked file counts as changed. - Lines:
git diff -U0 --diff-filter=AMR <base>. Each hunk header@@ -a,b +c,d @@marks the new-side linesctoc + d - 1as changed. A hunk that only deletes lines (d= 0) marks the new-side linecat the deletion point as changed, so the function around a deletion is reviewed. - Paths are relative to the repository root, which is the workspace of the act-lite server.
- Files:
Nothing to review, or nothing in scope.
- If no file changed, write the report with "No file changed against
<base>." and call no tool. - If a tool call returns the error that starts with
no files to analyze in the requested scope(for example every changed file is a test), quote the message in the report's Not analyzed section, and still run the other step.
- If no file changed, write the report with "No file changed against
Structural problems. Call
analyze_patternswithpaths= the changed files andlimit= 1000.- Keep each finding whose
range("L:C-L:C", lines inclusive) overlaps a changed line of itsfile. - If the result has
truncated: true, split the file list in two halves and call once per half, again while a result is truncated and its list has more than one file. Concatenate the findings in file order. A single file that is still truncated contributes its first 1000 findings, and the Limits section gives itstotal.
- Keep each finding whose
Duplication. Call
analyze_cloneswithpaths=[](the whole workspace),focus= the changed files andlimit= 1000.- Keep each clone class with a member whose
fileis a changed file and whoserangeoverlaps a changed line of that file. - A result lists at most 20
membersof a class;members_totalgives the full count. When no listed member overlaps a changed line andmembers_totalis more than the members listed, keep the class too: a member that is not listed can overlap the change. Mark it "overlap not determined". Members infocusfiles are listed first, so this occurs only when at least 20 members are in changed files. - If the result has
truncated: true, splitfocusin two halves and call once per half, again while a result is truncated and its list has more than one file. A single file that is still truncated contributes its first 1000 classes, and the Limits section gives itstotal. Each call analyzes the whole workspace, so a split is the rare case.
- Keep each clone class with a member whose
4a. Files exposed to the change. Call analyze_impact with paths =
[] (the workspace), focus = the changed files that exist in the working
tree and limit = 100.
- A deleted path in a file list is dropped from
focusand named in the section. - A changed file in a language without dependency support still gets its
dependents (for example the importers of a changed CSS file). It is
listed in
details.focus_files_not_analyzed. - If the change only deletes files, there is no focus: skip this step. The section lists the deleted files (`git diff --name-only --diff-filter=D<base>`) and says that relationships that pointed at them now appear as unresolved in the Limits section (from step 4b).
- Steps 4a and 4b run whenever the change adds, modifies, deletes or renames a file. The rule in step 2 that no tool is called when no file changed applies to steps 1 to 4 only.
- Pass
include_teststoanalyze_impactandanalyze_cyclesonly when the operator asks to review tests too.
4b. Cycles that involve the change. Call analyze_cycles with paths =
[] and limit = 1000. Keep every cycle when the change can re-resolve a
relationship of an unchanged file, that is, when the change:
- (a) adds, deletes, renames or changes the type of a file
(
git diff --name-only --diff-filter=ADRT <base>, plus the untracked files). A file list has no base, so (a) cannot be decided and every cycle is kept; - (b) touches a resolution input or a discovery input: a file named
package.json,go.modorCargo.toml, any*.jsonfile (atsconfigextendstarget can have any name),.gitignore,.git/info/exclude,pyvenv.cfgorCACHEDIR.TAG. (b) also holds when the change touches any file listed in theanalyze_cyclesresult'sdetails.resolution_inputs(for example atsconfigextendstarget namedbase.jsoncor with no extension). Whendetails.resolution_inputs_totalis more than the entries shown, every cycle is kept. A listed input that is a symbolic link is read through its target, which the list does not name: when any listed input, or a directory above it, is a tracked symbolic link (git ls-files -smode120000), every cycle is kept; - (c) changes a
.rsfile that contains a module declaration (mod <name>;ormod <name> {, also inside a macro invocation) or a#[pathattribute, before or after the change (the Rust module tree); or - (d) modifies a Python
__init__.pyfile. Its bound names decide whetherfrom pkg import nin an unchanged file resolves topkg/__init__.pyor to the submodulepkg/n.py, so a change to them can create a cycle with no changed member. (A module filex.pyhas no submodules, so its bindings cannot redirect an edge to another file.) It also holds when the change modifies a Python file that a package's__init__.pyis a symbolic link to, because that file's bound names are the package's: when the repository tracks a symbolic link named__init__.py, or one to a directory (git ls-files -smode120000), treat a change to any.pyfile as (d).
Otherwise keep each cycle that has a changed file among its members. When
members_total is more than 20 and no shown member is a changed file, keep
the cycle and mark it "members not all shown". If truncated is true, say
how many of total were examined.
- The kept set holds every cycle that the change creates: a new cycle needs an edge that did not exist before. An edge appears only when a relationship of a changed file changes (that file is then a member of the cycle), or when an unchanged relationship resolves differently, which happens only in (a), (b), (c) or (d). The kept set can also hold older cycles. The report does not claim that each cycle is new: comparing with the base needs an analysis of a second checkout.
- Report. Write the report below from the kept findings and classes and
from each result's
status,complete,coverage,excluded,incompleteanddetails. Write nothing else, and change no file.
Report
## Code health review: <base>..<working tree>
Scope: <n> changed files; patterns <status>, complete <bool>; clones <status>, complete <bool>.
### Structural problems in changed functions
| Location | Function | Rule | Value | Threshold |
### Duplication that touches the change
- <kind> clone, <tokens> tokens, <members_total> members: <file>:<range> (variant <v>), …
### Files exposed to the change
<n> dependents (<direct> direct, <transitive> transitive) of <changed> changed files (<focus_files> focus files; not analyzed themselves: <focus_files_not_analyzed>).
| Dependent | Depth | Reaches | Through |
A dependent is a file that imports a focus file, directly or through other files. It is exposed to a change in the focus file. This is not proof that it will break, and a file that is not listed is not proof that it is safe: relationships that act-lite does not model (see the README) are not followed.
### Cycles that involve the change
<all cycles kept, because the change <adds, deletes or renames a file | touches a resolution or discovery input | changes a Rust file with a module declaration | modifies a Python __init__.py file>> | <n> cycles with a changed file>.
- <members_total> files: <path as <file> → <to> → …>[ (path of <path_total> steps, <n> shown)]; members: <file>, …[ (members not all shown)]; changed members: <file>, …
### Not analyzed
Unsupported languages, excluded counts (tests, generated, vendored, ignored), skipped files, parse failures, component regions not analyzed (regions_not_analyzed).
### Limits
A clone class shows that code has the same syntax tree, apart from formatting, comments and, for renamed clones, identifier names and literal values. It does not show that the members behave the same, that they can share one function, or that extracting them is safe. Findings are measurements against fixed thresholds (<details.thresholds>).
- Structural problems: one row per kept finding, in result order.
Location is
<file>:<range>; Function isfunction, or(anonymous)fornull; Rule, Value and Threshold arerule,valueandthreshold. With no kept finding, write "None in the changed functions." - Duplication: one line per kept class, in result order, with its
members(each<file>:<range> (variant <variant>)); whenmembers_totalis more than the members listed, add "and <n> more". For a class marked "overlap not determined", end its line with "— overlap not determined (<listed> of <members_total> members shown)". With no kept class, write "None that touches the change." - Files exposed to the change: the
details.direct,details.transitive,details.focus_total,details.focus_filesanddetails.focus_files_not_analyzedcounts, then one row per finding, in result order: Dependent isfile, Depth isdepth, Reaches isreaches, Through ischainwritten as<file> → <to>steps. Then the statement line, word for word. With no finding, write "No dependent." If step 4a was skipped, name the deleted files instead of the table. - Cycles that involve the change: the reason that all cycles were kept
(the case of (a), (b), (c) or (d) that applies), else "<n> cycles with a
changed file". One line per kept cycle, in result order:
members_total; thepathsteps, from the cycle'sfileback to it, written as<file> → <to>steps (the first step'sfile, then each step'sto), with "(path of <path_total> steps, <n> shown)" whenpath_totalis more than the <n> steps shown; themembers, listed separately, with "(members not all shown)" whenmembers_totalis more than the members listed (this includes each cycle marked "members not all shown" in step 4b); and the changed members. Themembersare sorted by path, not in traversal order: never write them as a path. The section says that the cycles are those that involve the change, not only those that it creates. With no kept cycle, write "No cycle involves the change." - Not analyzed: from each result, the
coverage.languagesentries withsupported: false, theexcluded.counts, theincomplete.skipped_filesand theincomplete.parse_failures, and any empty-scope message from step- A result with
complete: falsecan miss findings in that evidence; say so. Also add eachdetails.regions_not_analyzedentry (file,range,reason) of theanalyze_patternscalls, anddetails.files_without_analyzed_code; whendetails.regions_not_analyzed_totalis more than the entries shown, give the total. Do not use the list of theanalyze_clonescall: that call analyzes the whole workspace. In component files (.vue,.svelte,.astro), only the script regions are analyzed; the template, style and other regions are not.
- A result with
- Limits: the sentence above, the thresholds from the
analyze_patternsresult'sdetails.thresholds, and, for each split that stayed truncated, the file and itstotal. - Dependency limits: add to the Not analyzed and Limits sections, from the
analyze_impactandanalyze_cyclesresults: thecoverage.languagesentries withsupported: false; thedetails.relationshipscounts (total,resolved,external,unresolved); the relevantincomplete.counts.unresolvedand the first entries ofincomplete.unresolved(file,kind,target,reason), with a note that an unresolved relationship can hide a dependent or a cycle; eachlimits_hitentry; theexcluded.counts,incomplete.skipped_filesandincomplete.parse_failures; and each truncation (totalagainst what is shown). A result withcomplete: falsecan miss dependents or cycles; say so.