Skip to content

Encode review practices mined from apify-core/worker/crawlee PR feedback#13

Draft
jirispilka wants to merge 1 commit into
mainfrom
claude/shepherd-engineering-practices-2chpb1
Draft

Encode review practices mined from apify-core/worker/crawlee PR feedback#13
jirispilka wants to merge 1 commit into
mainfrom
claude/shepherd-engineering-practices-2chpb1

Conversation

@jirispilka

Copy link
Copy Markdown
Collaborator

What we're solving

Shepherd's pipeline misses several finding classes that human reviewers raise over and over in the repos it serves. We mined 56 discussion-heavy PRs (~790 review threads) across apify-core, apify-worker, crawlee, and crawlee-python, cross-referenced everything against this repo's open work (#7, #11, and issues #9/#10) to avoid duplication, and folded in published loop-design guidance (LLM-judge self-preference bias, evidence-grounded critique). This PR encodes only the gaps not already covered by open PRs/issues.

How

  • Standing checks grow 3 → 6:
    • Evidence-grounded findings — every finding carries a checked file:line/command/doc; unverifiable suspicion is a question, not a finding; identifiers the diff relies on must exist (crawlee #3331 shipped hallucinated env vars).
    • Public-surface minimality — a new export/option/env var with no consumer in the diff needs a visibility reason (@internal, private); ~25 mined threads.
    • Doc sync — adding/renaming/removing a public symbol, option, or env var must update every doc that teaches it, in the same diff; ~30 mined threads, crawlee's biggest category.
  • Implementer: diff stays scoped to the design — incidental hunks are reverted or get a one-line rationale in claim.md; non-obvious mechanisms, suppressions, and magic constants get why-comments (the single largest mined category, ~70 threads across all four repos).
  • Architect: search before inventing — the design names the existing helpers/patterns it builds on; multi-step writes state failure ordering under Risks (crash mid-way, concurrent writer, fallback path).
  • Model tiering: the review panel never resolves entirely onto the implementer's model (self-preference bias; arXiv 2404.18796, 2410.21819).
  • Finish: PR body scales with the diff — a large or subtle change states its mechanism and known limitations (thin bodies on big diffs measurably generate review load: apify-worker #1829, crawlee-python #1339).

Each rule is pinned by a contract test; 92 tests pass; SKILL.md at 444/450 lines.

Alternatives considered

Notes

🤖 Generated with Claude Code

https://claude.ai/code/session_016G4uCkx2btpqfGtJRAPJKd


Generated by Claude Code

Mined ~790 review threads across 56 discussion-heavy PRs in apify-core,
apify-worker, crawlee, and crawlee-python, cross-referenced against this
repo's open issues/PRs (#7-#12) to avoid duplication, and folded in
published loop-design guidance (judge self-preference bias, evidence-
grounded critique). New rules cover only gaps not already raised:

- Standing checks grow from three to six: evidence-grounded findings
  (unverifiable suspicion is a question, not a finding; referenced
  identifiers must exist), public-surface minimality (new exports need
  a consumer or a visibility reason), and doc sync for public changes
  (README/guides/upgrading notes updated in the same diff).
- Implementer: diff stays scoped to the design (incidental hunks are
  reverted or get a one-line rationale in claim.md); non-obvious
  mechanisms, suppressions, and magic constants get why-comments.
- Architect: search before inventing (name the existing helpers the
  design builds on); multi-step writes state failure ordering in Risks.
- Model tiering: the review panel never resolves entirely onto the
  implementer's model (self-preference bias).
- Finish: PR body scales with the diff — mechanism and known
  limitations for large changes, one-line whys for incidental ones.

Each rule is pinned by a contract test; SKILL.md stays at 444/450 lines.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016G4uCkx2btpqfGtJRAPJKd
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.

3 participants