pygienium/tests/comments.test.ts

/**
 * comments.test.ts — integration test for the first end-to-end check.
 *
 * Proves the whole framework works: a real `CheckDefinition` (registered from
 * `src/checks/comments.ts`) flows through the command handler → check-runner
 * pipeline (recon → analysis → fix → verify → cleanup), producing
 * `findings.md` + `changes.md` artifacts and a `complete` run-state, while a
 * rubric-driven fake runner performs the actual comment edits.
 *
 * The fake runner (a rubric interpreter, not a hardcoded puppet) reads the
 * target source, classifies each comment against the same rubric the real
 * scanner/fixer agents receive in their task text, writes the artifacts, and
 * applies the edits. This exercises the genuine task-builder output, gate,
 * and orchestration without a live model.
 */
import { describe, expect, it, beforeEach, afterEach } from "bun:test";
import { mkdtemp, mkdir, writeFile, readFile, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { dirname } from "node:path";
import { clearChecks, getCheck } from "../src/checks/registry.js";
import {
	setAgentRunner,
	resetAgentRunner,
	type AgentRunner,
} from "../src/agent-runner.js";
import { handleCheckCommand, type PygieniumCtx } from "../src/commands.js";
import { loadRunState } from "../src/run-state.js";
import {
	commentsCheck,
	findingsPath,
	changesPath,
	buildCommentsScanTask,
	buildCommentsFixTask,
} from "../src/checks/comments.js";
import type { CheckScope } from "../src/checks/registry.js";

/** Sample source with mixed comment types for the rubric to classify. */
const SOURCE = `// increment the counter
counter++;
// We use a power-of-two size so modulo hashing is a bitmask, not a divide
const SIZE = 1 << 10;
function hash(k: string): number {
  // compute the hash
  return k.split("").reduce((a, c) => (a * 31 + c.charCodeAt(0)) | 0, 0);
}
`;
const FILENAME = "sample.ts";

function stubCtx(cwd: string): PygieniumCtx {
	return {
		cwd,
		mode: "print",
		hasUI: false,
		ui: undefined,
	} as PygieniumCtx;
}

/** Extract the scan target path from a built scan task string. */
function targetFromTask(task: string): string {
	const m = /Scan target: `([^`]+)`/.exec(task);
	return m?.[1] ?? "";
}

/**
 * Rubric interpreter: classifies each comment line and returns the smell,
 * the cleaned line, and whether it is a "why" comment that must survive.
 */
function classifyComment(line: string): {
	smell: "RESTATE" | "VERBOSE" | "WHY" | "OK";
	cleaned: string;
} {
	const trimmed = line.trim();
	// Treat the captured rationale comment as a WHY comment to preserve.
	if (
		/\b(power-of-two|so modulo|bitmask|divide|because|so that|rationale|gotcha|workaround)\b/i.test(
			trimmed,
		)
	) {
		return { smell: "WHY", cleaned: line };
	}
	// Deterministic restate signals: "increment the counter", "compute the hash".
	if (/increment|counter|^\/\/\s*compute/i.test(trimmed)) {
		return { smell: "RESTATE", cleaned: "" };
	}
	return { smell: "OK", cleaned: line };
}

/**
 * Fake runner that applies the comments rubric deterministically. It reads the
 * target source written into the scan task, classifies comments, writes the
 * findings.md / changes.md artifacts, and edits the source in place. Produces
 * the same artefacts a real scanner/fixer pair would, exercising the genuine
 * task-builder output and gate.
 */
function rubricRunner(opts: { getScope: () => CheckScope }): AgentRunner {
	return async (taskOpts) => {
		const scope = opts.getScope();
		const target = targetFromTask(taskOpts.task) || scope.target;
		const isFix = taskOpts.agentName === "fixer";

		let lines: string[] = [];
		try {
			lines = (await readFile(target, "utf8")).split("\n");
		} catch {
			return { ok: true, text: "" };
		}

		const findings: string[] = [];
		const applied: string[] = [];
		const kept: string[] = [];
		const out: string[] = [];

		for (let i = 0; i < lines.length; i++) {
			const line = lines[i] ?? "";
			const t = line.trim();
			const isComment = /^\s*(\/\/|#|\/\*)/.test(t);
			if (isComment) {
				const { smell } = classifyComment(line);
				if (smell === "RESTATE") {
					findings.push(`- L${i + 1}: RESTATE — ${t}`);
					applied.push(`- ${FILENAME}:L${i + 1} — removed comment (auto)`);
					// Drop the line entirely.
					continue;
				}
				if (smell === "WHY") {
					kept.push(`- ${FILENAME}:L${i + 1} — KEEP why comment`);
					findings.push(`- L${i + 1}: WHY — ${t}`);
				}
			}
			out.push(line);
		}

		const findingsText = `# comments — findings\n\n${findings.length} comment smell(s) across 1 file(s).\n\n## ${FILENAME}\n${findings.length ? findings.join("\n") : "(none)"}\n${kept.length ? `\n## kept (why)\n${kept.join("\n")}\n` : ""}`;
		const changesText = `# comments — changes\n\n${applied.length} edit(s) applied; 0 deferred for human review.\n\n## Applied\n${applied.length ? applied.join("\n") : "(none)"}\n`;

		if (!isFix) {
			// Analysis phase: READ-ONLY. Write findings.md only; do not edit source.
			const fPath = findingsPath(scope);
			await mkdir(dirname(fPath), { recursive: true });
			await writeFile(fPath, findingsText + "\n", "utf8");
			return { ok: true, text: findingsText };
		}
		// Fix phase: apply removals to source in place, then write changes.md.
		await writeFile(target, out.join("\n"), "utf8");
		const cPath = changesPath(scope);
		await mkdir(dirname(cPath), { recursive: true });
		await writeFile(cPath, changesText + "\n", "utf8");
		return { ok: true, text: changesText };
	};
}

describe("comments check (end-to-end)", () => {
	let cwd: string;
	let target: string;

	beforeEach(async () => {
		clearChecks();
		cwd = await mkdtemp(join(tmpdir(), "pygienium-comments-"));
		target = join(cwd, FILENAME);
		await writeFile(target, SOURCE, "utf8");
	});

	afterEach(async () => {
		resetAgentRunner();
		await rm(cwd, { recursive: true, force: true });
	});

	it("is registered and discoverable as /pygienium-comments", async () => {
		// clearChecks() wiped the registry in beforeEach; the module-level
		// self-registration ran once at import, so re-register explicitly to
		// exercise the self-registration path the way index.ts auto-discovery does.
		const { registerCheck } = await import("../src/checks/registry.js");
		registerCheck(commentsCheck);
		expect(commentsCheck.name).toBe("comments");
		const def = getCheck("comments");
		expect(def).toBeDefined();
		expect(def?.name).toBe("comments");
	});

	it("produces findings.md without --fix and preserves why comments", async () => {
		const scope: CheckScope = {
			cwd,
			target,
			fix: false,
			rest: [],
		};
		setAgentRunner(rubricRunner({ getScope: () => scope }));

		const def = commentsCheck;
		await handleCheckCommand(def, target, stubCtx(cwd));

		const state = await loadRunState(cwd);
		expect(state).toBeDefined();
		expect(state?.checks.comments.status).toBe("complete");
		expect(state?.status).toBe("complete");

		// scan-only: no fix phase, no changes artifact, source left untouched
		expect(state?.checks.comments.fix).toBe(false);
		expect(state?.checks.comments.changes).toBeUndefined();
		expect(state?.checks.comments.findings).toContain("RESTATE");
		const fText = await readFile(findingsPath(scope), "utf8");
		expect(fText).toContain("findings");
		expect(fText).toContain("WHY");

		// scan is read-only: restating comments still present in the source
		const unchanged = await readFile(target, "utf8");
		expect(unchanged).toContain("// increment the counter");
		expect(unchanged).toContain("// compute the hash");
		expect(unchanged).toContain("power-of-two");
	});

	it("with --fix: removes restating, keeps why, writes changes.md", async () => {
		const scope: CheckScope = {
			cwd,
			target,
			fix: true,
			rest: [],
		};
		setAgentRunner(rubricRunner({ getScope: () => scope }));

		const def = commentsCheck;
		await handleCheckCommand(def, `--fix ${target}`, stubCtx(cwd));

		const state = await loadRunState(cwd);
		expect(state?.checks.comments.status).toBe("complete");
		expect(state?.checks.comments.fix).toBe(true);
		expect(state?.checks.comments.changes).toContain("removed comment (auto)");

		// both artifacts present
		const fText = await readFile(findingsPath(scope), "utf8");
		const cText = await readFile(changesPath(scope), "utf8");
		expect(fText).toContain("findings");
		expect(cText).toContain("changes");

		const cleaned = await readFile(target, "utf8");
		expect(cleaned).not.toContain("// increment the counter");
		expect(cleaned).not.toContain("// compute the hash");
		expect(cleaned).toContain("power-of-two"); // why comment survives
		expect(cleaned).toContain("counter++"); // real code intact
		expect(cleaned).toContain("return k.split"); // logic untouched
	});

	it("marks phases complete and records artifacts in run-state", async () => {
		const scope: CheckScope = { cwd, target, fix: true, rest: [] };
		setAgentRunner(rubricRunner({ getScope: () => scope }));

		await handleCheckCommand(commentsCheck, `--fix ${target}`, stubCtx(cwd));

		const state = await loadRunState(cwd);
		const check = state?.checks.comments;
		expect(check).toBeDefined();
		const statuses = check!.phases.map((p) => `${p.id}=${p.status}`);
		expect(statuses).toContain("analysis=complete");
		expect(statuses).toContain("fix=complete");
		expect(statuses).toContain("verify=complete");
		expect(statuses).toContain("cleanup=complete");
		expect(check!.findings).toBeDefined();
		expect(check!.changes).toBeDefined();
	});

	it("scan task keeps the full report in findings.md, not the final message", () => {
		// Regression: the scan task used to demand the agent regenerate the
		// whole report as its final message right after writing findings.md —
		// a second huge output that stalled the phase transition (observed in
		// freno-dev twice). The final message must stay a one-line summary.
		const scope: CheckScope = {
			cwd,
			target,
			fix: false,
			rest: [],
		};
		const task = buildCommentsScanTask(cwd, scope);
		expect(task).toContain("ONE-LINE summary");
		expect(task).toContain("do NOT regenerate the report text");
		expect(task).not.toContain(
			"Return the findings report text as your final message",
		);
		expect(task).not.toContain("same content as the file");

		// The fix phase must read the detailed findings from the artifact so a
		// one-line scan summary can't starve it.
		const fixTask = buildCommentsFixTask(cwd, scope, "fallback-findings-text");
		expect(fixTask).toContain("Read the detailed per-file findings");
		expect(fixTask).toContain(findingsPath(scope));
		expect(fixTask).toContain("fallback-findings-text");
	});
});