Skip to content

feat: Add scope split, followups stage, and detection standing checks#11

Open
jirispilka wants to merge 2 commits into
shepherd-skill-slop-checksfrom
claude/review-tool-call-error-mapper-dqujxm
Open

feat: Add scope split, followups stage, and detection standing checks#11
jirispilka wants to merge 2 commits into
shepherd-skill-slop-checksfrom
claude/review-tool-call-error-mapper-dqujxm

Conversation

@jirispilka

@jirispilka jirispilka commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

What we're solving

Mined human review feedback across recent apify-mcp-server PRs (#1081, #1094, #1077, #1076, #1073, #1089, #1080): shepherd runs keep collecting the same finding classes despite passing the panel — untested new code arms (one shipped a data-corruption bug, #1094), undisclosed behavior deltas, convention/layering drift, and total silence about adjacent debt and scope boundaries. One run's own review scope even instructed reviewers to ignore carried-over defects — which the human reviewer then flagged.

How

  • Detection: standing checks gain new-arm test coverage (both sides + the old path's input classes), undisclosed-delta disclosure (incl. silently inverted helper defaults), and conventions/layering; per-lens detail goes into the base-registry reviewer scopes (uncapped), not orchestrator prose.
  • Scope: the design template gains ## Scope split (This PR / Prerequisite refactor / Follow-ups); a non-empty prerequisite is an explicit human gate decision. Reviewers tag adjacent pre-existing defects pre-existing instead of silencing them; a new followups stage (dispatched with fulfillment — file contract, not orchestrator memory, so a weak orchestrator model can't drop it) compiles them into a ledger the human dispositions at the create-PR gate: fix-here | issue | pr-note | drop, issues only on human approval. A hard rule forbids scopes from suppressing reporting.
  • Claims: PR-body numbers and references must match the final oracle run and repo state.

Line cap 450 → 500 (now 482). Contract tests pin the prose; 97 pass.

Stacked on #6-followup branch shepherd-skill-slop-checks (PR #7) — merge that first; this PR's base is that branch, so the diff shows only this change. Part of the same effort as #9 (refiner stage) and #10 (test-coverage lens).

Field test (2026-07-18)

This branch ran a full production run end-to-end: the apify-mcp-server #1084 task-machinery refactor — triage → verify → design (6 human decisions) → design gate → 3 inner iterations → final review → fulfillment → create-PR gate → two stacked PRs shipped (apify/apify-mcp-server#1105, #1106). 24 stage dispatches, ~2.36M subagent tokens. Per-feature verdicts, each grounded in the run's _progress.md ledger:

  • Conventions/layering standing check: caught a real major — the CONTRIBUTING.md >3-params rule broken by two extracted signatures (exactly the mined finding class; under the old prose this ships). The finding forced a design amendment and spawned a CI-enforcement follow-up issue (refactor: Convert >3-positional-param functions to object params apify-mcp-server#1111). New-arm and undisclosed-delta checks were unexercised (pure-move diff, no new conditionals) — no signal either way.
  • pre-existing tag + followups ledger: worked end-to-end, and the gate is live — 4 adjacent defects surfaced across 3 review rounds without blocking convergence; the human's dispositions diverged from the stage's recommendations (3× fix-here vs pr-note/drop), reopening the loop; zero issues auto-created.
  • Scope split: representation worked, detection missed — the architect declared a "refactor first, then move" plan as internal commit sequencing ("Prerequisite refactor: none") despite the repo's refactor-lands-first policy; the human had to force the split. Fixed in the follow-up commit.
  • Claims rule: complied — both shipped PR bodies carry the final oracle's exact numbers.
  • Suppression ban: held — pure-move scopes downgraded/routed, never silenced; all pre-existing items reported every round.

Second commit (80b4c2f) folds the field-test lessons back in: human-picked light re-review for trivial gate-authorized fixes; prerequisite classification must consult the repo's refactor-separation policy; the conventions doc settles style questions before they reach the human; delta-mode verify on same-base re-runs; lens-fit panel nudge at every gate (the orchestrator never edits the panel itself). Related follow-up out of scope here: #15 (local commits on a run scratch branch; gate remote writes only).

Alternatives considered

  • Orchestrator prose instead of a followups stage — rejected: compiling findings across iterations needs memory and judgment, exactly what a weak orchestrator drops; a dispatched stage re-reads the persisted review files fresh.
  • A comment-analyzer reviewer lens — rejected as redundant: comment-truth standing check + PR feat: Tighten reviewer standing checks, design Q&A, and PR body rules #7's slop checks already cover it.

🤖 Generated with Claude Code

https://claude.ai/code/session_017ZmSKShwVcKG2P7MY22X8k


Generated by Claude Code

…ing checks

Mined review feedback across apify-mcp-server PRs shows shepherd's runs keep
collecting the same human findings: untested new code arms (one shipped data
bug), undisclosed behavior deltas, convention/layering drift, and silence
about adjacent debt. Three additions:

- Standing checks gain new-arm test coverage, undisclosed-delta disclosure,
  and conventions/layering; reviewer scopes in the base registry carry the
  per-lens detail.
- The design template gains a Scope split (This PR / Prerequisite refactor /
  Follow-ups); a non-empty prerequisite is an explicit gate decision.
- Reviewers tag adjacent pre-existing defects instead of silencing them; a
  new followups stage compiles them with the scope-split leftovers into a
  ledger the human dispositions at the create-PR gate (fix-here / issue /
  pr-note / drop) — issues only on human approval. PR-body numbers and
  references must match the final oracle run.

Line cap raised 450 -> 500 (now 462). Contract tests pin the new prose.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ZmSKShwVcKG2P7MY22X8k
Five refinements from a live run (apify-mcp-server #1084 stack):
human-picked light re-review for trivial gate-authorized fixes;
prerequisite-refactor classification must consult the repo's
refactor-separation policy; conventions doc settles style questions
before they reach the human; delta-mode verify on same-base re-runs;
lens-fit panel nudge at every human gate (no orchestrator auto-edits).
Contract tests pin the new prose. SKILL.md 462 -> 482 lines (cap 500).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018MfTtEvQ9MYhydr8qSUopw
@jirispilka jirispilka self-assigned this Jul 19, 2026
@jirispilka
jirispilka marked this pull request as ready for review July 19, 2026 20:28
@jirispilka
jirispilka requested a review from Pijukatel July 19, 2026 20:28
code they describe are findings; also flag AI-slop — abnormal defensive try/catch (defensive
code at trust boundaries is fine), type-escape casts (`any` or equivalent), deep nesting that
should be early returns, and other patterns inconsistent with the surrounding file.
Three more: a conditional branch or guard the diff adds must be test-exercised on both sides,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three more: is somewhat strange here. I am not sure what is meant by it

adversarial vs live-probe vs contract/consumer). **Resolve every `"auto"` model to a concrete name** (see Model
adversarial vs live-probe vs contract/consumer). At every human gate, present the panel with a
one-line lens-fit assessment per reviewer — is the lens live for this design, or structurally
muted by it (e.g. a complexity/deletion lens on a behavior-preserving move that forbids cuts)?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instruction with a question mark at the end?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants