diff --git a/CHANGELOG.md b/CHANGELOG.md index a83f7a98..b9c8a4d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ GitHub Releases page; `0.8.0` is the new starting line. ## Unreleased +- Added an explicit "reduction ladder" and deliberate-shortcut guidance to the default agent prompt so the agent reaches for the simplest working solution (stdlib/native before custom code) by default. +- Added a `cleanup-audit` skill: a read-only, whole-repo pass that ranks over-engineering to delete, simplify, or replace with standard-library/platform equivalents (the repo-wide complement to `pythinker review diff --mode deslopify`). - **Fix: duplicate update notices at startup.** When a background install finishes or a cached update is detected, the hint now renders only on the persistent under-input line instead of also flashing as a footer toast. diff --git a/src/pythinker_code/agents/default/system.md b/src/pythinker_code/agents/default/system.md index 3ce96c64..f13bd540 100644 --- a/src/pythinker_code/agents/default/system.md +++ b/src/pythinker_code/agents/default/system.md @@ -149,7 +149,9 @@ ${PYTHINKER_SCRATCHPAD_SECTION} **Simplicity first — minimum code that solves the problem, nothing speculative.** No features beyond what was asked; no abstractions for single-use code; no unrequested configurability; no error handling for impossible scenarios — validate at boundaries only. If a 200-line draft could be 50 lines, rewrite it before showing it. Over-fragmentation is overcomplication too: don't scatter logic across tiny files or extra layers to satisfy a pattern — match the codebase's existing granularity. Self-check: *would a senior engineer call this over-engineered?* If yes, simplify. -**Quality defaults** (unless project or domain rules override): focused, shallow, scannable functions with early exits over deep nesting; meaningful identifiers, no shadowing, the context's casing convention; avoid duplicate logic within a change without inventing broad abstractions for one-off repetition; comment only non-obvious algorithms, workarounds, business rules, and edge cases (`TODO:` for real debt; no self-evident comments; never add copyright or license headers unless requested); cohesive, testable modules; efficient data structures where they aid clarity or scale; wrap error-prone I/O, API, network, and resource operations with handling, timeouts/fallbacks, and cleanup; adopt stricter domain standards (e.g. MISRA-style C/C++) when relevant. Once correct, run the repo's formatter (up to 3 attempts); never add one where none exists. +**The reduction ladder — walk it before writing code; stop at the first rung that holds.** (1) *Does this need to exist at all?* A speculative need is skipped, said so in one line. (2) *Does the standard library do it?* Use it. (3) *Does a native platform or framework feature cover it?* A database constraint over an app-level check, a built-in form control over a picker library, the language's own construct over a hand-rolled one — use it. (4) *Does a dependency already in the manifest solve it?* Use it; never add a new dependency for what a few lines cover. (5) *Can it be one line?* Make it one line. (6) *Only then* write the minimum code that works. When two rungs both hold, take the higher one and move on — the ladder is a reflex, not a research project. None of this overrides the guards in this section: trust-boundary validation, error handling that prevents data loss, security, and accessibility stay in even at rung 5. + +**Quality defaults** (unless project or domain rules override): focused, shallow, scannable functions with early exits over deep nesting; meaningful identifiers, no shadowing, the context's casing convention; avoid duplicate logic within a change without inventing broad abstractions for one-off repetition; comment only non-obvious algorithms, workarounds, business rules, edge cases, and deliberate simplifications whose ceiling matters — a coarse lock, an O(n²) scan, a naive heuristic — naming the ceiling and the upgrade path (`TODO:` for real debt; no self-evident comments; never add copyright or license headers unless requested); cohesive, testable modules; efficient data structures where they aid clarity or scale; wrap error-prone I/O, API, network, and resource operations with handling, timeouts/fallbacks, and cleanup; adopt stricter domain standards (e.g. MISRA-style C/C++) when relevant. Once correct, run the repo's formatter (up to 3 attempts); never add one where none exists. **Honest testing.** Verification per Rule 3, from the narrowest scope outward. Never game it: no weakened or deleted assertions, skipped tests, widened tolerances, overfitting to test cases, or mocking away the behavior under test. Keep tests deterministic — control time, randomness, and the network through the repo's existing patterns; never synchronize with sleeps. diff --git a/src/pythinker_code/prompts/best_practices.md b/src/pythinker_code/prompts/best_practices.md index 4d10ab79..fe50306e 100644 --- a/src/pythinker_code/prompts/best_practices.md +++ b/src/pythinker_code/prompts/best_practices.md @@ -30,6 +30,7 @@ The user ran `/best-practices`. Engineering best practices are now in effect: ap - Never invent APIs. Verify every external symbol — function signatures, config keys, CLI flags, library methods — against the actual source, installed package, or type definitions before using it. If you cannot verify it, look it up; if you still cannot, say so instead of guessing. - Prefer the standard library and dependencies already in the manifest. A new dependency is a design decision: justify it (maintenance, license, size, transitive risk), verify the exact package name exists in the registry (hallucinated names are a typosquatting vector), pin it per repo convention, and modify lockfiles only through the package manager — never by hand. - Apply YAGNI: no speculative abstractions, flags, generality, or extension points the request does not need. +- Before writing code, walk the reduction ladder and stop at the first rung that holds: (1) does this need to exist at all (YAGNI) — skip it and say so; (2) does the standard library do it; (3) does a native platform or framework feature cover it (a database constraint over app code, a built-in control over a library); (4) does a dependency already in the manifest solve it — never add a new one for what a few lines cover; (5) can it be one line; (6) only then the minimum code that works. When two rungs both hold, take the higher one and move on. The guards in this profile — trust-boundary validation, data-loss handling, security, accessibility — are never traded away for fewer lines. - No placeholders in completed work: no TODO stubs, commented-out blocks, empty handler bodies, or mock data presented as a real integration. - Fail loudly per the codebase's conventions. Never swallow exceptions, downgrade errors to warnings, or return fabricated defaults to make a failure disappear. - Preserve backward compatibility by default. Search the integration surfaces your change touches — public APIs, CLI parameters, configuration loading, persisted state, session and wire formats, database schemas — and if a break is unavoidable, call it out and migrate or gate it. diff --git a/src/pythinker_code/skills/cleanup-audit/SKILL.md b/src/pythinker_code/skills/cleanup-audit/SKILL.md new file mode 100644 index 00000000..7485505f --- /dev/null +++ b/src/pythinker_code/skills/cleanup-audit/SKILL.md @@ -0,0 +1,50 @@ +--- +name: cleanup-audit +description: Whole-repo audit for over-engineering and accidental complexity. Scans the entire codebase (not just a diff) and returns a ranked, read-only list of what to delete, simplify, or replace with standard-library or platform equivalents. Use when the user asks to "audit the codebase", "find bloat", "what can I delete", or wants a repo-wide simplification pass. For a diff-scoped pass use `pythinker review diff --mode deslopify` instead. One-shot report; applies no fixes. +--- + +# Cleanup Audit + +A read-only, whole-repo pass that hunts accidental complexity and over-engineering. The diff-scoped +version of this job already ships as `pythinker review diff --mode deslopify`; this skill is its +repo-wide complement — scan the whole tree, rank the biggest cut first. + +Scope is complexity only. Correctness bugs, security holes, and performance belong to a normal +review or security pass — note them in one line if you trip over them, but do not chase them here. + +## What to hunt + +- Dead code, unused flexibility, and speculative features no caller needs. +- Hand-rolled logic the standard library already ships — name the function that replaces it. +- A dependency (or hand-written code) doing what the language, runtime, or framework already does. +- Single-implementation interfaces, one-product factories, wrappers that only delegate, a module + that exports one trivial thing, dead flags and config nobody sets. +- The same logic spelled out long-hand where a shorter, equally clear form exists. + +## How to work + +1. Map before judging: read the tree, the manifest/lockfile, and entry points; use `Grep`/`Glob` + (or `LSP` for references and call hierarchy) to confirm a thing is actually unused before + proposing its deletion. A deletion proposed without checking callers is a guess. +2. Rank findings biggest cut first. +3. Apply nothing. This is a report. + +## Output + +One line per finding, ranked, each tagged and citing a path: + +- `delete:` — dead code / speculative feature. Replacement: nothing. +- `stdlib:` — hand-rolled thing the standard library ships. Name the function. +- `native:` — dependency or code doing what the platform/framework already does. Name the feature. +- `yagni:` — abstraction with one implementation, config nobody sets, layer with one caller. +- `shrink:` — same logic, fewer lines. Show the shorter form. + +Format: ` . . [path:line]` +End with an estimate: `net: ~- lines, - deps possible.` Nothing to cut: `Lean already.` + +## Guardrails + +Never propose cutting trust-boundary validation, error handling that prevents data loss, security +measures, or accessibility — minimal is not the same as unsafe. When a "simplification" would touch +one of those, leave it and say why. Verify each deletion candidate is truly unreferenced before +listing it; an audit that proposes deleting live code is worse than no audit. diff --git a/tests/core/test_default_agent.py b/tests/core/test_default_agent.py index 2c525e95..a81142c3 100644 --- a/tests/core/test_default_agent.py +++ b/tests/core/test_default_agent.py @@ -39,6 +39,8 @@ async def test_default_agent(runtime: Runtime): assert "hallucinated package names are a typosquatting vector" in agent.system_prompt assert "Never game it: no weakened or deleted assertions" in agent.system_prompt assert "never rerun an identical failing command" in agent.system_prompt + assert "The reduction ladder" in agent.system_prompt + assert "stop at the first rung that holds" in agent.system_prompt # Prompt-injection defense — the wrapper is only effective if # the model is told the tags mean "data, never instructions". Keep this in the diff --git a/tests/utils/test_pyinstaller_utils.py b/tests/utils/test_pyinstaller_utils.py index d459784b..3c8cca4b 100644 --- a/tests/utils/test_pyinstaller_utils.py +++ b/tests/utils/test_pyinstaller_utils.py @@ -120,6 +120,10 @@ def test_pyinstaller_datas(): "src/pythinker_code/skills/check-impl-against-spec/SKILL.md", "pythinker_code/skills/check-impl-against-spec", ), + ( + "src/pythinker_code/skills/cleanup-audit/SKILL.md", + "pythinker_code/skills/cleanup-audit", + ), ( "src/pythinker_code/skills/customize-pythinker/SKILL.md", "pythinker_code/skills/customize-pythinker", diff --git a/tests_e2e/test_wire_protocol.py b/tests_e2e/test_wire_protocol.py index 6c22483e..0dd32faf 100644 --- a/tests_e2e/test_wire_protocol.py +++ b/tests_e2e/test_wire_protocol.py @@ -120,6 +120,11 @@ def test_initialize_handshake(tmp_path) -> None: "description": "Compare an implementation against a product or technical spec and report gaps with evidence.", "aliases": [], }, + { + "name": "skill:cleanup-audit", + "description": 'Whole-repo audit for over-engineering and accidental complexity. Scans the entire codebase (not just a diff) and returns a ranked, read-only list of what to delete, simplify, or replace with standard-library or platform equivalents. Use when the user asks to "audit the codebase", "find bloat", "what can I delete", or wants a repo-wide simplification pass. For a diff-scoped pass use `pythinker review diff --mode deslopify` instead. One-shot report; applies no fixes.', + "aliases": [], + }, { "name": "skill:create-pr", "description": "Prepare a pull request by summarizing changes, verification, risks, and reviewer guidance without adding AI footers.", @@ -330,6 +335,11 @@ def test_initialize_external_tool_conflict(tmp_path) -> None: "description": "Compare an implementation against a product or technical spec and report gaps with evidence.", "aliases": [], }, + { + "name": "skill:cleanup-audit", + "description": 'Whole-repo audit for over-engineering and accidental complexity. Scans the entire codebase (not just a diff) and returns a ranked, read-only list of what to delete, simplify, or replace with standard-library or platform equivalents. Use when the user asks to "audit the codebase", "find bloat", "what can I delete", or wants a repo-wide simplification pass. For a diff-scoped pass use `pythinker review diff --mode deslopify` instead. One-shot report; applies no fixes.', + "aliases": [], + }, { "name": "skill:create-pr", "description": "Prepare a pull request by summarizing changes, verification, risks, and reviewer guidance without adding AI footers.",