From 2757d743daa49d9e2d1ec0fca8359fb2d85a4b4e Mon Sep 17 00:00:00 2001 From: Jack Felke Date: Thu, 26 Feb 2026 14:45:32 -0700 Subject: [PATCH] refactor: extract preflight-check helpers into testable module - Extract extractFilePaths, detectAmbiguity, estimateComplexity, classifyRisk, splitSubtasks into src/lib/preflight-helpers.ts - Update preflight-check.ts to import from helpers module - Add 22 tests covering all extracted helper functions - All 65 tests passing (up from 43) --- src/lib/preflight-helpers.ts | 45 +++++++++ src/tools/preflight-check.ts | 26 +---- tests/lib/preflight-helpers.test.ts | 150 ++++++++++++++++++++++++++++ 3 files changed, 200 insertions(+), 21 deletions(-) create mode 100644 src/lib/preflight-helpers.ts create mode 100644 tests/lib/preflight-helpers.test.ts diff --git a/src/lib/preflight-helpers.ts b/src/lib/preflight-helpers.ts new file mode 100644 index 0000000..8c2330e --- /dev/null +++ b/src/lib/preflight-helpers.ts @@ -0,0 +1,45 @@ +// ============================================================================= +// Pure helper functions for preflight_check +// Extracted for testability — no side effects, no server dependency. +// ============================================================================= + +/** Extract file paths from prompt text */ +export function extractFilePaths(prompt: string): string[] { + const matches = prompt.match(/[\w\-./\\]+\.\w{1,6}/g) || []; + return [...new Set(matches)]; +} + +/** Detect ambiguity signals in a prompt */ +export function detectAmbiguity(prompt: string, filePaths: string[]): string[] { + const issues: string[] = []; + if (/\b(it|them|the thing|that|those|this|these)\b/i.test(prompt)) + issues.push("Contains vague pronouns — clarify what 'it'/'them' refers to"); + if (/\b(fix|update|change|refactor|improve)\b/i.test(prompt) && !filePaths.length) + issues.push("Vague verb without specific file targets"); + if (prompt.trim().length < 40) + issues.push("Very short prompt — likely missing context"); + return issues; +} + +/** Estimate scope complexity based on file paths */ +export function estimateComplexity(filePaths: string[]): "SMALL" | "MEDIUM" | "LARGE" { + const hasMultipleFiles = filePaths.length > 3; + const hasMultipleDirs = new Set(filePaths.map(f => f.split("/")[0])).size > 2; + return hasMultipleFiles && hasMultipleDirs ? "LARGE" : filePaths.length > 1 ? "MEDIUM" : "SMALL"; +} + +/** Classify risk level for a task description */ +export function classifyRisk(text: string): "🔴 HIGH" | "🟡 MEDIUM" | "🟢 LOW" { + if (/schema|migrat|database|config|env|deploy/i.test(text)) return "🔴 HIGH"; + if (/api|route|endpoint/i.test(text)) return "🟡 MEDIUM"; + return "🟢 LOW"; +} + +/** Split a prompt into sequenced sub-tasks */ +export function splitSubtasks(prompt: string): string[] { + const parts = prompt + .split(/\b(?:then|after that|next|finally)\b|(?:,\s*and\s+)|(?:\band\b(?=\s+(?:update|add|remove|create|fix|change|refactor|implement|deploy)))/i) + .map(s => s.trim()) + .filter(s => s.length > 5); + return parts.length > 1 ? parts : [prompt]; +} diff --git a/src/tools/preflight-check.ts b/src/tools/preflight-check.ts index 8c9121a..719abe2 100644 --- a/src/tools/preflight-check.ts +++ b/src/tools/preflight-check.ts @@ -12,17 +12,12 @@ import { getConfig } from "../lib/config.js"; import { searchSemantic } from "../lib/timeline-db.js"; import { basename, join } from "path"; import { loadPatterns, matchPatterns, formatPatternMatches } from "../lib/patterns.js"; +import { extractFilePaths, detectAmbiguity, estimateComplexity, classifyRisk, splitSubtasks } from "../lib/preflight-helpers.js"; // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- -/** Extract file paths from prompt text */ -function extractFilePaths(prompt: string): string[] { - const matches = prompt.match(/[\w\-./\\]+\.\w{1,6}/g) || []; - return [...new Set(matches)]; -} - /** Verify files exist and return stats */ function verifyFiles(paths: string[]): string[] { const lines: string[] = []; @@ -104,10 +99,7 @@ function buildClarifySection(prompt: string): string[] { } // Ambiguity signals - const issues: string[] = []; - if (/\b(it|them|the thing|that|those|this|these)\b/i.test(prompt)) issues.push("Contains vague pronouns — clarify what 'it'/'them' refers to"); - if (/\b(fix|update|change|refactor|improve)\b/i.test(prompt) && !extractFilePaths(prompt).length) issues.push("Vague verb without specific file targets"); - if (prompt.trim().length < 40) issues.push("Very short prompt — likely missing context"); + const issues = detectAmbiguity(prompt, extractFilePaths(prompt)); if (issues.length > 0) { sections.push(`### ⚠️ Clarification Needed\n${issues.map(i => `- ${i}`).join("\n")}`); @@ -127,9 +119,7 @@ function buildScopeSection(prompt: string): string[] { } // Estimate complexity - const hasMultipleFiles = filePaths.length > 3; - const hasMultipleDirs = new Set(filePaths.map(f => f.split("/")[0])).size > 2; - const complexity = hasMultipleFiles && hasMultipleDirs ? "LARGE" : filePaths.length > 1 ? "MEDIUM" : "SMALL"; + const complexity = estimateComplexity(filePaths); sections.push(`### Scope: ${complexity}`); return sections; @@ -139,17 +129,11 @@ function buildScopeSection(prompt: string): string[] { function buildSequenceSection(prompt: string): string[] { // Split prompt into sub-tasks const subtasks: string[] = []; - - // Split on "and", "then", numbered lists, bullet points - const parts = prompt - .split(/\b(?:then|after that|next|finally)\b|(?:,\s*and\s+)|(?:\band\b(?=\s+(?:update|add|remove|create|fix|change|refactor|implement|deploy)))/i) - .map(s => s.trim()) - .filter(s => s.length > 5); + const parts = splitSubtasks(prompt); if (parts.length > 1) { for (let i = 0; i < parts.length; i++) { - const risk = /schema|migrat|database|config|env|deploy/i.test(parts[i]) ? "🔴 HIGH" : - /api|route|endpoint/i.test(parts[i]) ? "🟡 MEDIUM" : "🟢 LOW"; + const risk = classifyRisk(parts[i]); subtasks.push(`${i + 1}. ${parts[i].charAt(0).toUpperCase() + parts[i].slice(1)} — Risk: ${risk}`); } } else { diff --git a/tests/lib/preflight-helpers.test.ts b/tests/lib/preflight-helpers.test.ts new file mode 100644 index 0000000..4396cc8 --- /dev/null +++ b/tests/lib/preflight-helpers.test.ts @@ -0,0 +1,150 @@ +import { describe, it, expect } from "vitest"; +import { + extractFilePaths, + detectAmbiguity, + estimateComplexity, + classifyRisk, + splitSubtasks, +} from "../../src/lib/preflight-helpers.js"; + +// --------------------------------------------------------------------------- +// extractFilePaths +// --------------------------------------------------------------------------- +describe("extractFilePaths", () => { + it("extracts simple file paths", () => { + expect(extractFilePaths("fix the bug in src/index.ts")).toContain("src/index.ts"); + }); + + it("extracts multiple unique paths", () => { + const result = extractFilePaths("update src/a.ts and src/b.ts, also src/a.ts again"); + expect(result).toEqual(["src/a.ts", "src/b.ts"]); + }); + + it("returns empty for no file paths", () => { + expect(extractFilePaths("just do the thing")).toEqual([]); + }); + + it("handles nested paths", () => { + const result = extractFilePaths("edit src/lib/config.ts"); + expect(result).toContain("src/lib/config.ts"); + }); + + it("handles windows-style paths", () => { + const result = extractFilePaths("check src\\utils\\helper.js"); + expect(result).toContain("src\\utils\\helper.js"); + }); +}); + +// --------------------------------------------------------------------------- +// detectAmbiguity +// --------------------------------------------------------------------------- +describe("detectAmbiguity", () => { + it("flags vague pronouns", () => { + const issues = detectAmbiguity("fix it please", []); + expect(issues.some(i => /vague pronoun/i.test(i))).toBe(true); + }); + + it("flags vague verbs without file targets", () => { + const issues = detectAmbiguity("refactor the authentication module to be cleaner", []); + expect(issues.some(i => /vague verb/i.test(i))).toBe(true); + }); + + it("does NOT flag vague verbs when files are present", () => { + const issues = detectAmbiguity("refactor the authentication module", ["src/auth.ts"]); + expect(issues.some(i => /vague verb/i.test(i))).toBe(false); + }); + + it("flags very short prompts", () => { + const issues = detectAmbiguity("fix bug", []); + expect(issues.some(i => /short prompt/i.test(i))).toBe(true); + }); + + it("returns empty for clear prompts", () => { + const issues = detectAmbiguity( + "Add a new endpoint in src/routes/users.ts returning paginated results from the users table", + ["src/routes/users.ts"] + ); + expect(issues).toEqual([]); + }); +}); + +// --------------------------------------------------------------------------- +// estimateComplexity +// --------------------------------------------------------------------------- +describe("estimateComplexity", () => { + it("returns SMALL for 0-1 files", () => { + expect(estimateComplexity([])).toBe("SMALL"); + expect(estimateComplexity(["src/a.ts"])).toBe("SMALL"); + }); + + it("returns MEDIUM for 2-3 files", () => { + expect(estimateComplexity(["src/a.ts", "src/b.ts"])).toBe("MEDIUM"); + }); + + it("returns LARGE for 4+ files across 3+ dirs", () => { + expect(estimateComplexity([ + "src/a.ts", "lib/b.ts", "tests/c.ts", "config/d.yml" + ])).toBe("LARGE"); + }); + + it("returns MEDIUM for many files in few dirs", () => { + expect(estimateComplexity([ + "src/a.ts", "src/b.ts", "src/c.ts", "src/d.ts" + ])).toBe("MEDIUM"); + }); +}); + +// --------------------------------------------------------------------------- +// classifyRisk +// --------------------------------------------------------------------------- +describe("classifyRisk", () => { + it("returns HIGH for schema/migration/deploy keywords", () => { + expect(classifyRisk("run the database migration")).toBe("🔴 HIGH"); + expect(classifyRisk("update the schema")).toBe("🔴 HIGH"); + expect(classifyRisk("deploy to production")).toBe("🔴 HIGH"); + expect(classifyRisk("change env variables")).toBe("🔴 HIGH"); + }); + + it("returns MEDIUM for API/route keywords", () => { + expect(classifyRisk("add a new API endpoint")).toBe("🟡 MEDIUM"); + expect(classifyRisk("update the route handler")).toBe("🟡 MEDIUM"); + }); + + it("returns LOW for general tasks", () => { + expect(classifyRisk("add unit tests for the parser")).toBe("🟢 LOW"); + expect(classifyRisk("rename the variable")).toBe("🟢 LOW"); + }); +}); + +// --------------------------------------------------------------------------- +// splitSubtasks +// --------------------------------------------------------------------------- +describe("splitSubtasks", () => { + it("splits on 'then'", () => { + const parts = splitSubtasks("update the schema then run the migrations"); + expect(parts.length).toBe(2); + expect(parts[0]).toContain("schema"); + expect(parts[1]).toContain("migration"); + }); + + it("splits on 'after that'", () => { + const parts = splitSubtasks("create the model after that add the controller"); + expect(parts.length).toBe(2); + }); + + it("splits on 'and add/update/etc'", () => { + const parts = splitSubtasks("fix the login bug and add error handling for the signup flow"); + expect(parts.length).toBe(2); + }); + + it("returns single-element array for simple prompts", () => { + const parts = splitSubtasks("fix the login bug in auth.ts"); + expect(parts.length).toBe(1); + expect(parts[0]).toContain("fix the login bug"); + }); + + it("handles multiple splits", () => { + const parts = splitSubtasks("create the table then add the API endpoint then deploy"); + expect(parts.length).toBe(3); + }); +});