diff --git a/README.md b/README.md index ded2307..3eb29d9 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,7 @@ pnpm link --global clawpatch init clawpatch map clawpatch review --limit 3 --jobs 3 +clawpatch review --mode deslopify --limit 3 clawpatch report clawpatch next clawpatch show --finding @@ -109,6 +110,7 @@ Supported provider names today: - `clawpatch map`: write feature records - `clawpatch status`: show project, dirty state, feature/finding counts - `clawpatch review`: review pending or selected features +- `clawpatch review --mode deslopify`: review only for locally provable slop cleanup - `clawpatch report`: print or write a Markdown findings report - `clawpatch next`: print the next actionable finding - `clawpatch show --finding `: inspect one finding with evidence and suggested validation diff --git a/docs/code-review.md b/docs/code-review.md index 7013972..c5c3c70 100644 --- a/docs/code-review.md +++ b/docs/code-review.md @@ -12,6 +12,7 @@ clawpatch review --limit 3 clawpatch review --limit 12 --jobs 4 clawpatch review --feature clawpatch review --since origin/main +clawpatch review --mode deslopify --limit 3 clawpatch review --provider codex --model ``` @@ -56,6 +57,37 @@ count as `lockFiles`. There is no multi-provider panel yet. +### --mode deslopify + +Use deslopify mode when you want one narrow lane for simplifying code and +improving performance by removing code slop. It restricts findings to +maintainability or performance issues caused by accidental complexity, +inefficient indirection, semantic duplication, needless wrappers, dead code, or +avoidable repeated work. It should not report unrelated correctness, security, +API contract, data-loss, or build-release issues; provider findings outside +maintainability and performance are discarded in this mode. + +The deslopify rubric is intentionally narrow. It asks the provider to prioritize +locally provable slop patterns where the likely fix is deletion, consolidation, +or reuse of an existing local pattern: + +- semantic duplication across files, tests, CLIs, SQL queries, adapters, wrappers, + or generated-looking utilities +- shadow modules and thin pass-through wrappers +- concrete code bloat: generated-looking mass, production-included test/debug/demo + artifacts, wrapper swarms, duplicated boilerplate, or manual registries that + duplicate a source of truth +- dead legacy paths kept alive by tests +- cargo-cult defensive code that does not match a real trust boundary +- tautological or coupled tests that preserve implementation internals instead of + behavior +- type/build silencing and band-aid hacks such as broad disables, `any`, + `type-ignore`, sleeps/timeouts, path mutation, fake success returns, or removed + checks, when simplification is the fix + +It should not report file size, explicit generated files, normal framework +boilerplate, or domain modules that merely look large. + Categories requested from the provider: - `bug` diff --git a/src/app.ts b/src/app.ts index 2e923c5..93416e1 100644 --- a/src/app.ts +++ b/src/app.ts @@ -24,6 +24,7 @@ import { mapFeatures } from "./mapper.js"; import { emitProgress } from "./progress.js"; import { providerByName } from "./provider.js"; import { buildFixPrompt, buildReviewPrompt, buildRevalidatePrompt } from "./prompt.js"; +import type { ReviewMode } from "./prompt.js"; import { evidenceLabel, findingSummaries, @@ -66,6 +67,7 @@ import { FixPlanOutput, FindingRecord, PatchAttempt, + ReviewOutput, RunRecord, reasoningEffortSchema, reasoningEfforts, @@ -231,6 +233,7 @@ export async function reviewCommand( const loaded = await loadProjectState(context); const config = applyProviderFlags(loaded.config, flags); const provider = providerByName(config.provider.name); + const mode = reviewMode(flags); const features = await selectReviewFeatures(loaded, flags); if (features.length === 0 && typeof flags["since"] === "string") { return { next: "no features touched by diff" }; @@ -239,6 +242,7 @@ export async function reviewCommand( return { dryRun: true, wouldReview: features.length, + mode, jobs: reviewJobs(flags), featureIds: features.map((feature) => feature.featureId), }; @@ -276,6 +280,7 @@ export async function reviewCommand( currentRunId, index, total: features.length, + mode, allowNonPendingFeatureReview: stringFlag(flags, "feature") !== undefined, }); findingIds.push(...reviewed.findingIds); @@ -483,6 +488,7 @@ type ReviewFeatureOptions = { currentRunId: string; index: number; total: number; + mode: ReviewMode; allowNonPendingFeatureReview: boolean; }; @@ -496,6 +502,7 @@ async function reviewFeature(options: ReviewFeatureOptions): Promise<{ findingId currentRunId, index, total, + mode, allowNonPendingFeatureReview, } = options; const started = Date.now(); @@ -516,9 +523,16 @@ async function reviewFeature(options: ReviewFeatureOptions): Promise<{ findingId }, ); locked = lockedFeature; - const prompt = await buildReviewPrompt(loaded.root, loaded.project, lockedFeature, config); + const prompt = await buildReviewPrompt( + loaded.root, + loaded.project, + lockedFeature, + config, + mode, + ); const output = await provider.review(loaded.root, prompt, providerOptions(config)); - const records = output.findings + const modeFindings = reviewFindingsForMode(output.findings, mode); + const records = modeFindings .slice(0, config.review.maxFindingsPerFeature) .map((finding) => findingFromOutput(finding, lockedFeature.featureId, currentRunId)); const findingIds: string[] = []; @@ -1054,6 +1068,25 @@ function reviewJobs(flags: Record): number { return Math.min(Math.floor(parsed), 32); } +function reviewMode(flags: Record): ReviewMode { + const mode = stringFlag(flags, "mode") ?? "default"; + if (mode === "default" || mode === "deslopify") { + return mode; + } + throw new ClawpatchError("invalid --mode; expected default or deslopify", 2, "invalid-usage"); +} + +function reviewFindingsForMode( + findings: ReviewOutput["findings"], + mode: ReviewMode, +): ReviewOutput["findings"] { + if (mode !== "deslopify") { + return findings; + } + return findings.filter( + (finding) => finding.category === "maintainability" || finding.category === "performance", + ); +} function featureLock(currentRunId: string): NonNullable { return { lockedByRunId: currentRunId, diff --git a/src/cli.ts b/src/cli.ts index 289d1b0..a8f363e 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -154,6 +154,7 @@ const commandFlags = { "limit", "since", "jobs", + "mode", "provider", "model", "reasoningEffort", @@ -197,6 +198,7 @@ const valueFlagNames = new Set([ "limit", "since", "jobs", + "mode", "source", "provider", "model", @@ -267,6 +269,14 @@ function validateCommandRequirements( ) { throw new ClawpatchError("missing --finding or --all", 2, "invalid-usage"); } + if ( + command === "review" && + typeof flags["mode"] === "string" && + flags["mode"] !== "default" && + flags["mode"] !== "deslopify" + ) { + throw new ClawpatchError("invalid --mode; expected default or deslopify", 2, "invalid-usage"); + } } function isKnownCommand(command: string): command is keyof typeof commandFlags { @@ -370,6 +380,7 @@ Flags: --limit --since --jobs default: 10 + --mode --provider --model --reasoning-effort diff --git a/src/prompt.ts b/src/prompt.ts index df592ee..6c8b68d 100644 --- a/src/prompt.ts +++ b/src/prompt.ts @@ -2,6 +2,8 @@ import { readFile, realpath } from "node:fs/promises"; import { isAbsolute, relative, resolve } from "node:path"; import { ClawpatchConfig, FeatureRecord, FindingRecord, ProjectRecord } from "./types.js"; +export type ReviewMode = "default" | "deslopify"; + export function buildAgentMapPrompt(project: ProjectRecord, inventory: unknown): string { return `You are mapping a repository into semantic clawpatch review slices. @@ -57,6 +59,7 @@ export async function buildReviewPrompt( project: ProjectRecord, feature: FeatureRecord, config: ClawpatchConfig, + mode: ReviewMode = "default", ): Promise { const owned = feature.ownedFiles.slice(0, config.review.maxOwnedFiles); const context = feature.contextFiles.slice(0, config.review.maxContextFiles); @@ -87,6 +90,8 @@ Review categories: - release/build hazards - maintainability risks with concrete impact +${reviewModeInstructions(mode)} + Inspect owned files, context files, and linked tests. Treat included tests as first-class evidence of intended behavior. If tests contradict a suspected bug, either skip it or downgrade confidence and explain the uncertainty. Avoid reporting behavior as a bug @@ -120,6 +125,31 @@ Files: ${fileBlocks.join("\n\n")}`; } +function reviewModeInstructions(mode: ReviewMode): string { + if (mode === "default") { + return ""; + } + if (mode === "deslopify") { + return `Deslopify mode: +- report only simplification findings in category "maintainability" or "performance" +- stay separate from normal review: do not look for general bugs, security issues, API contract problems, or missing edge-case handling +- focus on locally provable AI-slop patterns whose likely fix is deletion, consolidation, or reuse of an existing local pattern +- prioritize semantic duplication: repeated behavior across files, tests, CLIs, SQL queries, adapters, wrappers, or generated-looking utilities +- prioritize shadow modules and useless wrappers: thin layers that pass through to another path without hiding real complexity +- prioritize concrete code bloat: generated-looking mass, production-included test/debug/demo artifacts, wrapper swarms, duplicated boilerplate, or manual registries that duplicate a source of truth +- prioritize dead legacy paths kept alive by tests: obsolete validators, schemas, adapters, compatibility branches, feature flags, or helpers +- prioritize cargo-cult defensive code: broad try/catch, fallback, logging, null guard, or "safe" wrapper code that does not match a real trust boundary +- prioritize tautological or coupled tests: tests that mirror implementation internals, repeat giant fake harnesses, or preserve accidental private structure instead of behavior +- prioritize type/build silencing and band-aid hacks: broad disables, any/type-ignore casts, sleeps/timeouts, path mutation, fake success returns, or removed checks when simplification is the fix +- every finding must have a concrete maintenance or runtime cost in the included files +- prefer deletion, consolidation, or existing local patterns over new abstractions +- do not report file size, explicit generated files, normal framework boilerplate, or domain modules that merely look large +- do not report style taste, naming preference, broad architecture opinions, or speculative cleanup +- do not report correctness, security, API contract, data-loss, or build-release issues unless the root cause is accidental complexity and the minimum fix is simplification`; + } + throw new Error(`Unsupported review mode: ${mode}`); +} + export async function buildRevalidatePrompt(root: string, findingJson: string): Promise { return `Revalidate this clawpatch finding against the current repository at ${root}. diff --git a/src/workflow.test.ts b/src/workflow.test.ts index 5d44583..e0637b6 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -247,6 +247,15 @@ describe("workflow", () => { expect(parseArgs(["review", "--since", "HEAD~5"]).flags).toMatchObject({ since: "HEAD~5", }); + expect(parseArgs(["review", "--mode", "deslopify"]).flags).toMatchObject({ + mode: "deslopify", + }); + expect(() => parseArgs(["review", "--mode", "simplify"])).toThrow( + "invalid --mode; expected default or deslopify", + ); + expect(() => parseArgs(["review", "--mode", "slop"])).toThrow( + "invalid --mode; expected default or deslopify", + ); expect(parseArgs(["revalidate", "--since", "origin/main"]).flags).toMatchObject({ since: "origin/main", }); @@ -2164,6 +2173,79 @@ describe("workflow", () => { delete process.env["CLAWPATCH_PROVIDER"]; }); + it("adds deslopify-only review instructions when requested", async () => { + const root = await fixtureRoot("clawpatch-deslopify-prompt-"); + await writeFixture(root, "package.json", JSON.stringify({ name: "deslopify-prompt" })); + await writeFixture(root, "src/index.ts", "export function main() { return 1; }\n"); + const context = await makeContext(testOptions(root)); + + await initCommand(context, {}); + const project = await readProject(statePaths(join(root, ".clawpatch"))); + expect(project).toBeDefined(); + const prompt = await buildReviewPrompt( + root, + project!, + { + schemaVersion: 1, + featureId: "feat_deslopify", + title: "deslopify", + summary: "deslopify", + kind: "library", + source: "test", + confidence: "high", + entrypoints: [{ path: "src/index.ts", symbol: null, route: null, command: null }], + ownedFiles: [{ path: "src/index.ts", reason: "test" }], + contextFiles: [], + tests: [], + tags: [], + trustBoundaries: [], + status: "pending", + lock: null, + findingIds: [], + patchAttemptIds: [], + analysisHistory: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }, + await loadConfig(root, testOptions(root)), + "deslopify", + ); + + expect(prompt).toContain("Deslopify mode:"); + expect(prompt).toContain( + 'only simplification findings in category "maintainability" or "performance"', + ); + expect(prompt).toContain("stay separate from normal review"); + expect(prompt).toContain("locally provable AI-slop patterns"); + expect(prompt).toContain("semantic duplication"); + expect(prompt).toContain("shadow modules and useless wrappers"); + expect(prompt).toContain("concrete code bloat"); + expect(prompt).toContain("dead legacy paths kept alive by tests"); + expect(prompt).toContain("cargo-cult defensive code"); + expect(prompt).toContain("tautological or coupled tests"); + expect(prompt).toContain("type/build silencing and band-aid hacks"); + expect(prompt).toContain("do not report file size"); + expect(prompt).toContain("do not report correctness, security, API contract"); + }); + + it("filters non-simplification findings in deslopify mode", async () => { + const root = await fixtureRoot("clawpatch-deslopify-filter-"); + await writeFixture(root, "package.json", JSON.stringify({ name: "deslopify-filter" })); + await writeFixture(root, "src/index.ts", "export const value = 'TODO_BUG';\n"); + process.env["CLAWPATCH_PROVIDER"] = "mock"; + const context = await makeContext(testOptions(root)); + + await initCommand(context, {}); + await mapCommand(context); + const reviewed = await reviewCommand(context, { limit: "1", mode: "deslopify" }); + const paths = statePaths(join(root, ".clawpatch")); + const findings = await readFindings(paths); + + expect(reviewed).toMatchObject({ findings: 0 }); + expect(findings).toHaveLength(0); + delete process.env["CLAWPATCH_PROVIDER"]; + }); + it("does not include escaped feature paths in prompts", async () => { const root = await fixtureRoot("clawpatch-path-escape-"); const siblingSecret = join(root, "..", "secret.txt");