Skill Tracer
Skills › Compound Engineering › lfg › review-followup.md

review-followup.md

78 lines · skills/lfg/references/review-followup.md at 3449230

Open on GitHub

Most marks come from matching words in the file, so treat them as pointers.

1# Pre-ship quality steps (LFG steps 3–7)
2
3These are the quality steps that run after implementation and before shipping. On the defect route the implementation was `ce-debug`'s fix; `references/debug-return.md` names what each step below receives in place of the plan path. `ce-code-review` is review-only. LFG applies eligible fixes itself, then commits.
4

Pre-ship quality steps (LFG steps 3–7)

These are the quality steps that run after implementation and before shipping. On the defect route the implementation was ce-debug's fix; references/debug-return.md names what each step below receives in place of the plan path. ce-code-review is review-only. LFG applies eligible fixes itself, then commits.

Raw lines 1 to 4
Names a skill ce-debug :3
Names a skill ce-code-review :3
5## The shipping precondition, in these steps
6
7When the repo has no remote, the run finishes locally. That is a final state, not an error: never retry a push or hunt for a remote. Step 5 still makes every commit it calls for; only the pushes and the records that would live on the PR are dropped. With no PR to hold the unapplied findings, step 6 files them as tracker tickets and the DONE report states the rest. Never write them to a committed file nobody will read.
8

The shipping precondition, in these steps

When the repo has no remote, the run finishes locally. That is a final state, not an error: never retry a push or hunt for a remote. Step 5 still makes every commit it calls for; only the pushes and the records that would live on the PR are dropped. With no PR to hold the unapplied findings, step 6 files them as tracker tickets and the DONE report states the rest. Never write them to a committed file nobody will read.

Raw lines 5 to 8
9## Step 3 — simplify before review
11Simplification runs before review so the code-review in step 4 covers the simplified code. Let `ce-simplify-code` resolve the branch-diff scope itself; it preserves behavior and runs the test suite. Pass the plan path from step 1 as context about structure the simplification must keep, not as the simplification scope (the branch diff remains the scope); on the defect route pass the debug return's `root_cause` and `changed_files` instead, so the fix is not simplified away. On either route, pass each test `verification_evidence` names as proving the work (the regression or red-before-fix test, characterization coverage, or an existing test used unchanged) as its own `proof-test:<path>`, so the simplification leaves those tests untouched. Add a one-line constraint: KTDs labeled `session-settled:` describe structure the simplification must preserve, so deliberate duplication stays duplicated.
13Do not commit in this step. `ce-simplify-code` leaves its changes in the working tree; step 4's review scopes the working tree (uncommitted changes included), and step 9's `ce-commit-push-pr` commits whatever remains. Committing here would sweep any still-uncommitted `ce-work` edits into a misleading `refactor` commit and could stall on a tree that never goes clean.

Step 3 — simplify before review

Simplification runs before review so the code-review in step 4 covers the simplified code. Let ce-simplify-code resolve the branch-diff scope itself; it preserves behavior and runs the test suite. Pass the plan path from step 1 as context about structure the simplification must keep, not as the simplification scope (the branch diff remains the scope); on the defect route pass the debug return's root_cause and changed_files instead, so the fix is not simplified away. On either route, pass each test verification_evidence names as proving the work (the regression or red-before-fix test, characterization coverage, or an existing test used unchanged) as its own proof-test:<path>, so the simplification leaves those tests untouched. Add a one-line constraint: KTDs labeled session-settled: describe structure the simplification must preserve, so deliberate duplication stays duplicated.

Do not commit in this step. ce-simplify-code leaves its changes in the working tree; step 4's review scopes the working tree (uncommitted changes included), and step 9's ce-commit-push-pr commits whatever remains. Committing here would sweep any still-uncommitted ce-work edits into a misleading refactor commit and could stall on a tree that never goes clean.

Raw lines 9 to 14
Names a skill ce-simplify-code :11
Names a skill ce-commit-push-pr :13
Names a skill ce-work :13
15## Step 4 — invoke `ce-code-review`
17Load `ce-code-review` from the host catalog's listed path. A host skill named `review` is not this step; do not invent `skills/review/SKILL.md` under this plugin.
19```
20ce-code-review mode:agent plan:<plan-path-from-step-1>
21```
23Read the **Actionable Findings** summary and artifact path. Do not pass `mode:autofix`.
25Pass the plan file path from step 1 so `ce-code-review` can verify requirements completeness. On the defect route there is no plan: omit `plan:` and pass the `root_cause` summary as review context; the requirements check is additive by `ce-code-review`'s own contract and review still covers the diff. Also read any findings stamped `settled_conflict` (each names the conflicting KTD). Stamped findings that are matters of preference do not stop the pipeline (they are report-only), but they must reach step 6's record of unapplied findings.
27`mode:agent` is report-only **by design**: it reports findings but never edits the tree, and LFG applies the eligible ones in step 5. When narrating progress to the user, frame this as "review found X -> applied X in step 5," not as "code review did not auto-fix." A report-only review followed by an LFG-applied fix is the intended contract, not a gap.
29Capture parsed JSON (`status`, `actionable_findings`, `findings`, `artifact_path`, `run_id`) or the markdown Actionable Findings section. If `status` is `failed`, stop and report `reason`.

Step 4 — invoke ce-code-review

Load ce-code-review from the host catalog's listed path. A host skill named review is not this step; do not invent skills/review/SKILL.md under this plugin.

ce-code-review mode:agent plan:<plan-path-from-step-1>

Read the Actionable Findings summary and artifact path. Do not pass mode:autofix.

Pass the plan file path from step 1 so ce-code-review can verify requirements completeness. On the defect route there is no plan: omit plan: and pass the root_cause summary as review context; the requirements check is additive by ce-code-review's own contract and review still covers the diff. Also read any findings stamped settled_conflict (each names the conflicting KTD). Stamped findings that are matters of preference do not stop the pipeline (they are report-only), but they must reach step 6's record of unapplied findings.

mode:agent is report-only by design: it reports findings but never edits the tree, and LFG applies the eligible ones in step 5. When narrating progress to the user, frame this as "review found X -> applied X in step 5," not as "code review did not auto-fix." A report-only review followed by an LFG-applied fix is the intended contract, not a gap.

Capture parsed JSON (status, actionable_findings, findings, artifact_path, run_id) or the markdown Actionable Findings section. If status is failed, stop and report reason.

Raw lines 15 to 30
Names a skill ce-code-review :15
31## Step 5 — apply and persist review fixes

Step 5 — apply and persist review fixes

Raw lines 31 to 32
33### What to apply
35Apply a finding in the working tree only when **all** of the following hold:
371. **`suggested_fix` is present** — concrete change shape from the reviewer.
382. **`confidence` is `100`, or `75` with cross-persona agreement noted in the report** — do not apply anchor-50 findings.
393. **The fix is mechanical** — one coherent change; no change to a contract, permission, or security stance; no new public API shape; no behavior change that needs product sign-off.
404. **Evidence still matches the code** at the cited `file:line` before editing.
42Do not treat `autofix_class` as permission to auto-apply.

What to apply

Apply a finding in the working tree only when all of the following hold:

  1. **suggested_fix is present** — concrete change shape from the reviewer.
  2. **confidence is 100, or 75 with cross-persona agreement noted in the report** — do not apply anchor-50 findings.
  3. The fix is mechanical — one coherent change; no change to a contract, permission, or security stance; no new public API shape; no behavior change that needs product sign-off.
  4. Evidence still matches the code at the cited file:line before editing.

Do not treat autofix_class as permission to auto-apply.

Raw lines 33 to 43
44### What not to apply
46- `autofix_class: manual` without a clear mechanical `suggested_fix`
47- `autofix_class: advisory` — report-only
48- `gated_auto` findings that change behavior, contracts, auth, or permissions
49- Anything that needs a design conversation

What not to apply

  • autofix_class: manual without a clear mechanical suggested_fix
  • autofix_class: advisory — report-only
  • gated_auto findings that change behavior, contracts, auth, or permissions
  • Anything that needs a design conversation
Raw lines 44 to 50
51### Execution
531. Filter `actionable_findings` (or markdown Actionable Findings) with the bar above.
542. Apply eligible fixes in the working tree in severity order (`#` stable from the review).
553. Run targeted tests when `requires_verification: true` on any applied finding.
564. If `git status --short` shows changes, stage only review-driven files and commit `fix(review): apply review findings`. Then push before step 6 **when a remote is configured** (per LFG's shipping precondition) and, on the defect route, when everything the push would publish is work the user offered (`references/debug-return.md`). To push: if an upstream exists, run `git push`. If no upstream exists but a remote is configured (common on a fresh feature branch), pick a writable remote at run time: prefer `origin` when present, otherwise run `git remote` and choose the first configured remote. Then run `git push --set-upstream <remote> HEAD`. If there is no remote at all, do not push; the local commit suffices. If no eligible fixes were applied, say so explicitly and skip the commit.

Execution

  1. Filter actionable_findings (or markdown Actionable Findings) with the bar above.
  2. Apply eligible fixes in the working tree in severity order (# stable from the review).
  3. Run targeted tests when requires_verification: true on any applied finding.
  4. If git status --short shows changes, stage only review-driven files and commit fix(review): apply review findings. Then push before step 6 when a remote is configured (per LFG's shipping precondition) and, on the defect route, when everything the push would publish is work the user offered (references/debug-return.md). To push: if an upstream exists, run git push. If no upstream exists but a remote is configured (common on a fresh feature branch), pick a writable remote at run time: prefer origin when present, otherwise run git remote and choose the first configured remote. Then run git push --set-upstream <remote> HEAD. If there is no remote at all, do not push; the local commit suffices. If no eligible fixes were applied, say so explicitly and skip the commit.
Raw lines 51 to 57
58## Step 6 — residual handoff
60Residuals are the findings left over: actionable findings **not** applied in step 5. They are not leftovers from an autofix inside `ce-code-review`. Use the Actionable Findings summary / artifact from step 4.
62Two further triggers also require step 6, both outside the apply path: step 4 emitted any `settled_conflict`-stamped findings, or step 2's return carried proceeded-and-flagged `settled_decision_conflicts` entries. These are the findings where the work diverged from a settled decision, and this step is where they get written down somewhere that lasts.
64A residual at this point is undecided, not accepted debt. Step 5 declined it because it needs judgment, and the pipeline does not merge unless the user granted it, so the human reviewing the PR supplies that judgment: fix it in this branch, dismiss it, or file it to carry past merge. The record therefore goes where that reviewer already looks, the PR body. The pipeline files no tickets for these; one ticket per finding, decided by nobody, is how a run of small nits floods a tracker.
66**When a PR will exist (a remote is configured):** compose a `## Unapplied review findings` section, one checkbox bullet per item so a human ticks it when they close it:
68- For each unapplied actionable finding: `- [ ] <severity> — <file:line> — <title>`, plus the reviewer's `suggested_fix` on the next line when present.
69- For each `settled_conflict`-stamped finding from step 4: the same bullet plus the conflicting KTD the stamp names — included even though the finding is report-only.
70- For each proceeded-and-flagged `settled_decision_conflicts` entry from step 2: a bullet with the KTD, the evidence, and how it was routed.
72Close the section with the review run context (`run_id`, `artifact_path`). Hand the section to step 9 as PR-description context; `references/shipping.md` says where step 9 passes it into `ce-commit-push-pr`. Step 9 writes the body, so nothing here edits the PR directly, and nothing posts a PR comment for these.
74**When no PR will exist (no remote):** load `references/tracker-defer.md` in **non-interactive mode** with the same items, collect `{ filed: [...], failed: [...], no_sink: [...] }`, and state every `failed` and `no_sink` item verbatim in the DONE report. The report is the only record those get.

Step 6 — residual handoff

Residuals are the findings left over: actionable findings not applied in step 5. They are not leftovers from an autofix inside ce-code-review. Use the Actionable Findings summary / artifact from step 4.

Two further triggers also require step 6, both outside the apply path: step 4 emitted any settled_conflict-stamped findings, or step 2's return carried proceeded-and-flagged settled_decision_conflicts entries. These are the findings where the work diverged from a settled decision, and this step is where they get written down somewhere that lasts.

A residual at this point is undecided, not accepted debt. Step 5 declined it because it needs judgment, and the pipeline does not merge unless the user granted it, so the human reviewing the PR supplies that judgment: fix it in this branch, dismiss it, or file it to carry past merge. The record therefore goes where that reviewer already looks, the PR body. The pipeline files no tickets for these; one ticket per finding, decided by nobody, is how a run of small nits floods a tracker.

When a PR will exist (a remote is configured): compose a ## Unapplied review findings section, one checkbox bullet per item so a human ticks it when they close it:

  • For each unapplied actionable finding: - [ ] <severity> — <file:line> — <title>, plus the reviewer's suggested_fix on the next line when present.
  • For each settled_conflict-stamped finding from step 4: the same bullet plus the conflicting KTD the stamp names — included even though the finding is report-only.
  • For each proceeded-and-flagged settled_decision_conflicts entry from step 2: a bullet with the KTD, the evidence, and how it was routed.

Close the section with the review run context (run_id, artifact_path). Hand the section to step 9 as PR-description context; references/shipping.md says where step 9 passes it into ce-commit-push-pr. Step 9 writes the body, so nothing here edits the PR directly, and nothing posts a PR comment for these.

When no PR will exist (no remote): load references/tracker-defer.md in non-interactive mode with the same items, collect { filed: [...], failed: [...], no_sink: [...] }, and state every failed and no_sink item verbatim in the DONE report. The report is the only record those get.

Raw lines 58 to 75
Names a skill ce-code-review :60
Names a skill ce-commit-push-pr :72
76## Step 7 — compound before shipping
78Invoke `ce-compound` with `mode:non-interactive` when the run produced durable reasoning that the final code, tests, and plan or diagnosis do not carry, and losing it would plausibly cause recurrence or substantial rediscovery. The plan or `root_cause`, the review residuals, and the fixes applied in step 5 are the evidence for that judgment. `ce-compound` writes into the repository's tracked learnings store on the branch and asks nothing; `Documentation skipped` is a successful outcome, not a stop. Step 9 commits and pushes whatever it wrote with the rest of the change, so the learning is in the PR at the moment it opens and CI runs against a head that contains it. Do not run it after the babysit result: a push after "CI decided" leaves an unwatched head.

Step 7 — compound before shipping

Invoke ce-compound with mode:non-interactive when the run produced durable reasoning that the final code, tests, and plan or diagnosis do not carry, and losing it would plausibly cause recurrence or substantial rediscovery. The plan or root_cause, the review residuals, and the fixes applied in step 5 are the evidence for that judgment. ce-compound writes into the repository's tracked learnings store on the branch and asks nothing; Documentation skipped is a successful outcome, not a stop. Step 9 commits and pushes whatever it wrote with the rest of the change, so the learning is in the PR at the moment it opens and CI runs against a head that contains it. Do not run it after the babysit result: a push after "CI decided" leaves an unwatched head.

Raw lines 76 to 78
Names a skill ce-compound :78