feat: reduction ladder + cleanup-audit skill (clean-design-by-default) - #170
Conversation
…default agent prompt Grafts the one missing operational reflex — the reduction ladder — into §6 of system.md so the agent defaults to stdlib/native/existing-dependency solutions before writing new code. Also extends the Quality defaults comment rule to allow deliberate-simplification ceiling comments (naming the ceiling and upgrade path), and mirrors the full ladder as a bullet in best_practices.md (/best-practices profile). Two new test assertions pin the ladder so it cannot be silently removed.
Adds the cleanup-audit bundled skill — a read-only, repo-wide pass that ranks accidental complexity (dead code, hand-rolled stdlib equivalents, YAGNI abstractions, shrinkable logic) with no fixes applied. This is the whole-repo complement to `pythinker review diff --mode deslopify`, which is diff-scoped only. Updates the PyInstaller datas manifest and regenerates the wire-protocol slash-command snapshots to include skill:cleanup-audit. CHANGELOG updated with both Task 1 (reduction ladder) and Task 2 (cleanup-audit) bullets.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new Changescleanup-audit skill and reduction ladder guidance
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pythinker_code/agents/default/system.md (1)
146-155:⚠️ Potential issue | 🟡 MinorMirror missing rung 4 guidance into best_practices.md.
system.mdline 152 andbest_practices.mdline 33 describe the reduction ladder inconsistently:
- Rung 4 context gap:
system.mdexplicitly states "never add a new dependency for what a few lines cover";best_practices.mdomits this constraint. This is load-bearing guidance about the zero-new-bundled-deps policy.- Tiebreaker missing:
system.mdincludes "When two rungs both hold, take the higher one and move on — the ladder is a reflex, not a research project";best_practices.mdhas no equivalent, leaving ambiguity about rung priority.- Example detail:
system.mdprovides concrete examples (e.g., "a built-in form control over a picker library");best_practices.mdis more abstract.Update
best_practices.mdto preserve the rung 4 warning and tiebreaker logic so the agent reads consistent policy from both sources.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pythinker_code/agents/default/system.md` around lines 146 - 155, The reduction ladder guidance in best_practices.md is missing critical constraints that are present in system.md. Specifically, best_practices.md omits the rung 4 warning about never adding new dependencies for what a few lines of code can cover, and lacks the tiebreaker rule stating "when two rungs both hold, take the higher one and move on — the ladder is a reflex, not a research project." Update the reduction ladder section in best_practices.md to include both of these elements from system.md, preserving the exact constraint about the zero-new-bundled-deps policy and the priority rule for ambiguous cases, so both documents provide consistent guidance.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/pythinker_code/agents/default/system.md`:
- Around line 146-155: The reduction ladder guidance in best_practices.md is
missing critical constraints that are present in system.md. Specifically,
best_practices.md omits the rung 4 warning about never adding new dependencies
for what a few lines of code can cover, and lacks the tiebreaker rule stating
"when two rungs both hold, take the higher one and move on — the ladder is a
reflex, not a research project." Update the reduction ladder section in
best_practices.md to include both of these elements from system.md, preserving
the exact constraint about the zero-new-bundled-deps policy and the priority
rule for ambiguous cases, so both documents provide consistent guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f0c943c6-961a-40e2-a438-ceef8f4678e0
📒 Files selected for processing (7)
CHANGELOG.mdsrc/pythinker_code/agents/default/system.mdsrc/pythinker_code/prompts/best_practices.mdsrc/pythinker_code/skills/cleanup-audit/SKILL.mdtests/core/test_default_agent.pytests/utils/test_pyinstaller_utils.pytests_e2e/test_wire_protocol.py
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…warning + tiebreaker)
|
CodeRabbit's single finding — mirror the rung-4 dependency warning + tiebreaker from |
What
Make the agent produce minimal-yet-robust code by default. Honest framing: the agent soul already
encodes most of this — §6 "Simplicity first" (minimalism), §6 "Production guardrails" (failure-first
defensive patterns), §2 + Security hygiene (fail-closed). This PR adds only the genuine delta.
Two commits:
feat(prompt): an explicit ordered reduction ladder (YAGNI → stdlib → native/framework →installed dep → one line → minimum code) in
agents/default/system.md§6 and the/best-practicesprofile, plus deliberate-shortcut ceiling-comment guidance folded into §6's existing allowed-comments
clause. Two prompt pins guard it against silent drift.
feat(skills): a new read-onlycleanup-auditskill — a one-shot, whole-repo over-engineeringaudit (ranked delete/simplify/replace-with-stdlib list). It is the repo-wide complement to the
existing diff-scoped
pythinker review diff --mode deslopify, and forbids cutting validation,data-loss handling, security, or accessibility.
Out of scope (deliberately not built)
be redundant here.
deslopify(diff-scoped over-engineering review already ships).Verification
make check-pythinker-code→ All checks passed (ruff + format + pyright + ty)make test-pythinker-code→ 65 passed, 4 skipped (real-LLM only)test_default_agent,test_pyinstaller_utils,test_wire_protocol→ 17 passed## Unreleasedchangelog entries added for both changes.Notes
unverified here. The honest proof would be a diff-LOC A/B on a feature task (agent with vs. without
the ladder) — out of scope for this PR.
plugin-test fixtures named after an external tool; they could be de-branded in a separate surgical
pass. Left untouched here to keep this PR scoped.
Summary by CodeRabbit
New Features
cleanup-auditskill to perform a whole-repository, read-only scan to flag accidental complexity and over-engineering for safe cleanup opportunities.Documentation
Tests
cleanup-auditskill and verify prompt invariants.