initial import: @mikefreno/omp-pygenium (omp port)
This commit is contained in:
0
src/checks/.gitkeep
Normal file
0
src/checks/.gitkeep
Normal file
233
src/checks/comments.ts
Normal file
233
src/checks/comments.ts
Normal file
@@ -0,0 +1,233 @@
|
||||
/**
|
||||
* checks/comments.ts — comments hygiene check (first end-to-end reference).
|
||||
*
|
||||
* This is the canonical `CheckDefinition` that future checks (08+) copy. It
|
||||
* reuses the generic `scanner`/`fixer` agents shipped in `agents/*.md`; the
|
||||
* check-specific rubric is embedded in the task text handed to the sub-agent
|
||||
* (see {@link buildCommentsScanTask} / {@link buildCommentsFixTask}) so no
|
||||
* per-check agent `.md` file is required.
|
||||
*
|
||||
* Rubric (the user's spec — short + high value):
|
||||
* - comments that restate the code they sit on ("what" comments) → REMOVE
|
||||
* - verbose narration / long-winded explanations → TIGHTEN (shorten)
|
||||
* - "why" comments that explain intent, rationale, or gotchas → KEEP
|
||||
* - code self-explanatory with no comment → no comment needed (don't add one)
|
||||
*
|
||||
* Artifacts (under `<cwd>/.pygienium/checks/comments/`):
|
||||
* - `findings.md` — per-file line refs for each smell
|
||||
* - `changes.md` — summary of edits + human-review items
|
||||
*
|
||||
* @module pygienium/checks/comments
|
||||
*/
|
||||
|
||||
import type { CheckDefinition, CheckScope } from "./registry.js";
|
||||
import { scopeRulesMarkdown } from "./scope.js";
|
||||
|
||||
/** Phase-strip phase this check belongs to. */
|
||||
export const COMMENTS_PHASE_ID = "C1";
|
||||
|
||||
/**
|
||||
* Directory where this check writes its `findings.md` and `changes.md`
|
||||
* artifacts: `<cwd>/.pygienium/checks/comments/`. Based on `scope.cwd` (the
|
||||
* project root, always a directory) so the path is valid whether the scan
|
||||
* target is a single file or a directory. Matches the spec's
|
||||
* `.pygienium/checks/comments/findings.md` relative-path notation.
|
||||
*/
|
||||
export function commentsArtifactDir(scope: CheckScope): string {
|
||||
const base = scope.cwd.replace(/\/+$/, "");
|
||||
return `${base}/.pygienium/checks/comments`;
|
||||
}
|
||||
|
||||
/** Absolute path to the findings artifact for this check. */
|
||||
export function findingsPath(scope: CheckScope): string {
|
||||
return `${commentsArtifactDir(scope)}/findings.md`;
|
||||
}
|
||||
|
||||
/** Absolute path to the changes artifact for this check. */
|
||||
export function changesPath(scope: CheckScope): string {
|
||||
return `${commentsArtifactDir(scope)}/changes.md`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Shared rubric block, injected into both scan and fix task text so the analysis
|
||||
* and remediation sub-agents apply identical judgement.
|
||||
*/
|
||||
const RUBRIC = `# Comments hygiene rubric
|
||||
|
||||
Short + high value is the goal. Evaluate every comment in the target:
|
||||
|
||||
- **RESTATE → REMOVE.** A comment that paraphrases the line(s) it sits on adds
|
||||
no information. Examples: \`// increment i\` over \`i++\`, \`// return the
|
||||
result\` over \`return result\`. Delete it.
|
||||
- **VERBOSE → TIGHTEN.** A comment that is high-value but needlessly long.
|
||||
Rewrite it to one tight sentence preserving the key insight. Do not delete.
|
||||
- **"WHY" → KEEP.** A comment explaining intent, rationale, a non-obvious
|
||||
decision, a workaround, a gotcha, or a constraint the code cannot express.
|
||||
Leave it untouched (tighten only if it is also verbose).
|
||||
- **NO COMMENT NEEDED.** When the code is self-explanatory, do not add a comment.
|
||||
- Keep inline section headers/dividers that aid navigation only if they mark a
|
||||
real boundary; remove pure decoration.`;
|
||||
|
||||
/**
|
||||
* Build the analysis sub-agent task. Instructs the agent to read candidate
|
||||
* source files, identify comment smells per the rubric, and write per-file line
|
||||
* references to `.pygienium/comments/findings.md`.
|
||||
*/
|
||||
export function buildCommentsScanTask(_cwd: string, scope: CheckScope): string {
|
||||
const outDir = commentsArtifactDir(scope);
|
||||
const findingsFile = findingsPath(scope);
|
||||
return `# Task: comments hygiene scan
|
||||
|
||||
You are running the **comments** hygiene check.
|
||||
|
||||
## Target
|
||||
- Scan target: \`${scope.target}\`
|
||||
|
||||
${scopeRulesMarkdown()}
|
||||
## What to do
|
||||
1. Read the recon snapshot at \`${scope.cwd}/.pygienium/recon.json\` if it
|
||||
exists; otherwise enumerate source files directly under the target.
|
||||
2. For each source file, read it and locate every comment (inline \`//\`,
|
||||
block \`/* */\`, doc \`/** */\`, \`#\` for scripting languages, etc.).
|
||||
3. Apply the rubric below to each comment and classify it: RESTATE, VERBOSE,
|
||||
WHY, or OK.
|
||||
4. Write a findings report to \`${findingsFile}\` with per-file line refs.
|
||||
5. Return the findings report text as your final message (same content as the
|
||||
file). The host captures it as the analysis-phase findings.
|
||||
|
||||
${RUBRIC}
|
||||
|
||||
## findings.md format
|
||||
|
||||
\`\`\`markdown
|
||||
# comments — findings
|
||||
|
||||
<count> comment smell(s) across <files> file(s).
|
||||
|
||||
## <relative-file>
|
||||
- L<line>: <smell: RESTATE|VERBOSE> — <quote or paraphrase>
|
||||
- L<line>: KEEP (why) — <one-line reason> # listed for transparency
|
||||
\`\`\`
|
||||
|
||||
If no smells are found, write \`# comments — findings\n\n0 comment smell(s).\`
|
||||
and return that text. Always create findings.md so the run has an artifact.
|
||||
|
||||
Write the report under \`${outDir}\` (create directories as needed).
|
||||
`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the fix sub-agent task from the scan findings. Instructs the agent to
|
||||
* apply safe removals/tightenings, leave "why" comments, and write a summary
|
||||
* of edits plus anything needing human review to `.pygienium/comments/changes.md`.
|
||||
*/
|
||||
export function buildCommentsFixTask(
|
||||
_cwd: string,
|
||||
scope: CheckScope,
|
||||
findings: string,
|
||||
): string {
|
||||
const outDir = commentsArtifactDir(scope);
|
||||
const changesFile = changesPath(scope);
|
||||
return `# Task: comments hygiene fix
|
||||
|
||||
You are running the **comments** hygiene fix phase.
|
||||
|
||||
## Target
|
||||
- Fix target: \`${scope.target}\`
|
||||
|
||||
## Input: scan findings
|
||||
${findings.trim().length > 0 ? findings : "(no findings text provided)"}
|
||||
|
||||
## What to do
|
||||
1. For each RESTATE finding: remove the comment entirely.
|
||||
2. For each VERBOSE finding: replace the comment with a tightened one-sentence
|
||||
version that keeps the key insight.
|
||||
3. For every WHY comment: leave it untouched (tighten only if it is also
|
||||
verbose, preserving the rationale).
|
||||
4. Do not change any code logic, formatting, or ordering — only comments.
|
||||
5. Write a summary to \`${changesFile}\` and return it as your final message.
|
||||
|
||||
${RUBRIC}
|
||||
|
||||
## changes.md format
|
||||
|
||||
\`\`\`markdown
|
||||
# comments — changes
|
||||
|
||||
<applied> edit(s) applied; <deferred> deferred for human review.
|
||||
|
||||
## Applied
|
||||
- <relative-file>:<line> — <removed|tightened> comment (auto)
|
||||
|
||||
## Needs human review
|
||||
- <relative-file>:<line> — <reason> (manual)
|
||||
\`\`\`
|
||||
|
||||
If nothing needed changing, write
|
||||
\`# comments — changes\n\n0 edit(s) applied.\` and return that text. Always
|
||||
create changes.md so the run has an artifact. Write it under \`${outDir}\`.
|
||||
`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Precondition gate. Returns an error string when the comments check cannot
|
||||
* proceed (target path missing or not a real file/directory), else
|
||||
* `undefined`. Idempotent — passes identically before analysis and at verify.
|
||||
*/
|
||||
async function commentsGate(cwd: string): Promise<string | undefined> {
|
||||
const { stat } = await import("node:fs/promises");
|
||||
const { resolve } = await import("node:path");
|
||||
const target = resolve(cwd);
|
||||
try {
|
||||
const s = await stat(target);
|
||||
if (s.isDirectory() || s.isFile()) return undefined;
|
||||
return `target is not a file or directory: ${target}`;
|
||||
} catch {
|
||||
return `target path does not exist: ${target}`;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Verify hook: confirms the check actually produced its artifacts. After the
|
||||
* scan phase `findings.md` must exist; after the fix phase `changes.md` must
|
||||
* exist too (the scan-phase-only run skips `changes.md` by design). Returns an
|
||||
* error string to fail verify, or `undefined` to pass. Replaces the historical
|
||||
* no-op verify (which only re-ran the existence gate) so the verify phase now
|
||||
* genuinely asserts the run produced its report.
|
||||
*/
|
||||
async function commentsVerify(scope: CheckScope): Promise<string | undefined> {
|
||||
const { stat } = await import("node:fs/promises");
|
||||
const f = findingsPath(scope);
|
||||
try {
|
||||
await stat(f);
|
||||
} catch {
|
||||
return `comments verify: expected findings.md at ${f} after scan, none found.`;
|
||||
}
|
||||
if (scope.fix) {
|
||||
const c = changesPath(scope);
|
||||
try {
|
||||
await stat(c);
|
||||
} catch {
|
||||
return `comments verify: expected changes.md at ${c} after --fix, none found.`;
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/** The comments hygiene check definition. */
|
||||
export const check = {
|
||||
name: "comments",
|
||||
label: "Comments",
|
||||
description:
|
||||
'Remove low-value/restating comments, tighten verbose ones, keep "why" comments.',
|
||||
agentName: "scanner",
|
||||
fixAgentName: "fixer",
|
||||
phaseId: COMMENTS_PHASE_ID,
|
||||
buildScanTask: buildCommentsScanTask,
|
||||
buildFixTask: buildCommentsFixTask,
|
||||
gate: commentsGate,
|
||||
verify: commentsVerify,
|
||||
} as const satisfies CheckDefinition;
|
||||
|
||||
// No self-registration here: `index.ts` auto-discovers every `checks/*.ts`
|
||||
// that exports `check` and registers it — a new check is still one file.
|
||||
317
src/checks/complexity.ts
Normal file
317
src/checks/complexity.ts
Normal file
@@ -0,0 +1,317 @@
|
||||
/**
|
||||
* checks/complexity.ts — excessive complexity check.
|
||||
*
|
||||
* Detects high cyclomatic complexity and structural complexity smells, then
|
||||
* refactors toward the simplest implementation that meets requirements.
|
||||
*
|
||||
* Cyclomatic complexity thresholds (MUST enforce, not advisory):
|
||||
* - 50+ → must refactor. No exceptions.
|
||||
* - 35–49 → heavy skepticism. Only keep if critical path + justified.
|
||||
* - <35 → not flagged on cyclomatic grounds (may still be flagged for other
|
||||
* structural smells).
|
||||
*
|
||||
* Structural smells detected:
|
||||
* - deep nesting (>3 levels)
|
||||
* - speculative abstractions
|
||||
* - premature config indirection
|
||||
* - non-idiomatic patterns
|
||||
* - over-engineered generics
|
||||
* - unnecessary wrappers
|
||||
*
|
||||
* @module pygienium/checks/complexity
|
||||
*/
|
||||
|
||||
import type { CheckDefinition, CheckScope } from "./registry.js";
|
||||
import { scopeRulesMarkdown } from "./scope.js";
|
||||
|
||||
/** Phase-strip phase this check belongs to. */
|
||||
export const COMPLEXITY_PHASE_ID = "C4";
|
||||
|
||||
/**
|
||||
* Artifact directory: `<cwd>/.pygienium/checks/complexity/`.
|
||||
*/
|
||||
export function complexityArtifactDir(scope: CheckScope): string {
|
||||
const base = scope.cwd.replace(/\/+$/, "");
|
||||
return `${base}/.pygienium/checks/complexity`;
|
||||
}
|
||||
|
||||
/** Absolute path to findings artifact. */
|
||||
export function findingsPath(scope: CheckScope): string {
|
||||
return `${complexityArtifactDir(scope)}/findings.md`;
|
||||
}
|
||||
|
||||
/** Absolute path to changes artifact. */
|
||||
export function changesPath(scope: CheckScope): string {
|
||||
return `${complexityArtifactDir(scope)}/changes.md`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Shared rubric for complexity analysis, injected into both scan and fix tasks.
|
||||
*/
|
||||
const RUBRIC = `# Complexity hygiene rubric
|
||||
|
||||
## Cyclomatic complexity thresholds
|
||||
|
||||
Cyclomatic complexity counts the number of independent paths through a function.
|
||||
Compute via language-native tools when available (lizard, radon, gocyclo), or
|
||||
count decision points (if/else if/for/while/case/&&/||/catch) per function.
|
||||
|
||||
| Score | Action |
|
||||
|-------|--------|
|
||||
| 50+ | **MUST refactor.** No exceptions. Break the function into smaller pieces. |
|
||||
| 35–49 | **Heavy skepticism.** Only keep if this is a massively critical point along the main path AND the complexity genuinely must be here. Document justification in findings.md; otherwise refactor. |
|
||||
| <35 | Not flagged on cyclomatic grounds (may still be flagged for other structural smells). |
|
||||
|
||||
## Structural complexity smells
|
||||
|
||||
- **Deep nesting (>3 levels).** Flatten with early returns, guard clauses, or extracting to named helpers.
|
||||
- **Speculative abstractions.** Remove abstractions created "just in case" — no concrete use case yet.
|
||||
- **Premature config indirection.** Remove configuration layers that add no value yet.
|
||||
- **Non-idiomatic patterns.** Replace with common conventions for the language.
|
||||
- **Over-engineered generics.** Simplify to concrete types when only one type is used.
|
||||
- **Unnecessary wrappers.** Inline trivial wrappers that add no logic.
|
||||
|
||||
## Refactoring principles
|
||||
|
||||
1. **Simplest implementation.** Choose the simplest implementation that fully meets current requirements.
|
||||
2. **No backward-compat baggage.** Remove obsolete paths rather than adding compatibility layers.
|
||||
3. **Grow in layers.** Build on a product that already works; don't trade a working product for unfinished complexity.
|
||||
4. **Use existing libraries.** Lean on well-maintained libraries when they reduce complexity or improve reliability.
|
||||
5. **Long-term decisions.** Make architectural decisions for the long term, not stopgaps meant to be replaced later.
|
||||
`;
|
||||
|
||||
/**
|
||||
* Build the analysis sub-agent task. Instructs the agent to:
|
||||
* 1. Compute cyclomatic complexity per function
|
||||
* 2. Identify structural complexity smells
|
||||
* 3. Write findings to .pygienium/checks/complexity/findings.md
|
||||
*/
|
||||
export function buildComplexityScanTask(
|
||||
_cwd: string,
|
||||
scope: CheckScope,
|
||||
): string {
|
||||
const findingsFile = findingsPath(scope);
|
||||
return `# Task: excessive complexity scan
|
||||
|
||||
You are running the **complexity** hygiene check.
|
||||
|
||||
## Target
|
||||
- Scan target: \`${scope.target}\`
|
||||
|
||||
${scopeRulesMarkdown()}
|
||||
## What to do
|
||||
|
||||
### 1. Compute cyclomatic complexity
|
||||
|
||||
For each source file in the target:
|
||||
|
||||
1. Read the recon snapshot at \`${scope.cwd}/.pygienium/recon.json\` if it
|
||||
exists; otherwise enumerate source files directly under the target.
|
||||
2. For each file, identify every function/method/class.
|
||||
3. Compute cyclomatic complexity:
|
||||
- Prefer language-native tools (lizard, radon, gocyclo, etc.) when available
|
||||
- Fall back to counting decision points: if/else if/for/while/case/&&/||/catch
|
||||
4. Classify each function into bands:
|
||||
- **50+** = MUST refactor (no exceptions)
|
||||
- **35–49** = heavy skepticism (must justify or refactor)
|
||||
- **<35** = not flagged on cyclomatic grounds
|
||||
|
||||
### 2. Identify structural complexity smells
|
||||
|
||||
For each file, identify:
|
||||
- Deep nesting (>3 levels)
|
||||
- Speculative abstractions
|
||||
- Premature config indirection
|
||||
- Non-idiomatic patterns
|
||||
- Over-engineered generics
|
||||
- Unnecessary wrappers
|
||||
|
||||
### 3. Write findings
|
||||
|
||||
Write a findings report to \`${findingsFile}\` with per-function scores and
|
||||
structural smell locations. Include a proposed simpler form for every flagged
|
||||
function.
|
||||
|
||||
${RUBRIC}
|
||||
|
||||
## findings.md format
|
||||
|
||||
\`\`\`markdown
|
||||
# complexity — findings
|
||||
|
||||
## Cyclomatic complexity
|
||||
|
||||
| File | Function | Score | Band | Action |
|
||||
|------|----------|-------|------|--------|
|
||||
| path/to/file:42 | myFunction | 65 | 50+ | MUST refactor |
|
||||
| path/to/file:100 | otherFunction | 42 | 35-49 | Skepticism — justify or refactor |
|
||||
| path/to/file:150 | simpleFunction | 8 | <35 | OK |
|
||||
|
||||
## Structural smells
|
||||
|
||||
- [severity] <file>:<line> — <smell type> — <description>
|
||||
- <proposed simplification>
|
||||
|
||||
## Justifications (35–49 band)
|
||||
|
||||
For each function kept at 35–49 complexity:
|
||||
- **Function:** <name> at <file>:<line>
|
||||
- **Score:** <score>
|
||||
- **Justification:** <why this is critical path and can't be simplified>
|
||||
\`\`\`
|
||||
|
||||
If no issues found, write:
|
||||
\`# complexity — findings\n\n0 complexity issues found.\`
|
||||
|
||||
Always create findings.md so the run has an artifact.
|
||||
`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the fix sub-agent task from the scan findings. Instructs the agent to:
|
||||
* 1. Split 50+ complexity functions
|
||||
* 2. Refactor or justify 35–49 functions
|
||||
* 3. Apply safe refactors for structural smells
|
||||
* 4. Write changes summary to .pygienium/checks/complexity/changes.md
|
||||
*/
|
||||
export function buildComplexityFixTask(
|
||||
_cwd: string,
|
||||
scope: CheckScope,
|
||||
findings: string,
|
||||
): string {
|
||||
const outDir = complexityArtifactDir(scope);
|
||||
const changesFile = changesPath(scope);
|
||||
return `# Task: excessive complexity fix
|
||||
|
||||
You are running the **complexity** hygiene fix phase.
|
||||
|
||||
## Target
|
||||
- Fix target: \`${scope.target}\`
|
||||
|
||||
## Input: scan findings
|
||||
${findings.trim().length > 0 ? findings : "(no findings text provided)"}
|
||||
|
||||
## What to do
|
||||
|
||||
### 1. Handle 50+ functions (MUST refactor)
|
||||
|
||||
For each function with cyclomatic complexity ≥ 50:
|
||||
- Split into smaller, focused functions
|
||||
- Extract complex conditional branches into named helper functions
|
||||
- Use early returns and guard clauses to reduce nesting
|
||||
- Preserve behavior after refactoring
|
||||
|
||||
### 2. Handle 35–49 functions
|
||||
|
||||
For each function in the 35–49 band:
|
||||
- If no justified critical-path reason exists, refactor
|
||||
- If kept, ensure justification is documented in findings.md
|
||||
- Prefer refactoring over keeping
|
||||
|
||||
### 3. Apply structural refactors
|
||||
|
||||
- Flatten deep nesting (>3 levels)
|
||||
- Remove speculative abstractions
|
||||
- Inline trivial wrappers
|
||||
- Replace non-idiomatic patterns with conventional ones
|
||||
- Simplify over-engineered generics to concrete types
|
||||
|
||||
### 4. Write changes summary
|
||||
|
||||
Write a summary to \`${changesFile}\` and return it as your final message.
|
||||
|
||||
${RUBRIC}
|
||||
|
||||
## changes.md format
|
||||
|
||||
\`\`\`markdown
|
||||
# complexity — changes
|
||||
|
||||
<applied> refactoring(s) applied; <deferred> deferred for human review.
|
||||
|
||||
## Applied
|
||||
|
||||
- <file>:<line> — <function> split (was <score>, now <scores>)
|
||||
- <file>:<line> — nested conditionals flattened
|
||||
- <file>:<line> — trivial wrapper inlined
|
||||
- <file>:<line> — speculative abstraction removed
|
||||
|
||||
## Deferred (needs human review)
|
||||
|
||||
- <file>:<line> — <function> — <reason> (manual)
|
||||
|
||||
## Justified (kept at 35–49)
|
||||
|
||||
- <file>:<line> — <function> (<score>) — <justification>
|
||||
\`\`\`
|
||||
|
||||
If nothing needed changing, write:
|
||||
\`# complexity — changes\n\n0 refactoring(s) applied.\`
|
||||
|
||||
Always create changes.md so the run has an artifact. Write it under \`${outDir}\`.
|
||||
`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Precondition gate. Returns an error string when the complexity check cannot
|
||||
* proceed (target path missing or not a real file/directory), else
|
||||
* `undefined`.
|
||||
*/
|
||||
async function complexityGate(cwd: string): Promise<string | undefined> {
|
||||
const { stat } = await import("node:fs/promises");
|
||||
const { resolve } = await import("node:path");
|
||||
const target = resolve(cwd);
|
||||
try {
|
||||
const s = await stat(target);
|
||||
if (s.isDirectory() || s.isFile()) return undefined;
|
||||
return `target is not a file or directory: ${target}`;
|
||||
} catch {
|
||||
return `target path does not exist: ${target}`;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Verify hook: confirms the check actually produced its artifacts. After the
|
||||
* scan phase `findings.md` must exist; after the fix phase `changes.md` must
|
||||
* exist too. Without this, a sub-agent that returns empty/ok without writing
|
||||
* its report would be stamped `complete` — a false positive. Mirrors
|
||||
* {@link commentsVerify} / {@link todosVerify}.
|
||||
*/
|
||||
async function complexityVerify(
|
||||
scope: CheckScope,
|
||||
): Promise<string | undefined> {
|
||||
const { stat } = await import("node:fs/promises");
|
||||
const f = findingsPath(scope);
|
||||
try {
|
||||
await stat(f);
|
||||
} catch {
|
||||
return `complexity verify: expected findings.md at ${f} after scan, none found.`;
|
||||
}
|
||||
if (scope.fix) {
|
||||
const c = changesPath(scope);
|
||||
try {
|
||||
await stat(c);
|
||||
} catch {
|
||||
return `complexity verify: expected changes.md at ${c} after --fix, none found.`;
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/** The excessive complexity check definition. */
|
||||
export const check = {
|
||||
name: "complexity",
|
||||
label: "Complexity",
|
||||
description:
|
||||
"Detect and refactor excessive complexity: high cyclomatic complexity (50+ must refactor, 35-49 needs justification), deep nesting, and speculative abstractions.",
|
||||
agentName: "scanner",
|
||||
fixAgentName: "fixer",
|
||||
phaseId: COMPLEXITY_PHASE_ID,
|
||||
buildScanTask: buildComplexityScanTask,
|
||||
buildFixTask: buildComplexityFixTask,
|
||||
gate: complexityGate,
|
||||
verify: complexityVerify,
|
||||
} as const satisfies CheckDefinition;
|
||||
|
||||
// No self-registration here: `index.ts` auto-discovers every `checks/*.ts`
|
||||
// that exports `check` and registers it — a new check is still one file.
|
||||
1144
src/checks/dead-code.ts
Normal file
1144
src/checks/dead-code.ts
Normal file
File diff suppressed because it is too large
Load Diff
185
src/checks/deep-modules.ts
Normal file
185
src/checks/deep-modules.ts
Normal file
@@ -0,0 +1,185 @@
|
||||
/**
|
||||
* checks/deep-modules.ts — "deep modules, not shallow ones" check.
|
||||
*
|
||||
* Detects modules with shallow abstractions (thin pass-throughs, one-line
|
||||
* re-export barrels, trivial getter classes, unnecessary adapter layers) and
|
||||
* recommends/applies consolidation. The rubric encodes John Ousterhout's
|
||||
* "deep modules" definition from *A Philosophy of Software Design*: a module
|
||||
* is valuable when it hides a substantial implementation behind a small
|
||||
* interface; a shallow one exposes as much complexity as it hides, so its
|
||||
* indirection adds cost without abstraction payoff.
|
||||
*
|
||||
* Lifecycle:
|
||||
* gate (need source files) → recon (shared) → scan sub-agent writes
|
||||
* `<cwd>/.pygienium/checks/deep-modules/findings.md` → [with --fix] fix
|
||||
* sub-agent writes `changes.md`, inlines safe pass-throughs, and lists
|
||||
* risky consolidations (external importers / public API) for human review.
|
||||
*
|
||||
* Registering this file is the ONLY wiring needed: `index.ts` auto-discovers
|
||||
* `src/checks/*.ts`, so dropping this file exposes `/pygienium-deep-modules`.
|
||||
*
|
||||
* @module pygienium/checks/deep-modules
|
||||
*/
|
||||
|
||||
import { readdirSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import type { CheckDefinition, CheckScope } from "./registry.js";
|
||||
import { isScopeSource, scopeRulesMarkdown } from "./scope.js";
|
||||
|
||||
/** Output directory for this check's persistent reports. */
|
||||
export function deepModulesOutputDir(cwd: string): string {
|
||||
return join(cwd, ".pygienium", "checks", "deep-modules");
|
||||
}
|
||||
|
||||
/** `findings.md` path for this check. */
|
||||
export function findingsPath(cwd: string): string {
|
||||
return join(deepModulesOutputDir(cwd), "findings.md");
|
||||
}
|
||||
|
||||
/** `changes.md` path for this check. */
|
||||
export function changesPath(cwd: string): string {
|
||||
return join(deepModulesOutputDir(cwd), "changes.md");
|
||||
}
|
||||
|
||||
/**
|
||||
* Gate: skip when the cwd has no inspectable source files at all. A workspace
|
||||
* with zero source files gives the scanner nothing to classify.
|
||||
*/
|
||||
function deepModulesGate(cwd: string): string | undefined {
|
||||
let found = false;
|
||||
try {
|
||||
const entries = readdirSync(cwd);
|
||||
for (const entry of entries) {
|
||||
if (isScopeSource(entry)) {
|
||||
found = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// unreadable cwd → let the agent decide; don't block.
|
||||
return undefined;
|
||||
}
|
||||
if (!found) {
|
||||
return "no source files found to inspect";
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Verify hook: confirms the check actually produced its artifacts (mirrors
|
||||
* {@link commentsVerify} / {@link todosVerify}). After scan `findings.md` must
|
||||
* exist; after `--fix` `changes.md` must exist too. Catches a sub-agent that
|
||||
* returns ok with no output — which would otherwise be a false `complete`.
|
||||
*/
|
||||
async function deepModulesVerify(
|
||||
scope: CheckScope,
|
||||
): Promise<string | undefined> {
|
||||
const { stat } = await import("node:fs/promises");
|
||||
const f = findingsPath(scope.cwd);
|
||||
try {
|
||||
await stat(f);
|
||||
} catch {
|
||||
return `deep-modules verify: expected findings.md at ${f} after scan, none found.`;
|
||||
}
|
||||
if (scope.fix) {
|
||||
const c = changesPath(scope.cwd);
|
||||
try {
|
||||
await stat(c);
|
||||
} catch {
|
||||
return `deep-modules verify: expected changes.md at ${c} after --fix, none found.`;
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the scan task. The deep-modules scanner agent inspects the target,
|
||||
* classifies modules by abstraction depth against the rubric, and writes a
|
||||
* structured findings report to `findings.md`. The output path is passed into
|
||||
* the task so both the real agent (which uses its `write` tool) and the
|
||||
* deterministic fake runner (which understands `!write <path> <text>`) persist
|
||||
* the report to the same location.
|
||||
*
|
||||
* Note: the `!write`/`!echo` lines are the deterministic fallback the fake
|
||||
* runner executes for tests/smoke runs; a real model-driven agent receives the
|
||||
* whole prompt and writes a real analysis.
|
||||
*/
|
||||
function buildDeepScanTask(cwd: string, scope: CheckScope): string {
|
||||
const findings = findingsPath(cwd);
|
||||
const target = scope.target;
|
||||
// The expected findings document shape, shown to a real model-driven agent
|
||||
// as the format spec. The `!write`/`!echo` lines below are the deterministic
|
||||
// fallback the fake runner executes for tests/smoke runs.
|
||||
return [
|
||||
`Inspect the target "${target}" (cwd: ${cwd}) for shallow modules.`,
|
||||
`Classify every source module by abstraction depth (see your rubric).`,
|
||||
`Write your full findings report to: ${findings}.`,
|
||||
`findings.md must list each flagged module with: kind, evidence, importer`,
|
||||
`count, recommendation, and risk (low if no external importers, high else).`,
|
||||
`Then emit a one-line summary referencing the findings path.`,
|
||||
"",
|
||||
scopeRulesMarkdown(),
|
||||
"",
|
||||
`# Deterministic fallback (executed by the fake runner in tests):`,
|
||||
`!write ${findings} # Deep-modules findings | summary: 1 shallow module(s) flagged | ## 1. ${target}/wrapper.ts | kind: pass-through-wrapper | evidence: forwards every argument to a single library call with no added logic | importers: 0 | recommendation: inline-and-remove | risk: low`,
|
||||
`!echo deep-modules: 1 issue — see ${findings}`,
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the fix task. The fixer consumes the scan findings and applies ONLY safe
|
||||
* consolidations: inline-and-remove pass-through wrappers that have **zero**
|
||||
* external importers. Risky consolidations (any importer, or unclear
|
||||
* ownership) are listed in `changes.md` as `review-manual` and NOT applied.
|
||||
* Every action — applied or deferred — is recorded in `changes.md`.
|
||||
*/
|
||||
function buildDeepFixTask(
|
||||
cwd: string,
|
||||
scope: CheckScope,
|
||||
findings: string,
|
||||
): string {
|
||||
const changes = changesPath(cwd);
|
||||
const findingsFile = findingsPath(cwd);
|
||||
const target = scope.target;
|
||||
return [
|
||||
`Consolidate shallow modules found in the scan.`,
|
||||
`cwd: ${cwd} target: ${target}`,
|
||||
`Findings report (also persisted at ${findingsFile}):`,
|
||||
`---`,
|
||||
findings,
|
||||
`---`,
|
||||
``,
|
||||
`Rules:`,
|
||||
`- Apply ONLY safe consolidations: a pass-through wrapper with zero external`,
|
||||
` importers may be inlined at its single use site and the wrapper removed.`,
|
||||
`- NEVER auto-delete or rewrite a module with any external importer — list it`,
|
||||
` for human review instead.`,
|
||||
`- Preserve public API boundaries; when in doubt, defer to manual review.`,
|
||||
`- Write changes.md to ${changes} describing every action (auto | manual) with`,
|
||||
` the file, the finding, and the disposition.`,
|
||||
``,
|
||||
`# Deterministic consolidation (executed by the fake runner in tests):`,
|
||||
`# Safe: zero-importer pass-through rewritten/removed (auto).`,
|
||||
`# Risky: external-importer adapter left in place (manual).`,
|
||||
`!write ${target}/wrapper.ts // Consolidated by pygienium-deep-modules: pass-through wrapper removed; callers now use the underlying implementation directly.`,
|
||||
`!write ${changes} # Deep-modules changes | 1. ${target}/wrapper.ts — pass-through-wrapper — consolidated: inlined the underlying call at the use site and removed the wrapper module (auto) | 2. ${target}/risky-adapter.ts — adapter-layer — 2 external importer(s): left in place; listed for review (manual)`,
|
||||
`!echo deep-modules: 1 auto-applied, 1 deferred to review — see ${changes}`,
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
/** The check definition; registers itself on import. */
|
||||
export const check: CheckDefinition = {
|
||||
name: "deep-modules",
|
||||
label: "Deep modules",
|
||||
description:
|
||||
"Detect shallow modules (pass-throughs, trivial wrappers, re-export barrels) and consolidate the safe ones.",
|
||||
agentName: "deep-modules",
|
||||
phaseId: "analysis",
|
||||
buildScanTask: buildDeepScanTask,
|
||||
buildFixTask: buildDeepFixTask,
|
||||
gate: deepModulesGate,
|
||||
verify: deepModulesVerify,
|
||||
};
|
||||
|
||||
// No self-registration here: `index.ts` auto-discovers every `checks/*.ts`
|
||||
// that exports `check` and registers it.
|
||||
215
src/checks/defensive-guards.ts
Normal file
215
src/checks/defensive-guards.ts
Normal file
@@ -0,0 +1,215 @@
|
||||
/**
|
||||
* checks/defensive-guards.ts — "redundant defensive guarding" check.
|
||||
*
|
||||
* Detects defensive code that guards invariants the type system or an
|
||||
* upstream validation already guarantees, and removes the redundant guards
|
||||
* while preserving guards that protect genuine external boundaries (user
|
||||
* input, IO, parsing, untrusted data). The rubric encodes the engineering rule:
|
||||
* no compatibility layers or fallbacks meant to be "replaced later" — remove
|
||||
* them outright rather than layering over them.
|
||||
*
|
||||
* Flagged smells (non-exhaustive):
|
||||
* - redundant-null-check — null/undefined check on a value whose declared
|
||||
* type is already non-nullable.
|
||||
* - swallowing-try-catch — try/catch that silently discards the error
|
||||
* (empty catch, catch that only logs, or catch returning a fallback that
|
||||
* hides the failure).
|
||||
* - rethrow-only-try-catch — try/catch whose body only rethrows the exact
|
||||
* error, adding nothing.
|
||||
* - error-masking-fallback — `return defaultValue` / `|| fallback` in a
|
||||
* catch that masks a real failure with a plausible-but-wrong value.
|
||||
* - defensive-guard-on-validated-input — re-checking input that a caller or
|
||||
* parser already validated (e.g. asserting a parsed enum is in range).
|
||||
* - compatibility-fallback — a fallback branch kept "for now" / "to be
|
||||
* replaced later" (engineering rule: remove, don't layer).
|
||||
*
|
||||
* Kept (legitimate boundary guards):
|
||||
* - untrusted input (HTTP params, CLI args, env vars, files on disk).
|
||||
* - IO (network, filesystem, subprocess) where failures are expected.
|
||||
* - parsing (`JSON.parse`, `parseInt`, `Date.parse`, schema decoders).
|
||||
*
|
||||
* Lifecycle:
|
||||
* gate (need source files) → recon (shared) → scan sub-agent writes
|
||||
* `<cwd>/.pygienium/checks/defensive-guards/findings.md` separating redundant
|
||||
* guards from boundary guards → [with --fix] fix sub-agent removes redundant
|
||||
* guards, preserves boundary guards, and writes `changes.md` distinguishing
|
||||
* removed vs kept-with-reason.
|
||||
*
|
||||
* Registering this file is the ONLY wiring needed: `index.ts` auto-discovers
|
||||
* `src/checks/*.ts`, so dropping this file exposes `/pygienium-defensive-guards`.
|
||||
*
|
||||
* @module pygienium/checks/defensive-guards
|
||||
*/
|
||||
|
||||
import { readdirSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import type { CheckDefinition, CheckScope } from "./registry.js";
|
||||
import { isScopeSource, scopeRulesMarkdown } from "./scope.js";
|
||||
|
||||
/** Output directory for this check's persistent reports. */
|
||||
export function defensiveGuardsOutputDir(cwd: string): string {
|
||||
return join(cwd, ".pygienium", "checks", "defensive-guards");
|
||||
}
|
||||
|
||||
/** `findings.md` path for this check. */
|
||||
export function findingsPath(cwd: string): string {
|
||||
return join(defensiveGuardsOutputDir(cwd), "findings.md");
|
||||
}
|
||||
|
||||
/** `changes.md` path for this check. */
|
||||
export function changesPath(cwd: string): string {
|
||||
return join(defensiveGuardsOutputDir(cwd), "changes.md");
|
||||
}
|
||||
|
||||
/**
|
||||
* Gate: skip when the cwd has no inspectable source files at all — a workspace
|
||||
* with zero source files gives the scanner nothing to analyse.
|
||||
*/
|
||||
function defensiveGuardsGate(cwd: string): string | undefined {
|
||||
let found = false;
|
||||
try {
|
||||
const entries = readdirSync(cwd);
|
||||
for (const entry of entries) {
|
||||
if (isScopeSource(entry)) {
|
||||
found = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// unreadable cwd → let the agent decide; don't block.
|
||||
return undefined;
|
||||
}
|
||||
if (!found) {
|
||||
return "no source files found to inspect";
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Verify hook: confirms the check actually produced its artifacts (mirrors
|
||||
* {@link commentsVerify} / {@link todosVerify}). After scan `findings.md` must
|
||||
* exist; after `--fix` `changes.md` must exist too. Catches a sub-agent that
|
||||
* returns ok with no output — which would otherwise be a false `complete`.
|
||||
*/
|
||||
async function defensiveGuardsVerify(
|
||||
scope: CheckScope,
|
||||
): Promise<string | undefined> {
|
||||
const { stat } = await import("node:fs/promises");
|
||||
const f = findingsPath(scope.cwd);
|
||||
try {
|
||||
await stat(f);
|
||||
} catch {
|
||||
return `defensive-guards verify: expected findings.md at ${f} after scan, none found.`;
|
||||
}
|
||||
if (scope.fix) {
|
||||
const c = changesPath(scope.cwd);
|
||||
try {
|
||||
await stat(c);
|
||||
} catch {
|
||||
return `defensive-guards verify: expected changes.md at ${c} after --fix, none found.`;
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the scan task. The defensive-guards scanner agent inspects the target,
|
||||
* classifies each guard as redundant or a legitimate boundary guard against the
|
||||
* rubric, and writes a structured findings report to `findings.md`. The output
|
||||
* path is passed into the task so both the real agent (which uses its `write`
|
||||
* tool) and the deterministic fake runner (which understands `!write <path>
|
||||
* <text>`) persist the report to the same location.
|
||||
*
|
||||
* The `!write`/`!echo` lines are the deterministic fallback the fake runner
|
||||
* executes for tests/smoke runs; a real model-driven agent receives the whole
|
||||
* prompt and writes a real analysis.
|
||||
*/
|
||||
function buildDefensiveGuardsScanTask(cwd: string, scope: CheckScope): string {
|
||||
const findings = findingsPath(cwd);
|
||||
const target = scope.target;
|
||||
return [
|
||||
`Inspect the target "${target}" (cwd: ${cwd}) for redundant defensive guarding.`,
|
||||
`Classify every guard (null check, try/catch, fallback) against your rubric as`,
|
||||
`either REDUNDANT (remove) or BOUNDARY (keep). Boundary guards protect real`,
|
||||
`external boundaries: untrusted input, IO, and parsing. Redundant guards protect`,
|
||||
`invariants the type system or upstream validation already guarantees.`,
|
||||
`Write your full findings report to: ${findings}.`,
|
||||
`findings.md must separate redundant guards from legitimate boundary guards,`,
|
||||
`listing each with: kind, evidence, disposition (remove | keep-boundary), and`,
|
||||
`reason.`,
|
||||
`Then emit a one-line summary referencing the findings path.`,
|
||||
``,
|
||||
scopeRulesMarkdown(),
|
||||
``,
|
||||
`# Deterministic fallback (executed by the fake runner in tests):`,
|
||||
`!write ${findings} # Defensive-guards findings | summary: 2 redundant guard(s) flagged, 1 boundary guard kept | ## 1. ${target}/noise.ts:2 | kind: redundant-null-check | evidence: \`if (name === null)\` on \`name\` whose declared type is \`string\` (non-nullable) | disposition: remove | reason: type system already guarantees non-null | ## 2. ${target}/noise.ts:7 | kind: swallowing-try-catch | evidence: try/catch around doThing() discards the error silently (empty catch body) | disposition: remove | reason: masks bugs; no error mapping or recovery logic | ## 3. ${target}/boundary.ts:2 | kind: parsing-guard | evidence: try/catch around JSON.parse(input) | disposition: keep-boundary | reason: protects an external parsing boundary (JSON.parse of untrusted input)`,
|
||||
`!echo defensive-guards: 2 redundant, 1 boundary kept — see ${findings}`,
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the fix task. The fixer consumes the scan findings and removes ONLY
|
||||
* redundant guards — those whose protected invariant is already guaranteed by
|
||||
* the type system or upstream validation. Boundary guards (IO, parsing,
|
||||
* untrusted input) are preserved untouched. Every action — removed or kept —
|
||||
* is recorded in `changes.md`, distinguishing removed (auto) from kept with a
|
||||
* reason (boundary).
|
||||
*
|
||||
* The fixer rewrites the affected source files with the redundant guards
|
||||
* excised; compatibility fallbacks are removed outright (engineering rule:
|
||||
* remove, don't layer), never left behind as a transitional shim.
|
||||
*/
|
||||
function buildDefensiveGuardsFixTask(
|
||||
cwd: string,
|
||||
scope: CheckScope,
|
||||
findings: string,
|
||||
): string {
|
||||
const changes = changesPath(cwd);
|
||||
const findingsFile = findingsPath(cwd);
|
||||
const target = scope.target;
|
||||
return [
|
||||
`Remove redundant defensive guards found in the scan.`,
|
||||
`cwd: ${cwd} target: ${target}`,
|
||||
`Findings report (also persisted at ${findingsFile}):`,
|
||||
`---`,
|
||||
findings,
|
||||
`---`,
|
||||
``,
|
||||
`Rules:`,
|
||||
`- Remove ONLY redundant guards: null/undefined checks on non-nullable types,`,
|
||||
` try/catch that only rethrows or swallows, fallback values that hide errors,`,
|
||||
` defensive guards on already-validated input, and compatibility fallbacks.`,
|
||||
`- PRESERVE boundary guards: anything protecting untrusted input, IO, or parsing`,
|
||||
` (e.g. JSON.parse, network, filesystem, subprocess errors). Do not touch them.`,
|
||||
`- No compatibility layers: remove fallbacks outright — never leave a shim meant`,
|
||||
` to be "replaced later".`,
|
||||
`- Apply the smallest diff that removes the guard without changing behaviour for`,
|
||||
` the happy path. Preserve tests and existing conventions.`,
|
||||
`- Write changes.md to ${changes} distinguishing removed (auto) from kept`,
|
||||
` (boundary — with reason) for every finding.`,
|
||||
``,
|
||||
`# Deterministic removal (executed by the fake runner in tests):`,
|
||||
`# Redundant null check + swallowing try/catch removed from noise.ts (auto).`,
|
||||
`# JSON.parse boundary guard in boundary.ts preserved (boundary).`,
|
||||
`!write ${target}/noise.ts // Cleaned by pygienium-defensive-guards: removed redundant null check on non-nullable \`name\` and the swallowing try/catch around doThing(). export function greet(name: string) { return \`hello \${name}\`; } export function swallow() { doThing(); } function doThing() {}`,
|
||||
`!write ${changes} # Defensive-guards changes | summary: 2 removed, 1 kept (boundary) | ## Removed (auto) | 1. ${target}/noise.ts:2 — redundant-null-check — removed \`if (name === null) return ""\`; type system guarantees non-null | 2. ${target}/noise.ts:7 — swallowing-try-catch — removed the try/catch around doThing(); the error is no longer silently swallowed | ## Kept (boundary — with reason) | 1. ${target}/boundary.ts:2 — parsing-guard — kept: try/catch around JSON.parse protects an external parsing boundary (untrusted input)`,
|
||||
`!echo defensive-guards: 2 removed, 1 kept (boundary) — see ${changes}`,
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
/** The check definition; registers itself on import. */
|
||||
export const check: CheckDefinition = {
|
||||
name: "defensive-guards",
|
||||
label: "Defensive guards",
|
||||
description:
|
||||
"Remove redundant defensive guarding (null checks on non-nullable types, swallowing try/catch, masking fallbacks) while keeping boundary guards (IO, parsing, untrusted input).",
|
||||
agentName: "defensive-guards",
|
||||
phaseId: "analysis",
|
||||
buildScanTask: buildDefensiveGuardsScanTask,
|
||||
buildFixTask: buildDefensiveGuardsFixTask,
|
||||
gate: defensiveGuardsGate,
|
||||
verify: defensiveGuardsVerify,
|
||||
};
|
||||
|
||||
// No self-registration here: `index.ts` auto-discovers every `checks/*.ts`
|
||||
// that exports `check` and registers it.
|
||||
139
src/checks/registry.ts
Normal file
139
src/checks/registry.ts
Normal file
@@ -0,0 +1,139 @@
|
||||
/**
|
||||
* checks/registry.ts — pluggable check registry.
|
||||
*
|
||||
* A `CheckDefinition` describes one hygiene check (e.g. `comments`, `complexity`).
|
||||
* The registry is a module-level `Map` so that adding a check only requires a
|
||||
* new file in `src/checks/` plus one `registerCheck(def)` call — no changes to
|
||||
* `index.ts` command wiring. At startup, `index.ts` iterates the registry and
|
||||
* auto-registers a `/pygienium-<name>` command per definition.
|
||||
*
|
||||
* Lifecycle of a single check run (orchestrated by `src/modes/check-runner.ts`):
|
||||
* Q0 recon (shared) → analysis sub-agent (buildScanTask) →
|
||||
* fix sub-agent (buildFixTask, only with --fix) → verify gate → cleanup.
|
||||
*
|
||||
* @module pygienium/checks/registry
|
||||
*/
|
||||
|
||||
/**
|
||||
* Scope passed to scan/fix task builders. Resolved from the command args:
|
||||
* a positional path (absolute or relative to `cwd`) plus parsed flags.
|
||||
*/
|
||||
export interface CheckScope {
|
||||
/** Absolute working directory the check operates on. */
|
||||
cwd: string;
|
||||
/** Target path (absolute) the check scans; defaults to `cwd` when none given. */
|
||||
target: string;
|
||||
/** Whether fixes should be applied (the `--fix` flag). */
|
||||
fix: boolean;
|
||||
/** Remaining raw tokens after flag parsing, for check-specific use. */
|
||||
rest: string[];
|
||||
}
|
||||
|
||||
/**
|
||||
* Verifies preconditions before a check runs its analysis phase. Returns an
|
||||
* error string when the check cannot proceed (e.g. no source files match),
|
||||
* or `undefined` when the gate passes. Implemented per-check so generic
|
||||
* checks can bail early without spawning an agent.
|
||||
*/
|
||||
export type CheckGate = (
|
||||
cwd: string,
|
||||
) => Promise<string | undefined> | string | undefined;
|
||||
|
||||
/**
|
||||
* Optional post-analysis (+ optional fix) verify hook. Confirms the check
|
||||
* actually produced its artifacts (e.g. `findings.md`/`changes.md`). Returns an
|
||||
* error string to fail the verify phase, or `undefined` to pass. When omitted,
|
||||
* the verify phase falls back to re-running {@link CheckDefinition.gate},
|
||||
* preserving the historical behaviour for checks that have nothing to verify.
|
||||
*/
|
||||
export type CheckVerify = (
|
||||
scope: CheckScope,
|
||||
) => Promise<string | undefined> | string | undefined;
|
||||
|
||||
/**
|
||||
* The structured task string handed to a sub-agent. `buildScanTask` produces
|
||||
* the analysis prompt; `buildFixTask` consumes the findings text the scan
|
||||
* agent emitted and produces a fix prompt.
|
||||
*
|
||||
* Task builders may be async: a check can pre-compute deterministic candidates
|
||||
* (e.g. an import-graph scan) before assembling the prompt, so the sub-agent's
|
||||
* job is to verify/refine rather than re-derive everything from scratch.
|
||||
*/
|
||||
export type BuildScanTask = (
|
||||
cwd: string,
|
||||
scope: CheckScope,
|
||||
) => string | Promise<string>;
|
||||
export type BuildFixTask = (
|
||||
cwd: string,
|
||||
scope: CheckScope,
|
||||
findings: string,
|
||||
) => string | Promise<string>;
|
||||
|
||||
/**
|
||||
* Definition of a single pluggable hygiene check.
|
||||
*/
|
||||
export interface CheckDefinition {
|
||||
/** Lowercase kebab command suffix → `/pygienium-<name>`. Must be unique. */
|
||||
name: string;
|
||||
/** Human label shown in help and status strips. */
|
||||
label: string;
|
||||
/** One-line description for `/pygienium-help`. */
|
||||
description: string;
|
||||
/**
|
||||
* Name of the agent definition (from `agents/*.md`) used for the analysis
|
||||
* phase. The fix phase uses the `fixer` agent unless `fixAgentName`
|
||||
* overrides it.
|
||||
*/
|
||||
agentName: string;
|
||||
/** Optional override for the fix-phase agent (defaults to `fixer`). */
|
||||
fixAgentName?: string;
|
||||
/** Identifier of the phase-strip phase this check belongs to (task 05). */
|
||||
phaseId: string;
|
||||
/** Builds the analysis sub-agent task. */
|
||||
buildScanTask: BuildScanTask;
|
||||
/** Builds the fix sub-agent task from scan findings. */
|
||||
buildFixTask: BuildFixTask;
|
||||
/**
|
||||
* Precondition gate. Returning a string skips the check (recorded as
|
||||
* `skipped`); returning `undefined` proceeds normally.
|
||||
*/
|
||||
gate: CheckGate;
|
||||
/**
|
||||
* Optional verify hook confirming artifacts landed (see {@link CheckVerify}).
|
||||
* Falls back to re-running `gate` when omitted.
|
||||
*/
|
||||
verify?: CheckVerify;
|
||||
}
|
||||
|
||||
const registry = new Map<string, CheckDefinition>();
|
||||
|
||||
/**
|
||||
* Register a check. Throws on duplicate names so wiring mistakes surface
|
||||
* loudly at startup rather than silently shadowing a command.
|
||||
*/
|
||||
export function registerCheck(def: CheckDefinition): void {
|
||||
if (!def.name || !/^[a-z0-9][a-z0-9-]*$/.test(def.name)) {
|
||||
throw new Error(
|
||||
`Invalid check name "${def.name}": must be lowercase kebab (e.g. "comments").`,
|
||||
);
|
||||
}
|
||||
if (registry.has(def.name)) {
|
||||
throw new Error(`Duplicate pygienium check name: "${def.name}".`);
|
||||
}
|
||||
registry.set(def.name, def);
|
||||
}
|
||||
|
||||
/** Look up a registered check by name. */
|
||||
export function getCheck(name: string): CheckDefinition | undefined {
|
||||
return registry.get(name);
|
||||
}
|
||||
|
||||
/** All registered checks in insertion order. */
|
||||
export function getAllChecks(): CheckDefinition[] {
|
||||
return [...registry.values()];
|
||||
}
|
||||
|
||||
/** Test-only: reset the registry between tests. */
|
||||
export function clearChecks(): void {
|
||||
registry.clear();
|
||||
}
|
||||
145
src/checks/scope.ts
Normal file
145
src/checks/scope.ts
Normal file
@@ -0,0 +1,145 @@
|
||||
/**
|
||||
* scope.ts — canonical source-of-truth for what pygienium checks inspect.
|
||||
*
|
||||
* Every check (recon, dead-code, deep-modules, defensive-guards, complexity,
|
||||
* comments) and every scanner agent prompt shares these definitions so the
|
||||
* "only inspect implementation code" rule is stated once, not copy-pasted
|
||||
* across four files that drift apart.
|
||||
*
|
||||
* @module pygienium/checks/scope
|
||||
*/
|
||||
|
||||
/**
|
||||
* Implementation-code file extensions pygienium inspects.
|
||||
*
|
||||
* Deliberately excludes documentation (`.md`, `.txt`, `.rst`), config
|
||||
* (`.json`, `.yaml`, `.yml`, `.toml`, `.env`, `.ini`), type declarations
|
||||
* (`.d.ts`), styles (`.css`, `.scss`), markup (`.html`, `.svg`), and lock
|
||||
* files. These are not implementation code — a comments or complexity check
|
||||
* flagging prose in a `.md` or a key in `package.json` is noise.
|
||||
*/
|
||||
export const SCOPE_EXTENSIONS: ReadonlySet<string> = new Set([
|
||||
".ts",
|
||||
".tsx",
|
||||
".js",
|
||||
".jsx",
|
||||
".mjs",
|
||||
".cjs",
|
||||
".py",
|
||||
".rb",
|
||||
".go",
|
||||
".rs",
|
||||
".java",
|
||||
".kt",
|
||||
".swift",
|
||||
".php",
|
||||
".cs",
|
||||
".lua",
|
||||
]);
|
||||
|
||||
/**
|
||||
* Directories pygienium never descends into — build output, dependency caches,
|
||||
* tooling state, and VCS metadata. When walking the tree with `glob`/`grep`/
|
||||
* `readdir`, skip these by name to avoid wasting tokens on vendored code and
|
||||
* generated artifacts the user can't act on.
|
||||
*/
|
||||
export const SCOPE_EXCLUDE_DIRS: ReadonlySet<string> = new Set([
|
||||
"node_modules",
|
||||
".git",
|
||||
".hg",
|
||||
".svn",
|
||||
"dist",
|
||||
"build",
|
||||
"out",
|
||||
"coverage",
|
||||
".next",
|
||||
".nuxt",
|
||||
".turbo",
|
||||
".svelte-kit",
|
||||
"__pycache__",
|
||||
".venv",
|
||||
"venv",
|
||||
"vendor",
|
||||
".cache",
|
||||
".pygienium",
|
||||
".ralpi",
|
||||
".idea",
|
||||
".vscode",
|
||||
]);
|
||||
|
||||
/**
|
||||
* Compound extensions (checked after the simple extension lookup) that should
|
||||
* be treated as non-source even though their tail extension appears in
|
||||
* {@link SCOPE_EXTENSIONS}. The primary case: `.d.ts` type declarations are
|
||||
* generated contracts, not implementation code.
|
||||
*/
|
||||
export const SCOPE_EXCLUDE_SUFFIXES: ReadonlySet<string> = new Set([
|
||||
".d.ts",
|
||||
".d.mts",
|
||||
".d.cts",
|
||||
".min.js",
|
||||
".min.mjs",
|
||||
".min.cjs",
|
||||
]);
|
||||
|
||||
/**
|
||||
* Test if a file path is implementation source pygienium should inspect.
|
||||
*
|
||||
* Returns `true` when the extension is in {@link SCOPE_EXTENSIONS} AND the
|
||||
* path does not end with a {@link SCOPE_EXCLUDE_SUFFIXES} pattern (e.g.
|
||||
* `.d.ts`).
|
||||
*/
|
||||
export function isScopeSource(path: string): boolean {
|
||||
const lower = path.toLowerCase();
|
||||
for (const suffix of SCOPE_EXCLUDE_SUFFIXES) {
|
||||
if (lower.endsWith(suffix)) return false;
|
||||
}
|
||||
const dot = lower.lastIndexOf(".");
|
||||
if (dot === -1) return false;
|
||||
return SCOPE_EXTENSIONS.has(lower.slice(dot));
|
||||
}
|
||||
|
||||
/**
|
||||
* Markdown section injected into every scan task string so the sub-agent knows
|
||||
* exactly what to inspect and what to skip — stated once here, not copy-pasted
|
||||
* into each task builder.
|
||||
*
|
||||
* Agents that use `glob`/`grep`/`readdir` for their own file discovery read
|
||||
* this before exploring, so the exclusion list governs their search too.
|
||||
*/
|
||||
export function scopeRulesMarkdown(): string {
|
||||
const extensions = [...SCOPE_EXTENSIONS]
|
||||
.sort((a, b) => a.localeCompare(b))
|
||||
.join("`, `");
|
||||
const excludeDirs = [...SCOPE_EXCLUDE_DIRS]
|
||||
.sort((a, b) => a.localeCompare(b))
|
||||
.join("`, `");
|
||||
return `## Scope of inspection
|
||||
|
||||
**Only inspect implementation source files.** Do not analyse documentation,
|
||||
config, type declarations, build output, or dependencies — flagging those is
|
||||
noise the user cannot act on.
|
||||
|
||||
### Inspect (extensions)
|
||||
\`${extensions}\`
|
||||
|
||||
### Skip (directory names — never descend into)
|
||||
\`${excludeDirs}\`
|
||||
|
||||
### Skip (file patterns)
|
||||
- Type declarations: \`*.d.ts\`, \`*.d.mts\`, \`*.d.cts\` — generated contracts, not impl
|
||||
- Minified bundles: \`*.min.js\`, \`*.min.mjs\`, \`*.min.cjs\` — generated, not editable
|
||||
- Docs: \`*.md\`, \`*.txt\`, \`*.rst\` — prose, not code
|
||||
- Config: \`*.json\`, \`*.yaml\`, \`*.yml\`, \`*.toml\`, \`*.ini\`, \`*.env\`
|
||||
- Styles/markup: \`*.css\`, \`*.scss\`, \`*.html\`, \`*.svg\`
|
||||
- Lock files: \`package-lock.json\`, \`*.lock\`, \`bun.lockb\`
|
||||
|
||||
### File discovery preference
|
||||
1. **Prefer the recon snapshot** at \`<cwd>/.pygienium/recon.json\` when it
|
||||
exists — it is the authoritative source inventory (git-tracked, extension-
|
||||
filtered, exclude-aware). Read its \`fileCounts\` for the quick picture.
|
||||
2. Otherwise enumerate files yourself, applying the rules above.
|
||||
3. When using \`glob\`/\`grep\`, add ignore patterns for the skip directories
|
||||
(e.g. exclude \`**/node_modules/**\` from your scans).
|
||||
`;
|
||||
}
|
||||
558
src/checks/todos.ts
Normal file
558
src/checks/todos.ts
Normal file
@@ -0,0 +1,558 @@
|
||||
/**
|
||||
* checks/todos.ts — "TODOs & stubs" check.
|
||||
*
|
||||
* Inventories unfinished work: TODO/FIXME/HACK markers and stub
|
||||
* implementations. The engineering rule encoded in the fix phase: pygienium
|
||||
* never *implements* a TODO and never deletes a marker — the fixer's only
|
||||
* action is to convert **silent stubs** (placeholder returns, empty bodies,
|
||||
* pass-only bodies) into loud failures, because a stub that silently returns
|
||||
* a plausible-but-wrong value ships the lie to every caller, while a stub
|
||||
* that throws is honest tracked debt.
|
||||
*
|
||||
* Classification (the scan agent applies judgment; a deterministic pre-scan
|
||||
* feeds it candidates):
|
||||
* - marker — `TODO` / `FIXME` / `HACK` / `XXX` / `@todo` in a comment.
|
||||
* - silent-stub — lone placeholder return / empty body / pass-only body;
|
||||
* the actionable, dangerous ones.
|
||||
* - loud-stub — explicit not-implemented failures (`throw new Error("Not
|
||||
* implemented")`, `todo!()`, `raise NotImplementedError`, `TODO("...")`);
|
||||
* already failing loudly → tracked debt, fixer never touches them.
|
||||
* - noise (dropped by the agent) — "TODO" inside a string literal, doc
|
||||
* examples, fixtures, abstract-method `NotImplementedError` (the correct
|
||||
* Python idiom), legit default returns (reducers, indexOf -1, catch
|
||||
* handlers returning null).
|
||||
*
|
||||
* Lifecycle:
|
||||
* gate (need source files) → recon (shared) → async scan task runs a
|
||||
* deterministic candidate pass over the scope tree, diffs the counts
|
||||
* against the previous run's findings (stored in run-state), hands the
|
||||
* candidates + delta to the `todos` agent, which verifies/drops noise and
|
||||
* writes `<cwd>/.pygienium/checks/todos/findings.md` → [with --fix] fixer
|
||||
* converts silent stubs to loud throws and writes `changes.md`.
|
||||
*
|
||||
* Registering this file is the ONLY wiring needed: `index.ts` auto-discovers
|
||||
* `src/checks/*.ts`, so dropping this file exposes `/pygienium-todos`.
|
||||
*
|
||||
* @module pygienium/checks/todos
|
||||
*/
|
||||
|
||||
import { readdirSync } from "node:fs";
|
||||
import { readFile, readdir, stat } from "node:fs/promises";
|
||||
import { join, relative } from "node:path";
|
||||
import { loadRunState } from "../run-state.js";
|
||||
import type { CheckDefinition, CheckScope } from "./registry.js";
|
||||
import {
|
||||
isScopeSource,
|
||||
SCOPE_EXCLUDE_DIRS,
|
||||
scopeRulesMarkdown,
|
||||
} from "./scope.js";
|
||||
|
||||
/** Output directory for this check's persistent reports. */
|
||||
export function todosOutputDir(cwd: string): string {
|
||||
return join(cwd, ".pygienium", "checks", "todos");
|
||||
}
|
||||
|
||||
/** `findings.md` path for this check. */
|
||||
export function findingsPath(cwd: string): string {
|
||||
return join(todosOutputDir(cwd), "findings.md");
|
||||
}
|
||||
|
||||
/** `changes.md` path for this check. */
|
||||
export function changesPath(cwd: string): string {
|
||||
return join(todosOutputDir(cwd), "changes.md");
|
||||
}
|
||||
|
||||
export type TodoKind = "marker" | "silent-stub" | "loud-stub";
|
||||
|
||||
/** One candidate line the deterministic pre-scan flagged. */
|
||||
export interface TodoCandidate {
|
||||
/** Absolute path of the file. */
|
||||
path: string;
|
||||
/** 1-based line number. */
|
||||
line: number;
|
||||
kind: TodoKind;
|
||||
/** Matched token (e.g. `TODO`, `Not implemented`, `empty-body`). */
|
||||
snippet: string;
|
||||
/** The trimmed line content. */
|
||||
code: string;
|
||||
/** Enclosing function name when one was seen, else the file path. */
|
||||
context: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Marker tokens: an unfinished-work note in a comment. Case-insensitive;
|
||||
* `@todo\b` (not `@todos`) and `\bHACK\b` (not `hacking`).
|
||||
*/
|
||||
const MARKER_RE = /\b(?:TODO|FIXME|HACK)\b|\bXXX\b|@todo\b/i;
|
||||
|
||||
/**
|
||||
* Loud-stub tokens: explicit not-implemented failures. `not implemented`
|
||||
* covers `throw new Error("Not implemented")` and `panic!("not implemented")`;
|
||||
* the `NotImplementedError` branch also catches Python's `raise
|
||||
* NotImplementedError`, and the Rust/Kotlin idioms (`todo!()`, `TODO("...")`)
|
||||
* are matched explicitly.
|
||||
*/
|
||||
const LOUD_STUB_RE =
|
||||
/not\s+implemented|NotImplementedError|NotImplementedException|\btodo!\s*\(|unimplemented!\s*\(|\bTODO\s*\(/i;
|
||||
|
||||
/** A lone placeholder return (`return 0;` / `return "";` / `return null;` …). */
|
||||
const PLACEHOLDER_RETURN_RE =
|
||||
/^\s*return\s+(?:null|undefined|0(?:\.0)?|""|''|\[\]|\{\}|None|nil|false)\s*;?\s*(?:\/\/.*)?$/;
|
||||
|
||||
/** Function/arrow header lines worth inspecting for a stub body. */
|
||||
const FN_HEADER_RE =
|
||||
/\b(?:function|def|func|fun|fn)\s+\w+|(?:const|let|var)\s+\w+\s*=\s*(?:async\s*)?\s*(?:\([^)]*\)|[A-Za-z_$][\w$]*)\s*=>/;
|
||||
|
||||
/** Single-line placeholder body: `function x() { return 0; }`. */
|
||||
const SINGLE_PLACEHOLDER_BODY_RE =
|
||||
/\{\s*return\s+(?:null|undefined|0(?:\.0)?|""|''|\[\]|\{\}|None|nil|false)\s*;?\s*\}/;
|
||||
|
||||
/** Single-line arrow expression body: `const f = () => 0;`. */
|
||||
const ARROW_PLACEHOLDER_RE =
|
||||
/=>\s*(?:null|undefined|0(?:\.0)?|""|''|\[\]|\{\}|false)\s*;?\s*$/;
|
||||
|
||||
/** Empty single-line body: `function notify(): void {}`. */
|
||||
const EMPTY_BODY_RE = /\{\s*\}/;
|
||||
|
||||
/** Hard cap on candidates so a huge tree can't blow the task prompt. */
|
||||
const MAX_CANDIDATES = 500;
|
||||
/** Candidate sections are truncated at this many entries in the fallback. */
|
||||
const FALLBACK_CAP = 16;
|
||||
/** Candidate list embedded in the live prompt is truncated at this many. */
|
||||
const PROMPT_CAP = 40;
|
||||
|
||||
/** Extract the declared function name from a header line, when present. */
|
||||
function headerName(line: string): string | undefined {
|
||||
const decl =
|
||||
/(?:function|def|func|fun|fn|class)\s+([A-Za-z_$][\w$]*)/.exec(line) ??
|
||||
/(?:const|let|var)\s+([A-Za-z_$][\w$]*)\s*=\s*(?:async\s*)?\s*(?:\([^)]*\)|[A-Za-z_$][\w$]*)\s*=>/.exec(
|
||||
line,
|
||||
);
|
||||
return decl?.[1];
|
||||
}
|
||||
|
||||
/** Index of the next non-blank line at or after `start`, else `undefined`. */
|
||||
function nextNonBlank(lines: string[], start: number): number | undefined {
|
||||
for (let i = start; i < lines.length; i++) {
|
||||
if ((lines[i] as string).trim()) return i;
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Walk the target collecting implementation-source files, honouring
|
||||
* {@link SCOPE_EXCLUDE_DIRS} and {@link isScopeSource} (same rules as
|
||||
* dead-code's walker).
|
||||
*/
|
||||
async function walkScopeFiles(root: string): Promise<string[]> {
|
||||
const st = await stat(root).catch(() => undefined);
|
||||
if (!st) return [];
|
||||
if (st.isFile()) return isScopeSource(root) ? [root] : [];
|
||||
const out: string[] = [];
|
||||
const stack = [root];
|
||||
while (stack.length > 0) {
|
||||
const dir = stack.pop() as string;
|
||||
let entries;
|
||||
try {
|
||||
entries = await readdir(dir, { withFileTypes: true });
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
for (const entry of entries) {
|
||||
const full = join(dir, entry.name);
|
||||
if (entry.isDirectory()) {
|
||||
if (SCOPE_EXCLUDE_DIRS.has(entry.name)) continue;
|
||||
stack.push(full);
|
||||
} else if (entry.isFile() && isScopeSource(entry.name)) {
|
||||
out.push(full);
|
||||
}
|
||||
}
|
||||
}
|
||||
return out.sort();
|
||||
}
|
||||
|
||||
/**
|
||||
* Deterministic pre-scan: flag marker/loud-stub/silent-stub candidates across
|
||||
* the target's scope tree. High recall by design — the scan agent verifies
|
||||
* each candidate and drops noise (in-string "TODO", doc examples, legit
|
||||
* default returns). Pure function of the tree: unit-testable without agents.
|
||||
*/
|
||||
export async function detectTodoStubs(
|
||||
target: string,
|
||||
): Promise<TodoCandidate[]> {
|
||||
const files = await walkScopeFiles(target);
|
||||
const out: TodoCandidate[] = [];
|
||||
for (const file of files) {
|
||||
const raw = await readFile(file, "utf8").catch(() => "");
|
||||
const lines = raw.split("\n");
|
||||
let lastFn = "";
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
const code = lines[i] as string;
|
||||
const trimmed = code.trim();
|
||||
if (!trimmed) continue;
|
||||
|
||||
const loud = LOUD_STUB_RE.exec(trimmed);
|
||||
if (loud) {
|
||||
out.push({
|
||||
path: file,
|
||||
line: i + 1,
|
||||
kind: "loud-stub",
|
||||
snippet: loud[0].slice(0, 40),
|
||||
code: trimmed,
|
||||
context: lastFn,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
const marker = MARKER_RE.exec(trimmed);
|
||||
if (marker) {
|
||||
out.push({
|
||||
path: file,
|
||||
line: i + 1,
|
||||
kind: "marker",
|
||||
snippet: marker[0],
|
||||
code: trimmed,
|
||||
context: lastFn,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
if (FN_HEADER_RE.test(trimmed)) {
|
||||
const name = headerName(trimmed);
|
||||
if (name) lastFn = name;
|
||||
const ctx = name ?? lastFn;
|
||||
// Single-line stub forms.
|
||||
if (
|
||||
SINGLE_PLACEHOLDER_BODY_RE.test(trimmed) ||
|
||||
ARROW_PLACEHOLDER_RE.test(trimmed)
|
||||
) {
|
||||
out.push({
|
||||
path: file,
|
||||
line: i + 1,
|
||||
kind: "silent-stub",
|
||||
snippet: "placeholder-return",
|
||||
code: trimmed,
|
||||
context: ctx,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
if (
|
||||
EMPTY_BODY_RE.test(trimmed) &&
|
||||
!/\b(?:return|throw)\b/.test(trimmed)
|
||||
) {
|
||||
out.push({
|
||||
path: file,
|
||||
line: i + 1,
|
||||
kind: "silent-stub",
|
||||
snippet: "empty-body",
|
||||
code: trimmed,
|
||||
context: ctx,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
// Multi-line forms: inspect the first non-blank body line.
|
||||
const bodyIdx = nextNonBlank(lines, i + 1);
|
||||
if (bodyIdx === undefined) continue;
|
||||
const body = (lines[bodyIdx] as string).trim();
|
||||
if (body === "}") {
|
||||
out.push({
|
||||
path: file,
|
||||
line: i + 1,
|
||||
kind: "silent-stub",
|
||||
snippet: "empty-body",
|
||||
code: trimmed,
|
||||
context: ctx,
|
||||
});
|
||||
} else if (body === "pass") {
|
||||
out.push({
|
||||
path: file,
|
||||
line: bodyIdx + 1,
|
||||
kind: "silent-stub",
|
||||
snippet: "pass-only",
|
||||
code: body,
|
||||
context: ctx,
|
||||
});
|
||||
} else if (PLACEHOLDER_RETURN_RE.test(body)) {
|
||||
// Lone placeholder return: the statement after it must be the
|
||||
// closing brace (a `try/catch { return null }` handler does not
|
||||
// match — its `return null` is followed by `}` inside `catch`).
|
||||
const after = nextNonBlank(lines, bodyIdx + 1);
|
||||
if (after !== undefined && (lines[after] as string).trim() === "}") {
|
||||
out.push({
|
||||
path: file,
|
||||
line: bodyIdx + 1,
|
||||
kind: "silent-stub",
|
||||
snippet: "placeholder-return",
|
||||
code: body,
|
||||
context: ctx,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if (out.length >= MAX_CANDIDATES) break;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/** Counts by kind across a candidate list. */
|
||||
function countByKind(candidates: TodoCandidate[]): {
|
||||
silent: number;
|
||||
loud: number;
|
||||
marker: number;
|
||||
} {
|
||||
let silent = 0;
|
||||
let loud = 0;
|
||||
let marker = 0;
|
||||
for (const c of candidates) {
|
||||
if (c.kind === "silent-stub") silent++;
|
||||
else if (c.kind === "loud-stub") loud++;
|
||||
else marker++;
|
||||
}
|
||||
return { silent, loud, marker };
|
||||
}
|
||||
|
||||
/** Previous run's verified counts, parsed from run-state findings text. */
|
||||
const PRIOR_SUMMARY_RE =
|
||||
/todos:\s*(\d+)\s+silent\s+stub\(s\)?,\s*(\d+)\s+loud\s+stub\(s\)?,\s*(\d+)\s+marker\(s\)?/;
|
||||
|
||||
/**
|
||||
* Parse the previous run's per-kind counts out of run-state (the scan agent's
|
||||
* one-line summary is persisted there). `undefined` when there is no prior
|
||||
* run or the stored summary isn't parseable — the delta is then all-new.
|
||||
*/
|
||||
export async function todosPriorCounts(
|
||||
cwd: string,
|
||||
): Promise<{ silent: number; loud: number; marker: number } | undefined> {
|
||||
const state = await loadRunState(cwd).catch(() => undefined);
|
||||
const stored = state?.checks["todos"]?.findings;
|
||||
if (!stored) return undefined;
|
||||
const m = PRIOR_SUMMARY_RE.exec(stored);
|
||||
if (!m) return undefined;
|
||||
return {
|
||||
silent: Number(m[1]),
|
||||
loud: Number(m[2]),
|
||||
marker: Number(m[3]),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Render the deterministic report the fake runner writes (and the real agent
|
||||
* uses as a shape reference): one pipe-separated line, sections per kind.
|
||||
*/
|
||||
function renderFindings(
|
||||
cwd: string,
|
||||
candidates: TodoCandidate[],
|
||||
delta: { nw: number; resolved: number },
|
||||
): string {
|
||||
const { silent, loud, marker } = countByKind(candidates);
|
||||
const total = silent + loud + marker;
|
||||
const parts = [
|
||||
`# TODOs & stubs findings | summary: ${marker} marker(s), ${silent} silent stub(s), ${loud} loud stub(s) | new: ${delta.nw} | resolved: ${delta.resolved} | reviewed: ${candidates.length}`,
|
||||
];
|
||||
if (total === 0) {
|
||||
parts.push(
|
||||
"No TODOs or stubs detected (deterministic pre-scan reviewed all inspected source).",
|
||||
);
|
||||
return parts.join(" | ");
|
||||
}
|
||||
const byKind: Record<TodoKind, TodoCandidate[]> = {
|
||||
marker: [],
|
||||
"silent-stub": [],
|
||||
"loud-stub": [],
|
||||
};
|
||||
for (const c of candidates) byKind[c.kind].push(c);
|
||||
const dump = (title: string, list: TodoCandidate[]): void => {
|
||||
parts.push(`## ${title}`);
|
||||
list.slice(0, FALLBACK_CAP).forEach((c, i) => {
|
||||
parts.push(
|
||||
`### ${i + 1}. ${relative(cwd, c.path)}:${c.line} — ${c.code} | snippet: ${c.snippet} | context: ${c.context || relative(cwd, c.path)}`,
|
||||
);
|
||||
});
|
||||
if (list.length > FALLBACK_CAP) {
|
||||
parts.push(`... and ${list.length - FALLBACK_CAP} more (truncated)`);
|
||||
}
|
||||
};
|
||||
dump("TODO markers", byKind.marker);
|
||||
dump("Silent stubs (actionable)", byKind["silent-stub"]);
|
||||
dump(
|
||||
"Loud stubs (already failing loudly — tracked debt)",
|
||||
byKind["loud-stub"],
|
||||
);
|
||||
return parts.join(" | ");
|
||||
}
|
||||
|
||||
/**
|
||||
* Gate: skip when the cwd has no inspectable source files at all — a workspace
|
||||
* with zero source files gives the scanner nothing to analyse.
|
||||
*/
|
||||
function todosGate(cwd: string): string | undefined {
|
||||
let found = false;
|
||||
try {
|
||||
const entries = readdirSync(cwd);
|
||||
for (const entry of entries) {
|
||||
if (isScopeSource(entry)) {
|
||||
found = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// unreadable cwd → let the agent decide; don't block.
|
||||
return undefined;
|
||||
}
|
||||
if (!found) {
|
||||
return "no source files found to inspect";
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the scan task. Deterministic pre-scan (async, like dead-code's) finds
|
||||
* candidates and diffs them against the previous run's counts; the `todos`
|
||||
* agent verifies each candidate, drops noise, and writes the verified report
|
||||
* to `findings.md`. The output path is passed into the task so both the real
|
||||
* agent (which uses its `write` tool) and the deterministic fake runner
|
||||
* (which understands `!write <path> <text>`) persist to the same location.
|
||||
*/
|
||||
export async function buildTodosScanTask(
|
||||
cwd: string,
|
||||
scope: CheckScope,
|
||||
): Promise<string> {
|
||||
const findings = findingsPath(cwd);
|
||||
const target = scope.target;
|
||||
const candidates = await detectTodoStubs(target);
|
||||
const prior = await todosPriorCounts(cwd);
|
||||
const { silent, loud, marker } = countByKind(candidates);
|
||||
const prevTotal = prior ? prior.silent + prior.loud + prior.marker : 0;
|
||||
const total = silent + loud + marker;
|
||||
const nw = Math.max(0, total - prevTotal);
|
||||
const resolved = Math.max(0, prevTotal - total);
|
||||
const report = renderFindings(cwd, candidates, { nw, resolved });
|
||||
|
||||
const candidateList = candidates
|
||||
.slice(0, PROMPT_CAP)
|
||||
.map((c) => ` - ${relative(cwd, c.path)}:${c.line} [${c.kind}] ${c.code}`);
|
||||
if (candidates.length > PROMPT_CAP) {
|
||||
candidateList.push(
|
||||
` - … and ${candidates.length - PROMPT_CAP} more (truncated for brevity)`,
|
||||
);
|
||||
}
|
||||
|
||||
return [
|
||||
`Inspect the target "${target}" (cwd: ${cwd}) for unfinished work: TODO markers and stub implementations.`,
|
||||
`A deterministic pre-scan found ${candidates.length} candidate line(s). Verify each candidate:`,
|
||||
...candidateList,
|
||||
``,
|
||||
`Classify against the todos rubric: markers (TODO/FIXME/HACK/XXX/@todo), silent stubs`,
|
||||
`(placeholder return / empty body / pass-only body), loud stubs (not-implemented`,
|
||||
`throws, todo!(), TODO("..."), raise NotImplementedError).`,
|
||||
`Drop noise: "TODO" inside a string literal, doc examples, fixtures,`,
|
||||
`abstract-method NotImplementedError (correct Python idiom), and legit default`,
|
||||
`returns (a reducer returning 0, indexOf returning -1, a catch handler returning null).`,
|
||||
`Previous run reported: ${
|
||||
prior
|
||||
? `${prior.silent} silent, ${prior.loud} loud, ${prior.marker} marker`
|
||||
: "none (first run)"
|
||||
}.`,
|
||||
`Write your full verified report to: ${findings}.`,
|
||||
`findings.md must begin with the machine-readable summary line, then the three`,
|
||||
`sections (## TODO markers / ## Silent stubs (actionable) / ## Loud stubs ...),`,
|
||||
`each entry with file:line, evidence, and disposition. The summary line MUST be:`,
|
||||
`summary: <M> marker(s), <S> silent stub(s), <L> loud stub(s) | new: <N> | resolved: <R> | reviewed: <K>`,
|
||||
``,
|
||||
scopeRulesMarkdown(),
|
||||
``,
|
||||
`# Deterministic fallback (executed by the fake runner in tests — verify every item yourself;`,
|
||||
`# do not copy the counts below blindly):`,
|
||||
`!write ${findings} ${report}`,
|
||||
`!echo todos: ${silent} silent stub(s), ${loud} loud stub(s), ${marker} marker(s) — see ${findings}`,
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the fix task. The fixer converts ONLY silent stubs into loud failures,
|
||||
* per language idiom, preserving signature/exports; records every conversion
|
||||
* (and any kept-with-reason) in `changes.md`. Markers are never implemented
|
||||
* or deleted; loud stubs are never touched.
|
||||
*/
|
||||
function buildTodosFixTask(
|
||||
cwd: string,
|
||||
scope: CheckScope,
|
||||
findings: string,
|
||||
): string {
|
||||
const changes = changesPath(cwd);
|
||||
const target = scope.target;
|
||||
return [
|
||||
`Convert silent stubs to loud failures (the fix phase of the todos check).`,
|
||||
`cwd: ${cwd} target: ${target}`,
|
||||
`Scan summary (also persisted at ${findingsPath(cwd)}):`,
|
||||
`---`,
|
||||
findings,
|
||||
`---`,
|
||||
``,
|
||||
`Rules:`,
|
||||
`- Convert ONLY silent stubs. For each, replace the placeholder body with an explicit`,
|
||||
` loud failure naming the function, using the project's language idiom:`,
|
||||
` TS/JS/C#/Java: throw new Error("todos: <fn>() is a stub");`,
|
||||
` Python: raise NotImplementedError("<fn> is a stub")`,
|
||||
` Go: panic("todos: <fn> is a stub")`,
|
||||
` Rust: todo!("<fn> is a stub")`,
|
||||
` generic: throw new Error("todos: <fn> is a stub")`,
|
||||
`- Preserve the signature, exports, async-ness, and type shape of the function.`,
|
||||
`- Leave the original placeholder as a comment directly above the throw, and add a`,
|
||||
` note that the stub was made loud by pygienium.`,
|
||||
`- NEVER implement TODOs, NEVER delete unresolved markers, NEVER touch loud stubs`,
|
||||
` (they already fail loudly), NEVER touch code that is not a verified silent stub.`,
|
||||
`- If a candidate turned out NOT to be a stub (a legit default return), keep it and`,
|
||||
` record it as kept-with-reason in changes.md.`,
|
||||
`- Apply the smallest possible diff; preserve tests and conventions.`,
|
||||
`- Write changes.md to ${changes} listing every conversion (auto) or keep (reason).`,
|
||||
``,
|
||||
`# Deterministic conversion (executed by the fake runner in tests):`,
|
||||
`# getPrice() + notify() in stubs.ts converted from silent placeholders to loud throws.`,
|
||||
`# NOTE: the fallback writes one physical line (the fake runner takes the rest of the !write`,
|
||||
`# line as file content); trailing // comments keep the single line valid source.`,
|
||||
`!write ${target}/stubs.ts export function getPrice(): number { throw new Error("todos: getPrice() is a stub"); } export function notify(): void { throw new Error("todos: notify() is a stub"); } export function connect(): Promise<void> { throw new Error("Not implemented"); } // TODO: add pagination // Cleaned by pygienium-todos: converted 2 silent stubs (getPrice, notify) to loud failures.`,
|
||||
`!write ${changes} # TODOs & stubs changes | summary: 2 silent stub(s) converted to loud, 0 kept | ## Converted to loud (auto) | 1. ${target}/stubs.ts:3 — getPrice() — body was \`return 0\` placeholder; now throws \`todos: getPrice() is a stub\` | 2. ${target}/stubs.ts:6 — notify() — body was empty; now throws \`todos: notify() is a stub\` | ## Kept (with reason) | (none)`,
|
||||
`!echo todos: 2 silent stub(s) converted — see ${changes}`,
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
/**
|
||||
* Verify hook: confirms the check actually produced its artifacts — after the
|
||||
* scan phase `findings.md` must exist; after the fix phase `changes.md` must
|
||||
* exist too. Returns an error string to fail verify, or `undefined` to pass.
|
||||
*/
|
||||
async function todosVerify(scope: CheckScope): Promise<string | undefined> {
|
||||
const f = findingsPath(scope.cwd);
|
||||
try {
|
||||
await stat(f);
|
||||
} catch {
|
||||
return `todos verify: expected findings.md at ${f} after scan, none found.`;
|
||||
}
|
||||
if (scope.fix) {
|
||||
const c = changesPath(scope.cwd);
|
||||
try {
|
||||
await stat(c);
|
||||
} catch {
|
||||
return `todos verify: expected changes.md at ${c} after --fix, none found.`;
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/** The todos check definition; registers itself on import. */
|
||||
export const check: CheckDefinition = {
|
||||
name: "todos",
|
||||
label: "TODOs & stubs",
|
||||
description:
|
||||
"Inventory TODO/FIXME markers and stub implementations; with --fix, convert silent stubs (placeholder returns, empty bodies) into loud failures — never implementing TODOs.",
|
||||
agentName: "todos",
|
||||
phaseId: "analysis",
|
||||
buildScanTask: buildTodosScanTask,
|
||||
buildFixTask: buildTodosFixTask,
|
||||
gate: todosGate,
|
||||
verify: todosVerify,
|
||||
};
|
||||
|
||||
// No self-registration here: `index.ts` auto-discovers every `checks/*.ts`
|
||||
// that exports `check` and registers it.
|
||||
Reference in New Issue
Block a user