Compare commits
5 Commits
723b7be84a
...
25e76679c5
| Author | SHA1 | Date | |
|---|---|---|---|
| 25e76679c5 | |||
| d31fca3cb3 | |||
| c29fdd750a | |||
| b86174782f | |||
| 88f6b4df93 |
13
AGENTS.md
13
AGENTS.md
@@ -126,9 +126,20 @@ Key config fields in `execution`:
|
|||||||
loop startup via `selectLoopOptions`; review is asked FIRST, commit is
|
loop startup via `selectLoopOptions`; review is asked FIRST, commit is
|
||||||
mandated when review is on)
|
mandated when review is on)
|
||||||
- `models` — slot-aware round-robin model list for parallel mode, with
|
- `models` — slot-aware round-robin model list for parallel mode, with
|
||||||
automatic failover to the next model per task
|
failover to the next model per task (only after exhausting same-model
|
||||||
|
retries, see `maxSameModelAttempts`)
|
||||||
|
- `maxSameModelAttempts` — max attempts on the SAME model before cycling to
|
||||||
|
the next model on failure (default 5, matching pi's normal retry count).
|
||||||
|
Applies to task execution, commit/review follow-up sessions, and
|
||||||
|
review-fix re-execution alike
|
||||||
- `implModel` / `commitModel` / `reviewModel` — `<provider>/<model>` strings
|
- `implModel` / `commitModel` / `reviewModel` — `<provider>/<model>` strings
|
||||||
resolved via `resolveModelSpec` in `utils.ts`
|
resolved via `resolveModelSpec` in `utils.ts`
|
||||||
|
- `prompts.reviewFocus` — per-review custom focus/instructions, injected as a
|
||||||
|
`## Custom Review Focus` section in review prompts
|
||||||
|
- `review.extraIgnorePatterns` — extra noise-filter exclusion regexes (file
|
||||||
|
paths) merged into the default rules
|
||||||
|
- `review.ignorePaths` — pathspec allowlist keeping matching files in review
|
||||||
|
scope even when a default noise rule would exclude them
|
||||||
- `maxReviewRetries` / `reviewBlockOnFail` — review-gated loop retry behavior
|
- `maxReviewRetries` / `reviewBlockOnFail` — review-gated loop retry behavior
|
||||||
- `worktrees` — `"never" | "parallel" | "always"` git worktree isolation
|
- `worktrees` — `"never" | "parallel" | "always"` git worktree isolation
|
||||||
(default `"parallel"`; see `shouldUseWorktrees` in `src/executor.ts`)
|
(default `"parallel"`; see `shouldUseWorktrees` in `src/executor.ts`)
|
||||||
|
|||||||
23
README.md
23
README.md
@@ -221,8 +221,25 @@ execution:
|
|||||||
prompts:
|
prompts:
|
||||||
projectContext: "Additional context for all tasks"
|
projectContext: "Additional context for all tasks"
|
||||||
reflectionPrompt: "" # custom suffix for reflection extraction
|
reflectionPrompt: "" # custom suffix for reflection extraction
|
||||||
|
reviewFocus: "" # per-review custom focus/instructions (e.g. "check security only")
|
||||||
|
review:
|
||||||
|
extraIgnorePatterns: [] # extra noise-filter exclusion regexes (merged into the default rules)
|
||||||
|
ignorePaths: [] # pathspec allowlist — files matching these stay in review scope
|
||||||
```
|
```
|
||||||
|
|
||||||
|
Review prompts (committed + uncommitted) run the diff through a noise filter
|
||||||
|
before inlining: lockfiles, minified/generated assets, source maps,
|
||||||
|
snapshots, build output, `node_modules`/`vendor`, and binary/media files are
|
||||||
|
excluded by default. The prompt gets a per-file `+/−` summary table, an
|
||||||
|
`### Excluded Files (n)` section listing what was filtered (path, counts,
|
||||||
|
reason), and — when a diff is oversized or touches >20 files — a
|
||||||
|
file-list + "use `read`" instruction instead of a byte-truncated diff.
|
||||||
|
`prompts.reviewFocus` injects a `### Custom Review Focus` section into each
|
||||||
|
review prompt. `review.extraIgnorePatterns` adds exclusion regexes (matched
|
||||||
|
against file paths), and `review.ignorePaths` is a pathspec allowlist that
|
||||||
|
keeps matching files in review scope even when a default rule would exclude
|
||||||
|
them.
|
||||||
|
|
||||||
> `execution.models` uses slot-aware round-robin: with 3 models and 2 concurrent
|
> `execution.models` uses slot-aware round-robin: with 3 models and 2 concurrent
|
||||||
> tasks, only the first two models are used. The third model is only touched when
|
> tasks, only the first two models are used. The third model is only touched when
|
||||||
> a third concurrent task starts. Freed model slots are reused before new ones
|
> a third concurrent task starts. Freed model slots are reused before new ones
|
||||||
@@ -273,3 +290,9 @@ in parallel mode).
|
|||||||
.ralpi/prompts/ # Generated prompts (timestamped, for debugging)
|
.ralpi/prompts/ # Generated prompts (timestamped, for debugging)
|
||||||
.ralpi/config.yaml # Project-level config (optional)
|
.ralpi/config.yaml # Project-level config (optional)
|
||||||
```
|
```
|
||||||
|
|
||||||
|
Every `/ralpi run`, `/ralpi resume`, and `/ralpi reset` (plus the auto-resume
|
||||||
|
on session reload) ensures `.ralpi/` is present in the project's `.gitignore`,
|
||||||
|
so ralpi's own artifacts never show up as untracked/staged files in the user's
|
||||||
|
repo. Opt out per command with `--no-gitignore` (e.g. `/ralpi-run README.md
|
||||||
|
--no-gitignore`).
|
||||||
|
|||||||
47
index.ts
47
index.ts
@@ -34,6 +34,7 @@ import {
|
|||||||
deleteLoopActive,
|
deleteLoopActive,
|
||||||
readLoopActive,
|
readLoopActive,
|
||||||
findRalpiDir,
|
findRalpiDir,
|
||||||
|
ensureRalpiIgnored,
|
||||||
listPRDsSorted,
|
listPRDsSorted,
|
||||||
countPRDResumeStats,
|
countPRDResumeStats,
|
||||||
formatDuration,
|
formatDuration,
|
||||||
@@ -43,6 +44,39 @@ type ExecutionMode = "parallel" | "sequential";
|
|||||||
|
|
||||||
// ─── Helpers ─────────────────────────────────────────────────────────────────
|
// ─── Helpers ─────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Split a `--no-gitignore` opt-out out of the command args (in place). The
|
||||||
|
* flag controls whether `/ralpi run|resume|reset` auto-adds `.ralpi/` to the
|
||||||
|
* project's `.gitignore` — it defaults to on so ralpi's own artifacts never
|
||||||
|
* end up staged in the user's repo.
|
||||||
|
*/
|
||||||
|
function stripNoGitignore(args: string[]): boolean {
|
||||||
|
const i = args.indexOf("--no-gitignore");
|
||||||
|
if (i === -1) return false;
|
||||||
|
args.splice(i, 1);
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Ensure `.ralpi/` is gitignored in the project (unless opted out), and
|
||||||
|
* notify once when the guard actually appended the entry.
|
||||||
|
*/
|
||||||
|
function ensureIgnoredNote(
|
||||||
|
projectDir: string,
|
||||||
|
ctx: ExtensionContext,
|
||||||
|
noGitignore = false,
|
||||||
|
): void {
|
||||||
|
if (noGitignore) return;
|
||||||
|
if (ensureRalpiIgnored(projectDir)) {
|
||||||
|
ctx.ui.notify(
|
||||||
|
"· .ralpi/ added to .gitignore (opt out with --no-gitignore)",
|
||||||
|
"info",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// ─── Helpers ─────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Detect if a token looks like a file path rather than a subcommand.
|
* Detect if a token looks like a file path rather than a subcommand.
|
||||||
* Matches: @path, /path, ./path, ../path, path/to/file, path.md, path.yaml
|
* Matches: @path, /path, ./path, ../path, path/to/file, path.md, path.yaml
|
||||||
@@ -558,6 +592,10 @@ export default function ralpiLoopExtension(pi: ExtensionAPI): void {
|
|||||||
const loopState = readLoopActive(projectDir);
|
const loopState = readLoopActive(projectDir);
|
||||||
if (!loopState) return;
|
if (!loopState) return;
|
||||||
|
|
||||||
|
// The auto-resume path has no CLI flag, so the gitignore guard is
|
||||||
|
// always on: keep `.ralpi/` out of the user's repo on reload too.
|
||||||
|
ensureRalpiIgnored(projectDir);
|
||||||
|
|
||||||
// Load progress state
|
// Load progress state
|
||||||
const progressPath = path.join(projectDir, ".ralpi", "progress.json");
|
const progressPath = path.join(projectDir, ".ralpi", "progress.json");
|
||||||
|
|
||||||
@@ -885,6 +923,7 @@ async function handleRun(
|
|||||||
parentModel?: unknown,
|
parentModel?: unknown,
|
||||||
parentThinkingLevel?: unknown,
|
parentThinkingLevel?: unknown,
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
|
const noGitignore = stripNoGitignore(args);
|
||||||
const taskFile = resolveTaskArg(args[0] || "README.md", ctx.cwd);
|
const taskFile = resolveTaskArg(args[0] || "README.md", ctx.cwd);
|
||||||
|
|
||||||
// If targeting a specific task file and there's existing progress for it,
|
// If targeting a specific task file and there's existing progress for it,
|
||||||
@@ -921,6 +960,7 @@ async function handleRun(
|
|||||||
}
|
}
|
||||||
|
|
||||||
const projectDir = found ? path.dirname(path.dirname(found.path)) : ctx.cwd;
|
const projectDir = found ? path.dirname(path.dirname(found.path)) : ctx.cwd;
|
||||||
|
ensureIgnoredNote(projectDir, ctx, noGitignore);
|
||||||
|
|
||||||
const project = parseTaskFile(taskFile);
|
const project = parseTaskFile(taskFile);
|
||||||
const config = loadConfig(projectDir);
|
const config = loadConfig(projectDir);
|
||||||
@@ -1200,6 +1240,7 @@ async function handleResume(
|
|||||||
parentModel?: unknown,
|
parentModel?: unknown,
|
||||||
parentThinkingLevel?: unknown,
|
parentThinkingLevel?: unknown,
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
|
const noGitignore = stripNoGitignore(args);
|
||||||
let taskFile: string;
|
let taskFile: string;
|
||||||
let projectDir: string;
|
let projectDir: string;
|
||||||
let prdKey: string | undefined;
|
let prdKey: string | undefined;
|
||||||
@@ -1266,6 +1307,8 @@ async function handleResume(
|
|||||||
return undefined;
|
return undefined;
|
||||||
})();
|
})();
|
||||||
|
|
||||||
|
ensureIgnoredNote(projectDir, ctx, noGitignore);
|
||||||
|
|
||||||
await resumeLoop(
|
await resumeLoop(
|
||||||
ctx,
|
ctx,
|
||||||
taskFile,
|
taskFile,
|
||||||
@@ -1287,6 +1330,7 @@ async function handleReset(
|
|||||||
ctx: ExtensionContext,
|
ctx: ExtensionContext,
|
||||||
args: string[],
|
args: string[],
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
|
const noGitignore = stripNoGitignore(args);
|
||||||
let sourcePath: string;
|
let sourcePath: string;
|
||||||
let prdKey: string | undefined;
|
let prdKey: string | undefined;
|
||||||
let progress: ProgressTracker;
|
let progress: ProgressTracker;
|
||||||
@@ -1295,6 +1339,7 @@ async function handleReset(
|
|||||||
const taskFile = resolveTaskArg(args[0], ctx.cwd);
|
const taskFile = resolveTaskArg(args[0], ctx.cwd);
|
||||||
const found = findProgressFile(ctx.cwd, taskFile);
|
const found = findProgressFile(ctx.cwd, taskFile);
|
||||||
const projectDir = found ? path.dirname(path.dirname(found.path)) : ctx.cwd;
|
const projectDir = found ? path.dirname(path.dirname(found.path)) : ctx.cwd;
|
||||||
|
ensureIgnoredNote(projectDir, ctx, noGitignore);
|
||||||
sourcePath = taskFile;
|
sourcePath = taskFile;
|
||||||
prdKey = found?.prdKey;
|
prdKey = found?.prdKey;
|
||||||
progress = new ProgressTracker(projectDir, taskFile, prdKey);
|
progress = new ProgressTracker(projectDir, taskFile, prdKey);
|
||||||
@@ -1309,6 +1354,8 @@ async function handleReset(
|
|||||||
}
|
}
|
||||||
const projectDir = path.dirname(path.dirname(found.path));
|
const projectDir = path.dirname(path.dirname(found.path));
|
||||||
|
|
||||||
|
ensureIgnoredNote(projectDir, ctx, noGitignore);
|
||||||
|
|
||||||
// Multiple loops may have progress — let the user select which one to
|
// Multiple loops may have progress — let the user select which one to
|
||||||
// reset (sorted by most recent first), same as resume.
|
// reset (sorted by most recent first), same as resume.
|
||||||
const selected = await selectPRD(
|
const selected = await selectPRD(
|
||||||
|
|||||||
274
src/diff.ts
Normal file
274
src/diff.ts
Normal file
@@ -0,0 +1,274 @@
|
|||||||
|
/**
|
||||||
|
* Reusable unified-diff engine: parses a diff into per-file +/− stats and
|
||||||
|
* filters out noise files (locks, build output, vendor, generated, media
|
||||||
|
* binaries) so review prompts feed the model only clean, review-relevant
|
||||||
|
* changes.
|
||||||
|
*
|
||||||
|
* Ported from @piex-dev/review's `EXCLUDED_PATTERNS` + `parseDiff` (MIT).
|
||||||
|
* Kept the excluded-files-not-totaled behavior that fixed the upstream
|
||||||
|
* double-count bug.
|
||||||
|
*/
|
||||||
|
|
||||||
|
// ─── Types ──────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
/** Per-file diff stats. */
|
||||||
|
export interface FileDiff {
|
||||||
|
/** File path as it appears in the diff (`a/` path). */
|
||||||
|
path: string;
|
||||||
|
/** Number of added lines (excluding the `+++` header). */
|
||||||
|
linesAdded: number;
|
||||||
|
/** Number of removed lines (excluding the `---` header). */
|
||||||
|
linesRemoved: number;
|
||||||
|
/** File extension (empty when the path has none). */
|
||||||
|
ext: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** An excluded (noise) file with the reason it was filtered. */
|
||||||
|
export interface ExcludedFile extends FileDiff {
|
||||||
|
/** Why the file was excluded (e.g. "lockfile"). */
|
||||||
|
reason: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Result of parsing a unified diff. */
|
||||||
|
export interface DiffSummary {
|
||||||
|
/** Files kept in scope (review-relevant). */
|
||||||
|
files: FileDiff[];
|
||||||
|
/** Files filtered out as noise. */
|
||||||
|
excluded: ExcludedFile[];
|
||||||
|
/** Sum of added lines over included files only. */
|
||||||
|
totalAdded: number;
|
||||||
|
/** Sum of removed lines over included files only. */
|
||||||
|
totalRemoved: number;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Caller-supplied overrides for the noise filter. */
|
||||||
|
export interface DiffOptions {
|
||||||
|
/** Additional exclusion regexes merged into EXCLUDED_PATTERNS. */
|
||||||
|
extraPatterns?: RegExp[];
|
||||||
|
/** Pathspec allowlist — files matching these stay in scope even if a
|
||||||
|
* default rule would exclude them. */
|
||||||
|
ignorePaths?: string[];
|
||||||
|
}
|
||||||
|
|
||||||
|
// ─── Noise-Filter Rules ─────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
/** Default noise-exclusion rules, ported from @piex-dev/review (MIT).
|
||||||
|
* Each entry is a regex tested against the file path plus a human-readable
|
||||||
|
* reason surfaced in the "Excluded Files" prompt section. */
|
||||||
|
export const EXCLUDED_PATTERNS: { pattern: RegExp; reason: string }[] = [
|
||||||
|
// Lockfiles
|
||||||
|
{ pattern: /(^|\/)package-lock\.json$/i, reason: "lockfile" },
|
||||||
|
{ pattern: /(^|\/)yarn\.lock$/i, reason: "lockfile" },
|
||||||
|
{ pattern: /(^|\/)pnpm-lock\.yaml$/i, reason: "lockfile" },
|
||||||
|
{ pattern: /(^|\/)Cargo\.lock$/i, reason: "lockfile" },
|
||||||
|
{ pattern: /(^|\/)Gemfile\.lock$/i, reason: "lockfile" },
|
||||||
|
{ pattern: /\.lock$/i, reason: "lockfile" },
|
||||||
|
// Minified assets
|
||||||
|
{ pattern: /\.min\.(js|css)$/i, reason: "minified asset" },
|
||||||
|
// Generated / tooling output
|
||||||
|
{ pattern: /\.generated\./i, reason: "generated file" },
|
||||||
|
{ pattern: /\.snap$/i, reason: "snapshot" },
|
||||||
|
{ pattern: /\.map$/i, reason: "source map" },
|
||||||
|
// Build output directories
|
||||||
|
{ pattern: /(^|\/)(dist|build|out|coverage)\//i, reason: "build output" },
|
||||||
|
// Dependency trees
|
||||||
|
{ pattern: /(^|\/)node_modules\//i, reason: "dependency" },
|
||||||
|
{ pattern: /(^|\/)vendor\//i, reason: "vendored dependency" },
|
||||||
|
// Image / font / binary extensions
|
||||||
|
{
|
||||||
|
pattern:
|
||||||
|
/\.(png|jpe?g|gif|webp|svg|ico|avif|bmp|woff2?|ttf|otf|eot|pdf|zip|tar|gz|mp[34]|wav|ogg|flac|wasm|bin|exe|dll|so|a|o|class|jar|pyc)$/i,
|
||||||
|
reason: "binary/media asset",
|
||||||
|
},
|
||||||
|
];
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Return the exclusion reason for a file path, or undefined when the file is
|
||||||
|
* review-relevant. Extra caller-supplied patterns are merged into the default
|
||||||
|
* rule set.
|
||||||
|
*/
|
||||||
|
export function isExcluded(
|
||||||
|
fp: string,
|
||||||
|
extraPatterns?: RegExp[],
|
||||||
|
): string | undefined {
|
||||||
|
for (const rule of EXCLUDED_PATTERNS) {
|
||||||
|
if (rule.pattern.test(fp)) return rule.reason;
|
||||||
|
}
|
||||||
|
if (extraPatterns) {
|
||||||
|
for (const p of extraPatterns) {
|
||||||
|
if (p.test(fp)) return "extra ignore pattern";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Safely compile user-supplied regex strings into RegExp objects. Invalid
|
||||||
|
* patterns (that don't compile) are skipped so a bad config value never
|
||||||
|
* crashes review prompt building.
|
||||||
|
*/
|
||||||
|
export function compileIgnorePatterns(patterns: string[]): RegExp[] {
|
||||||
|
const out: RegExp[] = [];
|
||||||
|
for (const p of patterns) {
|
||||||
|
if (!p) continue;
|
||||||
|
try {
|
||||||
|
out.push(new RegExp(p));
|
||||||
|
} catch {
|
||||||
|
// Skip malformed patterns silently
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return out;
|
||||||
|
}
|
||||||
|
|
||||||
|
// ─── Chunking + Counting Helpers ────────────────────────────────────────────
|
||||||
|
|
||||||
|
/** Split a raw diff into per-file chunks, each starting at a `diff --git`
|
||||||
|
* line. The leading non-diff preamble (e.g. a `--stat` block) is dropped —
|
||||||
|
* per-file stats are derived from the patch chunks themselves. */
|
||||||
|
function chunkDiff(raw: string): string[] {
|
||||||
|
if (!raw) return [];
|
||||||
|
const lines = raw.split("\n");
|
||||||
|
const chunks: string[] = [];
|
||||||
|
let current: string[] = [];
|
||||||
|
let started = false;
|
||||||
|
for (const line of lines) {
|
||||||
|
if (line.startsWith("diff --git ")) {
|
||||||
|
if (started && current.length > 0) chunks.push(current.join("\n"));
|
||||||
|
current = [line];
|
||||||
|
started = true;
|
||||||
|
} else if (started) {
|
||||||
|
current.push(line);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (started && current.length > 0) chunks.push(current.join("\n"));
|
||||||
|
return chunks;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Parse the `a/<path>` from a `diff --git a/… b/…` header. Returns null for
|
||||||
|
* malformed chunks that lack the a/… b/… header (guarded, never crashes). */
|
||||||
|
function chunkPath(chunk: string): string | null {
|
||||||
|
const m = chunk.match(/^diff --git a\/(.+?) b\//);
|
||||||
|
return m ? m[1] : null;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Count added/removed lines in a chunk, excluding the `+++`/`---` headers. */
|
||||||
|
function countLines(chunk: string): { added: number; removed: number } {
|
||||||
|
let added = 0;
|
||||||
|
let removed = 0;
|
||||||
|
for (const line of chunk.split("\n")) {
|
||||||
|
if (line.startsWith("+") && !line.startsWith("+++")) added++;
|
||||||
|
else if (line.startsWith("-") && !line.startsWith("---")) removed++;
|
||||||
|
}
|
||||||
|
return { added, removed };
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Extract the file extension from a path (no ext → empty string). */
|
||||||
|
function getExt(fp: string): string {
|
||||||
|
const base = fp.split("/").pop() ?? "";
|
||||||
|
const idx = base.lastIndexOf(".");
|
||||||
|
return idx > 0 ? base.slice(idx + 1) : "";
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Convert a git pathspec glob into a regex (supports `*`, `**`, `?`). */
|
||||||
|
function globToRegExp(glob: string): RegExp {
|
||||||
|
let re = "";
|
||||||
|
for (let i = 0; i < glob.length; i++) {
|
||||||
|
const c = glob[i];
|
||||||
|
if (c === "*") {
|
||||||
|
if (glob[i + 1] === "*") {
|
||||||
|
re += ".*";
|
||||||
|
i++;
|
||||||
|
} else {
|
||||||
|
re += "[^/]*";
|
||||||
|
}
|
||||||
|
} else if (c === "?") {
|
||||||
|
re += "[^/]";
|
||||||
|
} else if (c === ".") {
|
||||||
|
re += "\\.";
|
||||||
|
} else {
|
||||||
|
re += c;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return new RegExp(`^${re}$`);
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Whether a file path matches a pathspec allowlist entry. */
|
||||||
|
function matchesPathspec(pathspec: string, fp: string): boolean {
|
||||||
|
const ps = pathspec.trim();
|
||||||
|
if (!ps) return false;
|
||||||
|
// Directory prefix: "tests/" or a bare dir name matches everything under it.
|
||||||
|
if (ps.endsWith("/") && fp.startsWith(ps)) return true;
|
||||||
|
if (ps.includes("*") || ps.includes("?")) return globToRegExp(ps).test(fp);
|
||||||
|
// Plain path — exact file or prefix directory.
|
||||||
|
if (fp === ps) return true;
|
||||||
|
if (fp.startsWith(ps + "/")) return true;
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Decide whether a file path is kept in scope or noise-excluded. */
|
||||||
|
function classify(
|
||||||
|
path: string,
|
||||||
|
opts?: DiffOptions,
|
||||||
|
): { kept: boolean; reason?: string } {
|
||||||
|
const reason = isExcluded(path, opts?.extraPatterns);
|
||||||
|
if (reason === undefined) return { kept: true };
|
||||||
|
// Excluded by a rule, but an ignorePaths allowlist can keep it in scope.
|
||||||
|
const keptByPathspec =
|
||||||
|
opts?.ignorePaths?.some((ps) => matchesPathspec(ps, path)) ?? false;
|
||||||
|
return keptByPathspec ? { kept: true } : { kept: false, reason };
|
||||||
|
}
|
||||||
|
|
||||||
|
// ─── Public API ─────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Parse a unified diff into per-file +/− stats, splitting excluded (noise)
|
||||||
|
* files from included files. Totals are summed over included files only.
|
||||||
|
* Malformed chunks (no a/… b/… header) are skipped without crashing.
|
||||||
|
*/
|
||||||
|
export function parseDiff(raw: string, opts?: DiffOptions): DiffSummary {
|
||||||
|
const files: FileDiff[] = [];
|
||||||
|
const excluded: ExcludedFile[] = [];
|
||||||
|
let totalAdded = 0;
|
||||||
|
let totalRemoved = 0;
|
||||||
|
|
||||||
|
for (const chunk of chunkDiff(raw)) {
|
||||||
|
if (!chunk) continue;
|
||||||
|
const path = chunkPath(chunk);
|
||||||
|
if (path === null) continue; // malformed chunk — skip
|
||||||
|
const { added, removed } = countLines(chunk);
|
||||||
|
const base: FileDiff = {
|
||||||
|
path,
|
||||||
|
linesAdded: added,
|
||||||
|
linesRemoved: removed,
|
||||||
|
ext: getExt(path),
|
||||||
|
};
|
||||||
|
const decision = classify(path, opts);
|
||||||
|
if (decision.kept) {
|
||||||
|
files.push(base);
|
||||||
|
totalAdded += added;
|
||||||
|
totalRemoved += removed;
|
||||||
|
} else if (decision.reason) {
|
||||||
|
excluded.push({ ...base, reason: decision.reason });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return { files, excluded, totalAdded, totalRemoved };
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Return the diff re-emitted with excluded (noise) file chunks removed, so an
|
||||||
|
* inlined review diff never contains filtered content. The stat preamble is
|
||||||
|
* dropped — the per-file summary table carries that information. Empty string
|
||||||
|
* when every changed file is noise.
|
||||||
|
*/
|
||||||
|
export function filterNoise(raw: string, opts?: DiffOptions): string {
|
||||||
|
const kept: string[] = [];
|
||||||
|
for (const chunk of chunkDiff(raw)) {
|
||||||
|
if (!chunk) continue;
|
||||||
|
const path = chunkPath(chunk);
|
||||||
|
if (path === null) continue;
|
||||||
|
const decision = classify(path, opts);
|
||||||
|
if (decision.kept) kept.push(chunk);
|
||||||
|
}
|
||||||
|
return kept.join("\n");
|
||||||
|
}
|
||||||
178
src/executor.ts
178
src/executor.ts
@@ -19,6 +19,7 @@ import {
|
|||||||
buildConflictResolutionPrompt,
|
buildConflictResolutionPrompt,
|
||||||
MAX_DIFF_BYTES,
|
MAX_DIFF_BYTES,
|
||||||
} from "./prompts";
|
} from "./prompts";
|
||||||
|
import { compileIgnorePatterns } from "./diff";
|
||||||
import { extractReflection } from "./reflection";
|
import { extractReflection } from "./reflection";
|
||||||
import {
|
import {
|
||||||
extractReview,
|
extractReview,
|
||||||
@@ -45,6 +46,7 @@ import {
|
|||||||
ensureDir,
|
ensureDir,
|
||||||
captureGitCommits,
|
captureGitCommits,
|
||||||
captureGitHead,
|
captureGitHead,
|
||||||
|
canComputeRange,
|
||||||
getCommitRangeDiff,
|
getCommitRangeDiff,
|
||||||
hasUncommittedChanges,
|
hasUncommittedChanges,
|
||||||
getGitStatusPorcelain,
|
getGitStatusPorcelain,
|
||||||
@@ -756,10 +758,20 @@ async function executeTask(
|
|||||||
conflicts?: BatchConflict[],
|
conflicts?: BatchConflict[],
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
// Model failover: when a provider/API is down, cycle through available models.
|
// Model failover: when a provider/API is down, cycle through available models.
|
||||||
// Pi's built-in retry (via SettingsManager) handles transient errors with
|
// Pi's built-in retry (via SettingsManager) handles transient HTTP errors
|
||||||
// exponential backoff within each model. Ralpi only handles model cycling.
|
// with exponential backoff WITHIN a single prompt. Ralpi adds two layers on
|
||||||
|
// top: (1) reattempt the SAME model up to `maxSameModelAttempts` times — a
|
||||||
|
// sustained provider hiccup can exhaust pi's in-call retries mid-session,
|
||||||
|
// and flapping to a different model on the first hard failure throws away
|
||||||
|
// model-specific context; (2) once same-model retries are exhausted, cycle
|
||||||
|
// to the next model in the round-robin pool.
|
||||||
const maxModelAttempts = roundRobin ? roundRobin.length : 1;
|
const maxModelAttempts = roundRobin ? roundRobin.length : 1;
|
||||||
|
const maxSameModelAttempts = Math.max(
|
||||||
|
1,
|
||||||
|
config.execution.maxSameModelAttempts,
|
||||||
|
);
|
||||||
let modelAttempt = 0;
|
let modelAttempt = 0;
|
||||||
|
let sameModelAttempt = 0;
|
||||||
// Resolve implModel from config (used in sequential mode when no round-robin assignment).
|
// Resolve implModel from config (used in sequential mode when no round-robin assignment).
|
||||||
// In parallel mode, the round-robin assignedModel takes precedence.
|
// In parallel mode, the round-robin assignedModel takes precedence.
|
||||||
const implModel = resolveModelSpec(
|
const implModel = resolveModelSpec(
|
||||||
@@ -787,12 +799,9 @@ async function executeTask(
|
|||||||
const worktreeDir = wt?.dir ?? projectDir;
|
const worktreeDir = wt?.dir ?? projectDir;
|
||||||
|
|
||||||
while (modelAttempt < maxModelAttempts) {
|
while (modelAttempt < maxModelAttempts) {
|
||||||
// On subsequent model attempts, advance to the next model.
|
// Model advancement happens in the cycling branch below (not here) so a
|
||||||
// Uses advance() instead of assign() so we don't get stuck on
|
// same-model retry `continue` doesn't re-advance and accidentally swap
|
||||||
// the same freed slot when the current model is down.
|
// models mid-retry. The first model uses `currentModel` set above.
|
||||||
if (modelAttempt > 0 && roundRobin) {
|
|
||||||
currentModel = roundRobin.advance(task.id);
|
|
||||||
}
|
|
||||||
|
|
||||||
try {
|
try {
|
||||||
// Mark as in progress
|
// Mark as in progress
|
||||||
@@ -897,19 +906,39 @@ async function executeTask(
|
|||||||
// baseRef was captured before runTask (above). Each review iteration
|
// baseRef was captured before runTask (above). Each review iteration
|
||||||
// diffs the range baseRef..HEAD — the complete task output including
|
// diffs the range baseRef..HEAD — the complete task output including
|
||||||
// all fix attempts. On re-execution the same baseRef is reused.
|
// all fix attempts. On re-execution the same baseRef is reused.
|
||||||
|
// A FAILED range computation (broken/stale base ref, git error) is
|
||||||
|
// logged as a distinct warning and is never treated as a clean,
|
||||||
|
// verified task — only a GENUINE "no changes" skips review.
|
||||||
while (true) {
|
while (true) {
|
||||||
const reviewInfo = baseRef
|
if (!baseRef) {
|
||||||
? getCommitRangeDiff(worktreeDir, baseRef)
|
|
||||||
: null;
|
|
||||||
if (!reviewInfo || !reviewInfo.diff) {
|
|
||||||
const reason = !baseRef
|
|
||||||
? "could not capture base ref before execution"
|
|
||||||
: "no changes found between base and HEAD";
|
|
||||||
sendChatMessage?.(
|
sendChatMessage?.(
|
||||||
`~ review for ${task.id} · ${task.title} — skipping review (${reason})`,
|
`~ review for ${task.id} · ${task.title} — diff could not be computed (could not capture base ref before execution)`,
|
||||||
);
|
);
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
// Cheap guard mirroring canCompareToBase: if the captured base ref no
|
||||||
|
// longer resolves (stale/broken worktree ref), warn explicitly and
|
||||||
|
// never treat the task as review-verified.
|
||||||
|
if (!canComputeRange(worktreeDir, baseRef)) {
|
||||||
|
sendChatMessage?.(
|
||||||
|
`~ review for ${task.id} · ${task.title} — diff could not be computed (base ref ${baseRef} no longer resolves)`,
|
||||||
|
);
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
const rangeDiff = getCommitRangeDiff(worktreeDir, baseRef);
|
||||||
|
if (rangeDiff.kind === "error") {
|
||||||
|
sendChatMessage?.(
|
||||||
|
`~ review for ${task.id} · ${task.title} — diff could not be computed (${rangeDiff.error})`,
|
||||||
|
);
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
if (rangeDiff.kind === "no-changes") {
|
||||||
|
sendChatMessage?.(
|
||||||
|
`~ review for ${task.id} · ${task.title} — skipping review (no changes found between base and HEAD)`,
|
||||||
|
);
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
const reviewInfo = rangeDiff;
|
||||||
|
|
||||||
const reviewPrompt = buildReviewPrompt(
|
const reviewPrompt = buildReviewPrompt(
|
||||||
task,
|
task,
|
||||||
@@ -917,7 +946,16 @@ async function executeTask(
|
|||||||
reviewInfo.hash,
|
reviewInfo.hash,
|
||||||
reviewInfo.subject,
|
reviewInfo.subject,
|
||||||
reviewInfo.diff,
|
reviewInfo.diff,
|
||||||
config.prompts.projectContext,
|
{
|
||||||
|
projectContext: config.prompts.projectContext,
|
||||||
|
focus: config.prompts.reviewFocus,
|
||||||
|
diffOptions: {
|
||||||
|
extraPatterns: compileIgnorePatterns(
|
||||||
|
config.review.extraIgnorePatterns,
|
||||||
|
),
|
||||||
|
ignorePaths: config.review.ignorePaths,
|
||||||
|
},
|
||||||
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
const reviewModel = resolveFollowUpModel(
|
const reviewModel = resolveFollowUpModel(
|
||||||
@@ -1049,19 +1087,33 @@ async function executeTask(
|
|||||||
fixAttempt++
|
fixAttempt++
|
||||||
) {
|
) {
|
||||||
const fixModel = fixModels[fixAttempt];
|
const fixModel = fixModels[fixAttempt];
|
||||||
fixResult = await runTask(
|
let fixSameAttempt = 0;
|
||||||
task,
|
// Reattempt on the same model before cycling, matching the main
|
||||||
project,
|
// task loop's behavior.
|
||||||
config,
|
for (;;) {
|
||||||
depReflections,
|
fixResult = await runTask(
|
||||||
ctx,
|
task,
|
||||||
sendChatMessage,
|
project,
|
||||||
worktreeDir,
|
config,
|
||||||
parallelState,
|
depReflections,
|
||||||
fixModel,
|
ctx,
|
||||||
batchRender,
|
sendChatMessage,
|
||||||
review ?? undefined,
|
worktreeDir,
|
||||||
);
|
parallelState,
|
||||||
|
fixModel,
|
||||||
|
batchRender,
|
||||||
|
review ?? undefined,
|
||||||
|
);
|
||||||
|
if (fixResult.success) break;
|
||||||
|
if (fixSameAttempt < maxSameModelAttempts - 1) {
|
||||||
|
fixSameAttempt++;
|
||||||
|
sendChatMessage?.(
|
||||||
|
`~ re-execution for ${task.id} · ${task.title} — reattempting model ${fixAttempt + 1}/${fixModels.length} (${fixSameAttempt + 1}/${maxSameModelAttempts}, previous: ${fixResult.error})`,
|
||||||
|
);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
break; // same-model retries exhausted
|
||||||
|
}
|
||||||
if (fixResult.success) break;
|
if (fixResult.success) break;
|
||||||
// Connection/error failover — try the next model.
|
// Connection/error failover — try the next model.
|
||||||
if (fixAttempt < fixModels.length - 1) {
|
if (fixAttempt < fixModels.length - 1) {
|
||||||
@@ -1226,9 +1278,22 @@ async function executeTask(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Agent session failed (provider error).
|
// Agent session failed (provider error).
|
||||||
// Pi's built-in retry already exhausted for this model. Cycle to the next.
|
// Pi's built-in in-call retry already exhausted for this attempt.
|
||||||
|
// Reattempt on the SAME model a few more times before cycling — a
|
||||||
|
// transient outage can outlast pi's per-prompt backoff window.
|
||||||
|
sameModelAttempt++;
|
||||||
|
if (sameModelAttempt < maxSameModelAttempts) {
|
||||||
|
sendChatMessage?.(
|
||||||
|
`~ ${task.id} · ${task.title} — reattempting model ${modelAttempt + 1}/${maxModelAttempts} (${sameModelAttempt + 1}/${maxSameModelAttempts}, previous: ${result.error})`,
|
||||||
|
);
|
||||||
|
continue; // same model, fresh session
|
||||||
|
}
|
||||||
|
|
||||||
|
// Same-model retries exhausted — cycle to the next model (if any).
|
||||||
if (roundRobin && modelAttempt < maxModelAttempts - 1) {
|
if (roundRobin && modelAttempt < maxModelAttempts - 1) {
|
||||||
modelAttempt++;
|
modelAttempt++;
|
||||||
|
sameModelAttempt = 0;
|
||||||
|
currentModel = roundRobin.advance(task.id);
|
||||||
sendChatMessage?.(
|
sendChatMessage?.(
|
||||||
`~ ${task.id} · ${task.title} — cycling to model ${modelAttempt + 1}/${maxModelAttempts} (previous: ${result.error})`,
|
`~ ${task.id} · ${task.title} — cycling to model ${modelAttempt + 1}/${maxModelAttempts} (previous: ${result.error})`,
|
||||||
);
|
);
|
||||||
@@ -1417,28 +1482,43 @@ async function runFollowUpSession(
|
|||||||
}, 100);
|
}, 100);
|
||||||
|
|
||||||
let result: Awaited<ReturnType<typeof runAgentSession>> | undefined;
|
let result: Awaited<ReturnType<typeof runAgentSession>> | undefined;
|
||||||
|
const maxSameModelAttempts = Math.max(
|
||||||
|
1,
|
||||||
|
config.execution.maxSameModelAttempts,
|
||||||
|
);
|
||||||
try {
|
try {
|
||||||
for (let attempt = 0; attempt < models.length; attempt++) {
|
for (let attempt = 0; attempt < models.length; attempt++) {
|
||||||
const model = models[attempt];
|
const model = models[attempt];
|
||||||
result = await runAgentSession(
|
// Reattempt on the same model before cycling — matches the main task
|
||||||
prompt,
|
// loop. Clear partial tool calls between failed attempts so the
|
||||||
projectDir,
|
// widget reflects only the successful (or final) attempt.
|
||||||
timeoutMs,
|
for (let same = 0; same < maxSameModelAttempts; same++) {
|
||||||
(event) => {
|
result = await runAgentSession(
|
||||||
if (event.type === "tool_execution_start") {
|
prompt,
|
||||||
const label = formatToolArg(event.toolName, event.args);
|
projectDir,
|
||||||
toolCalls.push({ name: event.toolName, label });
|
timeoutMs,
|
||||||
requestRender();
|
(event) => {
|
||||||
}
|
if (event.type === "tool_execution_start") {
|
||||||
},
|
const label = formatToolArg(event.toolName, event.args);
|
||||||
undefined,
|
toolCalls.push({ name: event.toolName, label });
|
||||||
model,
|
requestRender();
|
||||||
config.thinkingLevel,
|
}
|
||||||
false, // noSkills=false — follow-up sessions load skills too
|
},
|
||||||
(ctx.modelRegistry as any).runtime as ModelRuntime,
|
undefined,
|
||||||
);
|
model,
|
||||||
|
config.thinkingLevel,
|
||||||
|
false, // noSkills=false — follow-up sessions load skills too
|
||||||
|
(ctx.modelRegistry as any).runtime as ModelRuntime,
|
||||||
|
);
|
||||||
|
|
||||||
if (result.success) break;
|
if (result.success) break;
|
||||||
|
if (same < maxSameModelAttempts - 1) {
|
||||||
|
toolCalls.length = 0;
|
||||||
|
requestRender();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (result!.success) break;
|
||||||
|
|
||||||
// If there's a next model to try, cycle; otherwise give up.
|
// If there's a next model to try, cycle; otherwise give up.
|
||||||
if (attempt < models.length - 1) {
|
if (attempt < models.length - 1) {
|
||||||
|
|||||||
182
src/prompts.ts
182
src/prompts.ts
@@ -1,27 +1,34 @@
|
|||||||
import type { Task, Project, Reflection, ReviewResult } from "./types";
|
import type { Task, Project, Reflection, ReviewResult } from "./types";
|
||||||
import { readTaskSpec } from "./parser";
|
import { readTaskSpec } from "./parser";
|
||||||
|
import {
|
||||||
|
parseDiff,
|
||||||
|
filterNoise,
|
||||||
|
type DiffSummary,
|
||||||
|
type DiffOptions,
|
||||||
|
} from "./diff";
|
||||||
|
|
||||||
/** Maximum bytes of a commit diff embedded in a review/commit prompt.
|
/** Maximum bytes of an inlined review diff before we stop inlining it and
|
||||||
* Diffs larger than this are truncated to avoid blowing past the model's
|
* instead list the changed files + tell the model to `read` them.
|
||||||
* context window. The agent can always run `git show HEAD` itself to
|
* Diffs larger than this are never byte-truncated into a review prompt —
|
||||||
* inspect the full diff when it needs more detail.
|
* truncation loses the middle of a large diff, so the file-list + read
|
||||||
|
* instruction is strictly better.
|
||||||
*
|
*
|
||||||
* ~50 KB ≈ 12.5K tokens — comfortably fits even on models with a 128K
|
* ~50 KB ≈ 12.5K tokens — comfortably fits even on models with a 128K
|
||||||
* context window once system-prompt overhead is accounted for. */
|
* context window once system-prompt overhead is accounted for. */
|
||||||
export const MAX_DIFF_BYTES = 50_000;
|
export const MAX_DIFF_BYTES = 50_000;
|
||||||
|
|
||||||
/**
|
/** Max included files before an oversized diff is replaced by a read
|
||||||
* Truncate a diff to MAX_DIFF_BYTES, appending a clear notice when truncated.
|
* instruction rather than inlined. */
|
||||||
*/
|
const MAX_REVIEW_FILES = 20;
|
||||||
function truncateDiff(diff: string): string {
|
|
||||||
if (diff.length <= MAX_DIFF_BYTES) return diff;
|
/** Optional knobs for the review prompt builders. */
|
||||||
const omitted = diff.length - MAX_DIFF_BYTES;
|
export interface ReviewPromptOptions {
|
||||||
return (
|
/** Extra context injected into the prompt (config.prompts.projectContext). */
|
||||||
diff.slice(0, MAX_DIFF_BYTES) +
|
projectContext?: string;
|
||||||
"\n\n... (diff truncated: omitted " +
|
/** Per-review custom focus/instructions (config.prompts.reviewFocus). */
|
||||||
omitted.toLocaleString() +
|
focus?: string;
|
||||||
" bytes; run `git show HEAD` to view the full diff)"
|
/** Noise-filter overrides (config.review.*). */
|
||||||
);
|
diffOptions?: DiffOptions;
|
||||||
}
|
}
|
||||||
|
|
||||||
// ─── Task Prompt ─────────────────────────────────────────────────────────────
|
// ─── Task Prompt ─────────────────────────────────────────────────────────────
|
||||||
@@ -201,7 +208,7 @@ export function buildReviewPrompt(
|
|||||||
commitHash: string,
|
commitHash: string,
|
||||||
commitSubject: string,
|
commitSubject: string,
|
||||||
commitDiff: string,
|
commitDiff: string,
|
||||||
projectContext?: string,
|
opts: ReviewPromptOptions = {},
|
||||||
): string {
|
): string {
|
||||||
const parts: string[] = [];
|
const parts: string[] = [];
|
||||||
|
|
||||||
@@ -236,17 +243,34 @@ export function buildReviewPrompt(
|
|||||||
parts.push("## Commit Under Review");
|
parts.push("## Commit Under Review");
|
||||||
parts.push(`Commit: ${commitHash} — ${commitSubject}`);
|
parts.push(`Commit: ${commitHash} — ${commitSubject}`);
|
||||||
parts.push("");
|
parts.push("");
|
||||||
parts.push("### Diff");
|
|
||||||
parts.push("```diff");
|
// ── Changed-Files Summary + Exclusions (noise-filtered scope) ──
|
||||||
parts.push(truncateDiff(commitDiff));
|
|
||||||
parts.push("```");
|
const summary = parseDiff(commitDiff, opts.diffOptions);
|
||||||
|
const filtered = filterNoise(commitDiff, opts.diffOptions);
|
||||||
|
parts.push(buildFileSummaryTable(summary));
|
||||||
|
const excluded = renderExcludedFiles(summary);
|
||||||
|
if (excluded) parts.push(excluded);
|
||||||
parts.push("");
|
parts.push("");
|
||||||
|
|
||||||
|
// ── Diff (inline, or file-list + read instruction when oversized) ──
|
||||||
|
|
||||||
|
parts.push(renderDiffSection(summary, filtered, "### Diff"));
|
||||||
|
parts.push("");
|
||||||
|
|
||||||
|
// ── Custom Review Focus ──
|
||||||
|
|
||||||
|
if (opts.focus) {
|
||||||
|
parts.push("## Custom Review Focus");
|
||||||
|
parts.push(opts.focus);
|
||||||
|
parts.push("");
|
||||||
|
}
|
||||||
|
|
||||||
// ── Project Context ──
|
// ── Project Context ──
|
||||||
|
|
||||||
if (projectContext) {
|
if (opts.projectContext) {
|
||||||
parts.push("## Additional Context");
|
parts.push("## Additional Context");
|
||||||
parts.push(projectContext);
|
parts.push(opts.projectContext);
|
||||||
parts.push("");
|
parts.push("");
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -254,7 +278,7 @@ export function buildReviewPrompt(
|
|||||||
|
|
||||||
parts.push("## Review Instructions");
|
parts.push("## Review Instructions");
|
||||||
parts.push(
|
parts.push(
|
||||||
"Review the commit above against the task description. Check for:",
|
"Review the changes above against the task description. Check for:",
|
||||||
);
|
);
|
||||||
parts.push(...reviewInstructions());
|
parts.push(...reviewInstructions());
|
||||||
parts.push("");
|
parts.push("");
|
||||||
@@ -279,7 +303,7 @@ export function buildReviewPromptUncommitted(
|
|||||||
project: Project,
|
project: Project,
|
||||||
status: string,
|
status: string,
|
||||||
diff: string,
|
diff: string,
|
||||||
projectContext?: string,
|
opts: ReviewPromptOptions = {},
|
||||||
): string {
|
): string {
|
||||||
const parts: string[] = [];
|
const parts: string[] = [];
|
||||||
|
|
||||||
@@ -321,17 +345,36 @@ export function buildReviewPromptUncommitted(
|
|||||||
parts.push(status || "(no status output)");
|
parts.push(status || "(no status output)");
|
||||||
parts.push("```");
|
parts.push("```");
|
||||||
parts.push("");
|
parts.push("");
|
||||||
parts.push("### Current Tracked Diff (git diff)");
|
|
||||||
parts.push("```diff");
|
// ── Changed-Files Summary + Exclusions (noise-filtered scope) ──
|
||||||
parts.push(truncateDiff(diff) || "(no tracked diff output)");
|
|
||||||
parts.push("```");
|
const summary = parseDiff(diff, opts.diffOptions);
|
||||||
|
const filtered = filterNoise(diff, opts.diffOptions);
|
||||||
|
parts.push(buildFileSummaryTable(summary));
|
||||||
|
const excluded = renderExcludedFiles(summary);
|
||||||
|
if (excluded) parts.push(excluded);
|
||||||
parts.push("");
|
parts.push("");
|
||||||
|
|
||||||
|
// ── Diff (inline, or file-list + read instruction when oversized) ──
|
||||||
|
|
||||||
|
parts.push(
|
||||||
|
renderDiffSection(summary, filtered, "### Current Tracked Diff (git diff)"),
|
||||||
|
);
|
||||||
|
parts.push("");
|
||||||
|
|
||||||
|
// ── Custom Review Focus ──
|
||||||
|
|
||||||
|
if (opts.focus) {
|
||||||
|
parts.push("## Custom Review Focus");
|
||||||
|
parts.push(opts.focus);
|
||||||
|
parts.push("");
|
||||||
|
}
|
||||||
|
|
||||||
// ── Project Context ──
|
// ── Project Context ──
|
||||||
|
|
||||||
if (projectContext) {
|
if (opts.projectContext) {
|
||||||
parts.push("## Additional Context");
|
parts.push("## Additional Context");
|
||||||
parts.push(projectContext);
|
parts.push(opts.projectContext);
|
||||||
parts.push("");
|
parts.push("");
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -354,6 +397,78 @@ export function buildReviewPromptUncommitted(
|
|||||||
|
|
||||||
// ─── Shared Review Prompt Helpers ───────────────────────────────────────────
|
// ─── Shared Review Prompt Helpers ───────────────────────────────────────────
|
||||||
|
|
||||||
|
/** Whether an oversized/wide diff should be replaced by a file-list + read
|
||||||
|
* instruction instead of being inlined. Thresholds: cleaned diff over
|
||||||
|
* MAX_DIFF_BYTES, or more than MAX_REVIEW_FILES included files. */
|
||||||
|
function shouldSkipInline(summary: DiffSummary, filteredLength: number): boolean {
|
||||||
|
return (
|
||||||
|
filteredLength > MAX_DIFF_BYTES || summary.files.length > MAX_REVIEW_FILES
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Render a per-file +/− summary Markdown table (with type column and a total
|
||||||
|
* line) from a parsed diff. Handles the empty/all-noise diff gracefully — an
|
||||||
|
* empty table with zero totals, no crash.
|
||||||
|
*/
|
||||||
|
function buildFileSummaryTable(summary: DiffSummary): string {
|
||||||
|
const lines: string[] = [];
|
||||||
|
lines.push("### Changed Files");
|
||||||
|
lines.push("");
|
||||||
|
lines.push("| File | +/− | Type |");
|
||||||
|
lines.push("|------|-----|------|");
|
||||||
|
if (summary.files.length === 0) {
|
||||||
|
lines.push("| _(no included changes)_ | — | — |");
|
||||||
|
} else {
|
||||||
|
for (const f of summary.files) {
|
||||||
|
lines.push(
|
||||||
|
`| \`${f.path}\` | +${f.linesAdded}/-${f.linesRemoved} | ${f.ext || "—"} |`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
lines.push(`| **Total** | **+${summary.totalAdded}/-${summary.totalRemoved}** | |`);
|
||||||
|
return lines.join("\n");
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Render the `### Excluded Files (n)` bullet list (path, +/− counts, reason).
|
||||||
|
* Returns an empty string when there are no exclusions so callers omit the
|
||||||
|
* section entirely (no empty heading).
|
||||||
|
*/
|
||||||
|
function renderExcludedFiles(summary: DiffSummary): string {
|
||||||
|
if (summary.excluded.length === 0) return "";
|
||||||
|
const lines: string[] = [];
|
||||||
|
lines.push(`### Excluded Files (${summary.excluded.length})`);
|
||||||
|
lines.push("");
|
||||||
|
for (const f of summary.excluded) {
|
||||||
|
lines.push(
|
||||||
|
`- \`${f.path}\` (+${f.linesAdded}/-${f.linesRemoved}) — ${f.reason}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return lines.join("\n");
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Render the diff section of a review prompt. Under the threshold, inline the
|
||||||
|
* noise-filtered diff. Over the threshold (size or file count), emit a
|
||||||
|
* file-list + read-instruction notice and never byte-truncate the diff.
|
||||||
|
*/
|
||||||
|
function renderDiffSection(
|
||||||
|
summary: DiffSummary,
|
||||||
|
filtered: string,
|
||||||
|
heading: string,
|
||||||
|
): string {
|
||||||
|
if (shouldSkipInline(summary, filtered.length)) {
|
||||||
|
return `${heading} — _Diff too large (${filtered.length.toLocaleString()} chars, ${summary.files.length} files). Use \`read\` to inspect the changed files._`;
|
||||||
|
}
|
||||||
|
const lines: string[] = [];
|
||||||
|
lines.push(heading);
|
||||||
|
lines.push("```diff");
|
||||||
|
lines.push(filtered || "(no included changes)");
|
||||||
|
lines.push("```");
|
||||||
|
return lines.join("\n");
|
||||||
|
}
|
||||||
|
|
||||||
function reviewInstructions(): string[] {
|
function reviewInstructions(): string[] {
|
||||||
return [
|
return [
|
||||||
"- **Correctness**: Does the implementation fulfill the task requirements?",
|
"- **Correctness**: Does the implementation fulfill the task requirements?",
|
||||||
@@ -373,7 +488,7 @@ function reviewVerdictBlock(): string[] {
|
|||||||
"VERDICT: [pass | warn | fail]",
|
"VERDICT: [pass | warn | fail]",
|
||||||
"SUMMARY: [1-2 sentence overall assessment]",
|
"SUMMARY: [1-2 sentence overall assessment]",
|
||||||
"FINDINGS:",
|
"FINDINGS:",
|
||||||
"- [blocker] file:line description (use severity: blocker|warning|nit|info)",
|
"- [blocker] file:line description (use severity: blocker|warning|nit|info; `critical` is accepted as a blocker synonym)",
|
||||||
"- [warning] file:line description",
|
"- [warning] file:line description",
|
||||||
"```",
|
"```",
|
||||||
"",
|
"",
|
||||||
@@ -387,7 +502,8 @@ function reviewVerdictBlock(): string[] {
|
|||||||
"",
|
"",
|
||||||
"Each FINDINGS line uses the form `- [severity] [file:line] message`.",
|
"Each FINDINGS line uses the form `- [severity] [file:line] message`.",
|
||||||
"The `file:line` part is optional. Severity must be one of:",
|
"The `file:line` part is optional. Severity must be one of:",
|
||||||
"`blocker`, `warning`, `nit`, `info`.",
|
"`blocker`, `warning`, `nit`, `info`. The `critical` token is accepted",
|
||||||
|
"and treated as `blocker`.",
|
||||||
];
|
];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -88,13 +88,16 @@ function extractFindings(block: string): ReviewFinding[] {
|
|||||||
.filter(Boolean);
|
.filter(Boolean);
|
||||||
|
|
||||||
const findings: ReviewFinding[] = [];
|
const findings: ReviewFinding[] = [];
|
||||||
const severityRe = /^\[(blocker|warning|warn|nit|info)\]\s*(.*)$/i;
|
// `critical` is accepted and normalized to ralpi's `blocker` severity,
|
||||||
|
// providing parity with @piex-dev/review's critical/warning/info grading.
|
||||||
|
const severityRe = /^\[(blocker|critical|warning|warn|nit|info)\]\s*(.*)$/i;
|
||||||
|
|
||||||
for (const line of lines) {
|
for (const line of lines) {
|
||||||
const sm = line.match(severityRe);
|
const sm = line.match(severityRe);
|
||||||
if (sm) {
|
if (sm) {
|
||||||
let sev = sm[1].toLowerCase();
|
let sev = sm[1].toLowerCase();
|
||||||
if (sev === "warn") sev = "warning";
|
if (sev === "warn") sev = "warning";
|
||||||
|
else if (sev === "critical") sev = "blocker";
|
||||||
const rest = sm[2].trim();
|
const rest = sm[2].trim();
|
||||||
const { file, line: lineNum, message } = parseFileRef(rest);
|
const { file, line: lineNum, message } = parseFileRef(rest);
|
||||||
findings.push({
|
findings.push({
|
||||||
|
|||||||
28
src/types.ts
28
src/types.ts
@@ -246,6 +246,16 @@ export interface RalpiConfig {
|
|||||||
reviewBlockOnFail: boolean;
|
reviewBlockOnFail: boolean;
|
||||||
/** Maximum total duration for the entire loop execution in milliseconds (0 = no limit). Checked between batches — in-progress tasks finish naturally. */
|
/** Maximum total duration for the entire loop execution in milliseconds (0 = no limit). Checked between batches — in-progress tasks finish naturally. */
|
||||||
loopTimeoutMs: number;
|
loopTimeoutMs: number;
|
||||||
|
/** Max attempts on the SAME model before cycling to the next model on
|
||||||
|
* failure. Pi retries transient HTTP errors within a single prompt,
|
||||||
|
* but a sustained provider hiccup can still exhaust those in-call
|
||||||
|
* retries mid-session. Re-running the whole session a few times on
|
||||||
|
* the same model avoids flapping to a different model (and losing
|
||||||
|
* model-specific context) on the first hard failure. Applies to task
|
||||||
|
* execution, commit/review follow-up sessions, and review-fix
|
||||||
|
* re-execution alike. After this many attempts on one model, ralpi
|
||||||
|
* advances to the next model in the round-robin pool. */
|
||||||
|
maxSameModelAttempts: number;
|
||||||
/** Isolate each task in a separate git worktree so parallel tasks can't
|
/** Isolate each task in a separate git worktree so parallel tasks can't
|
||||||
* stomp each other's files, and review/commit see a clean single-task diff.
|
* stomp each other's files, and review/commit see a clean single-task diff.
|
||||||
* - "never": all tasks run in the shared working tree (default, backward compat)
|
* - "never": all tasks run in the shared working tree (default, backward compat)
|
||||||
@@ -258,6 +268,18 @@ export interface RalpiConfig {
|
|||||||
projectContext: string;
|
projectContext: string;
|
||||||
/** Custom prompt suffix for reflection extraction */
|
/** Custom prompt suffix for reflection extraction */
|
||||||
reflectionPrompt: string;
|
reflectionPrompt: string;
|
||||||
|
/** Per-review custom focus/instructions (e.g. "check security only").
|
||||||
|
* Injected as a `### Custom Review Focus` section in committed and
|
||||||
|
* uncommitted review prompts when non-empty. */
|
||||||
|
reviewFocus: string;
|
||||||
|
};
|
||||||
|
review: {
|
||||||
|
/** Extra noise-filter exclusion regexes (strings compiled to RegExp),
|
||||||
|
* merged into EXCLUDED_PATTERNS for review diffs. */
|
||||||
|
extraIgnorePatterns: string[];
|
||||||
|
/** Pathspec allowlist — files matching these stay in scope even when a
|
||||||
|
* default noise rule would exclude them. */
|
||||||
|
ignorePaths: string[];
|
||||||
};
|
};
|
||||||
/** Parent session model to inherit in child agent sessions */
|
/** Parent session model to inherit in child agent sessions */
|
||||||
model?: unknown;
|
model?: unknown;
|
||||||
@@ -287,9 +309,15 @@ export const DEFAULT_CONFIG: RalpiConfig = {
|
|||||||
reviewBlockOnFail: false, // false = commit anyway after retries exhausted
|
reviewBlockOnFail: false, // false = commit anyway after retries exhausted
|
||||||
loopTimeoutMs: 0, // 0 = no limit
|
loopTimeoutMs: 0, // 0 = no limit
|
||||||
worktrees: "parallel", // worktree isolation for parallel tasks by default
|
worktrees: "parallel", // worktree isolation for parallel tasks by default
|
||||||
|
maxSameModelAttempts: 5, // retry the same model up to 5 times before cycling to the next
|
||||||
},
|
},
|
||||||
prompts: {
|
prompts: {
|
||||||
projectContext: "",
|
projectContext: "",
|
||||||
reflectionPrompt: "",
|
reflectionPrompt: "",
|
||||||
|
reviewFocus: "",
|
||||||
|
},
|
||||||
|
review: {
|
||||||
|
extraIgnorePatterns: [],
|
||||||
|
ignorePaths: [],
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|||||||
113
src/utils.ts
113
src/utils.ts
@@ -100,6 +100,49 @@ export function deleteLoopActive(projectDir: string): void {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ─── Git Hygiene ────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
const ralpiIgnoreMemo = new Set<string>();
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Ensure `.ralpi/` is excluded from the project's `.gitignore` so ralpi's own
|
||||||
|
* run-state, worktrees, and reviews never show up as tracked/untracked files
|
||||||
|
* in the user's repo.
|
||||||
|
*
|
||||||
|
* Memoized per project dir; only acts inside a git work tree (`.git` may be a
|
||||||
|
* directory or, in linked worktrees, a file). Creates or appends `.ralpi/` to
|
||||||
|
* `.gitignore`, best-effort: any failure returns `false` (never throws).
|
||||||
|
*
|
||||||
|
* @returns true when the ignore entry was newly added, false otherwise.
|
||||||
|
*/
|
||||||
|
export function ensureRalpiIgnored(projectDir: string): boolean {
|
||||||
|
if (ralpiIgnoreMemo.has(projectDir)) return false;
|
||||||
|
ralpiIgnoreMemo.add(projectDir);
|
||||||
|
try {
|
||||||
|
// Only act inside a git work tree (works for worktrees too: .git is a file).
|
||||||
|
fs.statSync(path.join(projectDir, ".git"));
|
||||||
|
const ignorePath = path.join(projectDir, ".gitignore");
|
||||||
|
const marker = ".ralpi/";
|
||||||
|
let content: string;
|
||||||
|
try {
|
||||||
|
content = fs.readFileSync(ignorePath, "utf8");
|
||||||
|
} catch {
|
||||||
|
fs.writeFileSync(ignorePath, `${marker}\n`, "utf8");
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
if (content.split(/\r?\n/).some((l) => l.trim() === marker)) return false;
|
||||||
|
const prefix = content.endsWith("\n") ? "" : "\n";
|
||||||
|
fs.appendFileSync(
|
||||||
|
ignorePath,
|
||||||
|
`${prefix}# ralpi run-state, worktrees, and reviews\n${marker}\n`,
|
||||||
|
"utf8",
|
||||||
|
);
|
||||||
|
return true;
|
||||||
|
} catch {
|
||||||
|
return false; // not a git work tree, or a best-effort write failed
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Discover the project directory by walking up to find `.ralpi/`.
|
* Discover the project directory by walking up to find `.ralpi/`.
|
||||||
*/
|
*/
|
||||||
@@ -910,17 +953,49 @@ export function captureGitHead(projectDir: string): string | undefined {
|
|||||||
* made since the base reference. Used by the review-gated loop so the reviewer
|
* made since the base reference. Used by the review-gated loop so the reviewer
|
||||||
* sees the full task diff (all commits, not just the latest) across execution
|
* sees the full task diff (all commits, not just the latest) across execution
|
||||||
* attempts and re-execution fixes. `baseRef` must be a validated hex SHA from
|
* attempts and re-execution fixes. `baseRef` must be a validated hex SHA from
|
||||||
* captureGitHead(). Returns the short HEAD hash, HEAD subject, and range diff,
|
* captureGitHead().
|
||||||
* or null when git is unavailable / baseRef is invalid / no changes exist.
|
*
|
||||||
|
* Returns a tri-state so the review loop can tell a FAILED range computation
|
||||||
|
* (invalid/stale base ref, git error) apart from a GENUINELY EMPTY range — a
|
||||||
|
* broken base must never be silently treated as a clean, verified task.
|
||||||
*/
|
*/
|
||||||
|
export type CommitRangeDiffResult =
|
||||||
|
| { kind: "ok"; hash: string; subject: string; diff: string }
|
||||||
|
| { kind: "no-changes" }
|
||||||
|
| { kind: "error"; error: string };
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether the `baseRef..HEAD` range can be computed — i.e. the base ref is a
|
||||||
|
* resolvable commit in this repo (mirrors @piex-dev/review's canCompareToBase).
|
||||||
|
* Only validated hex SHAs are passed to the shell.
|
||||||
|
*/
|
||||||
|
export function canComputeRange(projectDir: string, baseRef: string): boolean {
|
||||||
|
const { execSync } = require("node:child_process");
|
||||||
|
if (!/^[0-9a-f]{7,40}$/i.test(baseRef)) return false;
|
||||||
|
try {
|
||||||
|
// git cat-file -e truly verifies the object EXISTS (rev-parse --verify
|
||||||
|
// accepts any 40-hex SHA even if it was never created), so a stale/broken
|
||||||
|
// base ref is caught here rather than silently treated as no-changes.
|
||||||
|
execSync(`git cat-file -e ${baseRef}`, {
|
||||||
|
cwd: projectDir,
|
||||||
|
stdio: "pipe",
|
||||||
|
});
|
||||||
|
return true;
|
||||||
|
} catch {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export function getCommitRangeDiff(
|
export function getCommitRangeDiff(
|
||||||
projectDir: string,
|
projectDir: string,
|
||||||
baseRef: string,
|
baseRef: string,
|
||||||
): { hash: string; subject: string; diff: string } | null {
|
): CommitRangeDiffResult {
|
||||||
const { execSync } = require("node:child_process");
|
const { execSync } = require("node:child_process");
|
||||||
|
|
||||||
// Only pass validated hex SHAs to the shell.
|
// Only pass validated hex SHAs to the shell.
|
||||||
if (!/^[0-9a-f]{7,40}$/i.test(baseRef)) return null;
|
if (!/^[0-9a-f]{7,40}$/i.test(baseRef)) {
|
||||||
|
return { kind: "error", error: "invalid or stale base ref" };
|
||||||
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
execSync("git rev-parse --git-dir", {
|
execSync("git rev-parse --git-dir", {
|
||||||
@@ -928,7 +1003,20 @@ export function getCommitRangeDiff(
|
|||||||
stdio: "pipe",
|
stdio: "pipe",
|
||||||
});
|
});
|
||||||
} catch {
|
} catch {
|
||||||
return null;
|
return { kind: "error", error: "not a git repository" };
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify the base ref resolves before diffing — a stale/unfetched ref is a
|
||||||
|
// computation failure, not a clean "no changes" signal. git cat-file -e
|
||||||
|
// checks the object genuinely exists (rev-parse --verify would accept any
|
||||||
|
// 40-hex SHA even if it was never created).
|
||||||
|
try {
|
||||||
|
execSync(`git cat-file -e ${baseRef}`, {
|
||||||
|
cwd: projectDir,
|
||||||
|
stdio: "pipe",
|
||||||
|
});
|
||||||
|
} catch {
|
||||||
|
return { kind: "error", error: `base ref ${baseRef} cannot be resolved` };
|
||||||
}
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
@@ -946,17 +1034,20 @@ export function getCommitRangeDiff(
|
|||||||
// the snapshot. Includes stat overview + full patch.
|
// the snapshot. Includes stat overview + full patch.
|
||||||
//
|
//
|
||||||
// maxBuffer is set high (10 MB) so larger tasks don't cause execSync to
|
// maxBuffer is set high (10 MB) so larger tasks don't cause execSync to
|
||||||
// throw. The review prompt builder truncates to MAX_DIFF_BYTES (50 KB)
|
// throw. The review prompt builder filters noise and inlines only under
|
||||||
// before sending to the model, so the full diff in memory is fine.
|
// MAX_DIFF_BYTES, so the full diff in memory is fine.
|
||||||
const diff = execSync(`git diff ${baseRef} HEAD --stat --patch`, {
|
const diff = execSync(`git diff ${baseRef} HEAD --stat --patch`, {
|
||||||
cwd: projectDir,
|
cwd: projectDir,
|
||||||
encoding: "utf-8",
|
encoding: "utf-8",
|
||||||
maxBuffer: 10 * 1024 * 1024,
|
maxBuffer: 10 * 1024 * 1024,
|
||||||
}).trim();
|
}).trim();
|
||||||
|
|
||||||
if (!diff) return null; // no changes since baseRef
|
if (!diff) return { kind: "no-changes" }; // genuinely no changes since baseRef
|
||||||
return { hash, subject, diff };
|
return { kind: "ok", hash, subject, diff };
|
||||||
} catch {
|
} catch (error) {
|
||||||
return null;
|
return {
|
||||||
|
kind: "error",
|
||||||
|
error: error instanceof Error ? error.message : String(error),
|
||||||
|
};
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
82
tests/commit-range-diff.test.ts
Normal file
82
tests/commit-range-diff.test.ts
Normal file
@@ -0,0 +1,82 @@
|
|||||||
|
/**
|
||||||
|
* Tests for the tri-state commit-range diff (src/utils.ts getCommitRangeDiff):
|
||||||
|
* a FAILED range computation (invalid/stale base ref, git error) must be a
|
||||||
|
* distinct `error` signal, never collapsed into a clean `no-changes` — a
|
||||||
|
* broken base ref must never be silently treated as a verified task.
|
||||||
|
*
|
||||||
|
* Uses a real throwaway git repo so the shell-out behavior is exercised.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { describe, test, expect, beforeAll, afterAll } from "bun:test";
|
||||||
|
import * as fs from "node:fs";
|
||||||
|
import * as path from "node:path";
|
||||||
|
import * as os from "node:os";
|
||||||
|
import { execSync } from "node:child_process";
|
||||||
|
import { getCommitRangeDiff } from "../src/utils";
|
||||||
|
|
||||||
|
let repoDir: string;
|
||||||
|
|
||||||
|
function sh(cmd: string, cwd: string) {
|
||||||
|
execSync(cmd, { cwd, stdio: "pipe" });
|
||||||
|
}
|
||||||
|
|
||||||
|
beforeAll(() => {
|
||||||
|
repoDir = fs.mkdtempSync(path.join(os.tmpdir(), "ralpi-crd-"));
|
||||||
|
sh("git init -q", repoDir);
|
||||||
|
sh("git config user.email test@example.com", repoDir);
|
||||||
|
sh("git config user.name test", repoDir);
|
||||||
|
fs.writeFileSync(path.join(repoDir, "a.ts"), "one\n", "utf-8");
|
||||||
|
sh("git add -A", repoDir);
|
||||||
|
sh("git commit -q -m init", repoDir);
|
||||||
|
});
|
||||||
|
|
||||||
|
afterAll(() => {
|
||||||
|
fs.rmSync(repoDir, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("getCommitRangeDiff tri-state", () => {
|
||||||
|
test("ok: a resolvable base with committed changes yields the diff", () => {
|
||||||
|
fs.writeFileSync(path.join(repoDir, "a.ts"), "one\ntwo\n", "utf-8");
|
||||||
|
sh("git add -A", repoDir);
|
||||||
|
sh("git commit -q -m change", repoDir);
|
||||||
|
|
||||||
|
const base = execSync("git rev-parse HEAD~1", {
|
||||||
|
cwd: repoDir,
|
||||||
|
encoding: "utf-8",
|
||||||
|
}).trim();
|
||||||
|
|
||||||
|
const result = getCommitRangeDiff(repoDir, base);
|
||||||
|
expect(result.kind).toBe("ok");
|
||||||
|
if (result.kind === "ok") {
|
||||||
|
expect(result.diff).toContain("a.ts");
|
||||||
|
expect(result.hash.length).toBeGreaterThan(0);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test("error: a fake/unresolvable base ref yields the failure signal, not no-changes", () => {
|
||||||
|
// 40 hex chars that never existed in this repo.
|
||||||
|
const fake = "ffffffffffffffffffffffffffffffffffffffff";
|
||||||
|
const result = getCommitRangeDiff(repoDir, fake);
|
||||||
|
expect(result.kind).toBe("error");
|
||||||
|
if (result.kind === "error") {
|
||||||
|
expect(result.error).toContain("cannot be resolved");
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test("error: a non-hex base ref is rejected before reaching the shell", () => {
|
||||||
|
const result = getCommitRangeDiff(repoDir, "HEAD~1; rm -rf /");
|
||||||
|
expect(result.kind).toBe("error");
|
||||||
|
if (result.kind === "error") {
|
||||||
|
expect(result.error).toContain("invalid or stale base ref");
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test("no-changes: an empty range (base == HEAD) yields the no-changes signal", () => {
|
||||||
|
const head = execSync("git rev-parse HEAD", {
|
||||||
|
cwd: repoDir,
|
||||||
|
encoding: "utf-8",
|
||||||
|
}).trim();
|
||||||
|
const result = getCommitRangeDiff(repoDir, head);
|
||||||
|
expect(result.kind).toBe("no-changes");
|
||||||
|
});
|
||||||
|
});
|
||||||
237
tests/diff.test.ts
Normal file
237
tests/diff.test.ts
Normal file
@@ -0,0 +1,237 @@
|
|||||||
|
/**
|
||||||
|
* Tests for the noise-filtered diff engine (src/diff.ts).
|
||||||
|
* Covers: per-file +/− parsing, excluded-file split, totals excluding noise,
|
||||||
|
* malformed-chunk guard, isExcluded reasons, and configurable overrides.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { describe, test, expect } from "bun:test";
|
||||||
|
import {
|
||||||
|
parseDiff,
|
||||||
|
filterNoise,
|
||||||
|
isExcluded,
|
||||||
|
compileIgnorePatterns,
|
||||||
|
EXCLUDED_PATTERNS,
|
||||||
|
} from "../src/diff";
|
||||||
|
|
||||||
|
/** A synthetic unified diff mixing code, a lockfile, a minified file, and a binary. */
|
||||||
|
const SYNTH_DIFF = [
|
||||||
|
"diff --git a/src/index.ts b/src/index.ts",
|
||||||
|
"index 111..222 100644",
|
||||||
|
"--- a/src/index.ts",
|
||||||
|
"+++ b/src/index.ts",
|
||||||
|
"@@ -1,2 +1,4 @@",
|
||||||
|
' import { foo } from "./foo";',
|
||||||
|
"+export const baz = 1;",
|
||||||
|
"+export const qux = 2;",
|
||||||
|
"-foo();",
|
||||||
|
"+bar();",
|
||||||
|
"",
|
||||||
|
"diff --git a/package-lock.json b/package-lock.json",
|
||||||
|
"index 000..111 100644",
|
||||||
|
"--- a/package-lock.json",
|
||||||
|
"+++ b/package-lock.json",
|
||||||
|
"@@ -0,0 +1,3 @@",
|
||||||
|
"+{",
|
||||||
|
'+ "name": "x"',
|
||||||
|
"+}",
|
||||||
|
"",
|
||||||
|
"diff --git a/dist/foo.min.js b/dist/foo.min.js",
|
||||||
|
"index 111..222 100644",
|
||||||
|
"--- a/dist/foo.min.js",
|
||||||
|
"+++ b/dist/foo.min.js",
|
||||||
|
"@@ -1 +1 @@",
|
||||||
|
"-var a=1;",
|
||||||
|
"+var a=2;",
|
||||||
|
"",
|
||||||
|
"diff --git a/assets/logo.png b/assets/logo.png",
|
||||||
|
"index 111..222 100644",
|
||||||
|
"Binary files differ",
|
||||||
|
].join("\n");
|
||||||
|
|
||||||
|
describe("parseDiff", () => {
|
||||||
|
test("splits included vs excluded files and totals only included", () => {
|
||||||
|
const summary = parseDiff(SYNTH_DIFF);
|
||||||
|
|
||||||
|
// Included: only src/index.ts (code). Lockfile, minified, binary excluded.
|
||||||
|
expect(summary.files).toHaveLength(1);
|
||||||
|
expect(summary.files[0]).toEqual({
|
||||||
|
path: "src/index.ts",
|
||||||
|
linesAdded: 3,
|
||||||
|
linesRemoved: 1,
|
||||||
|
ext: "ts",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(summary.excluded).toHaveLength(3);
|
||||||
|
const byPath = new Map(
|
||||||
|
summary.excluded.map((f) => [f.path, f]),
|
||||||
|
);
|
||||||
|
expect(byPath.get("package-lock.json")).toMatchObject({
|
||||||
|
linesAdded: 3,
|
||||||
|
linesRemoved: 0,
|
||||||
|
reason: "lockfile",
|
||||||
|
});
|
||||||
|
expect(byPath.get("dist/foo.min.js")).toMatchObject({
|
||||||
|
linesAdded: 1,
|
||||||
|
linesRemoved: 1,
|
||||||
|
reason: "minified asset",
|
||||||
|
});
|
||||||
|
expect(byPath.get("assets/logo.png")).toMatchObject({
|
||||||
|
linesAdded: 0,
|
||||||
|
linesRemoved: 0,
|
||||||
|
reason: "binary/media asset",
|
||||||
|
});
|
||||||
|
|
||||||
|
// Totals exclude the noise files.
|
||||||
|
expect(summary.totalAdded).toBe(3);
|
||||||
|
expect(summary.totalRemoved).toBe(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("returns empty summary for an empty diff", () => {
|
||||||
|
const summary = parseDiff("");
|
||||||
|
expect(summary.files).toHaveLength(0);
|
||||||
|
expect(summary.excluded).toHaveLength(0);
|
||||||
|
expect(summary.totalAdded).toBe(0);
|
||||||
|
expect(summary.totalRemoved).toBe(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("skips malformed chunks without a/… b/ header without crashing", () => {
|
||||||
|
const malformed =
|
||||||
|
"diff --git weird-line\nindex 111..222\n--- a/x\n+++ b/x\n+x\n" +
|
||||||
|
"\n" +
|
||||||
|
"diff --git a/src/ok.ts b/src/ok.ts\n--- a/src/ok.ts\n+++ b/src/ok.ts\n+ok\n";
|
||||||
|
const summary = parseDiff(malformed);
|
||||||
|
// Only the well-formed chunk is counted.
|
||||||
|
expect(summary.files).toHaveLength(1);
|
||||||
|
expect(summary.files[0].path).toBe("src/ok.ts");
|
||||||
|
expect(summary.totalAdded).toBe(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("does not count +++/--- header lines as additions/removals", () => {
|
||||||
|
const diff = [
|
||||||
|
"diff --git a/src/a.ts b/src/a.ts",
|
||||||
|
"--- a/src/a.ts",
|
||||||
|
"+++ b/src/a.ts",
|
||||||
|
"@@ -0,0 +1,2 @@",
|
||||||
|
"+one",
|
||||||
|
"+two",
|
||||||
|
].join("\n");
|
||||||
|
const summary = parseDiff(diff);
|
||||||
|
expect(summary.files[0].linesAdded).toBe(2);
|
||||||
|
expect(summary.files[0].linesRemoved).toBe(0);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("isExcluded", () => {
|
||||||
|
test("returns the right reason per pattern", () => {
|
||||||
|
expect(isExcluded("package-lock.json")).toBe("lockfile");
|
||||||
|
expect(isExcluded("yarn.lock")).toBe("lockfile");
|
||||||
|
expect(isExcluded("src/app.min.js")).toBe("minified asset");
|
||||||
|
expect(isExcluded("src/styles.min.css")).toBe("minified asset");
|
||||||
|
expect(isExcluded("build/out.js")).toBe("build output");
|
||||||
|
expect(isExcluded("node_modules/foo/index.js")).toBe("dependency");
|
||||||
|
expect(isExcluded("vendor/lib.bundle.js")).toBe("vendored dependency");
|
||||||
|
expect(isExcluded("assets/icon.svg")).toBe("binary/media asset");
|
||||||
|
expect(isExcluded("src/api.generated.ts")).toBe("generated file");
|
||||||
|
expect(isExcluded("test/__snapshots__/x.snap")).toBe("snapshot");
|
||||||
|
expect(isExcluded("dist/x.js.map")).toBe("source map");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("returns undefined for review-relevant files", () => {
|
||||||
|
expect(isExcluded("src/foo.ts")).toBeUndefined();
|
||||||
|
expect(isExcluded("src/index.ts")).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
test("merges caller-supplied extra patterns", () => {
|
||||||
|
expect(isExcluded("src/data.foo", [/\.foo$/])).toBe("extra ignore pattern");
|
||||||
|
expect(isExcluded("src/data.foo")).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
test("EXCLUDED_PATTERNS covers lockfiles, min, generated, snap, map, build, vendor, binaries", () => {
|
||||||
|
for (const pat of [
|
||||||
|
"package-lock.json",
|
||||||
|
"src/app.min.js",
|
||||||
|
"src/thing.generated.ts",
|
||||||
|
"x.snap",
|
||||||
|
"x.js.map",
|
||||||
|
"dist/bundle.js",
|
||||||
|
"node_modules/a/b.js",
|
||||||
|
"vendor/x",
|
||||||
|
"a.png",
|
||||||
|
"f.woff2",
|
||||||
|
]) {
|
||||||
|
const hit = EXCLUDED_PATTERNS.some((r) => r.pattern.test(pat));
|
||||||
|
expect(hit, `${pat} should be covered by a default rule`).toBe(true);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("filterNoise", () => {
|
||||||
|
test("re-emits only included-file chunks", () => {
|
||||||
|
const filtered = filterNoise(SYNTH_DIFF);
|
||||||
|
expect(filtered).toContain("diff --git a/src/index.ts");
|
||||||
|
expect(filtered).not.toContain("package-lock.json");
|
||||||
|
expect(filtered).not.toContain("foo.min.js");
|
||||||
|
expect(filtered).not.toContain("logo.png");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("returns empty when every file is noise", () => {
|
||||||
|
const onlyNoise = [
|
||||||
|
"diff --git a/package-lock.json b/package-lock.json",
|
||||||
|
"--- a/package-lock.json",
|
||||||
|
"+++ b/package-lock.json",
|
||||||
|
"+x",
|
||||||
|
].join("\n");
|
||||||
|
expect(filterNoise(onlyNoise)).toBe("");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("configurable noise rules", () => {
|
||||||
|
test("extraPatterns excludes a matching file from the review diff", () => {
|
||||||
|
const diff = [
|
||||||
|
"diff --git a/src/foo.ts b/src/foo.ts",
|
||||||
|
"--- a/src/foo.ts",
|
||||||
|
"+++ b/src/foo.ts",
|
||||||
|
"+keep",
|
||||||
|
"diff --git a/src/data.foo b/src/data.foo",
|
||||||
|
"--- a/src/data.foo",
|
||||||
|
"+++ b/src/data.foo",
|
||||||
|
"+drop",
|
||||||
|
].join("\n");
|
||||||
|
const opts = { extraPatterns: compileIgnorePatterns(["\\.foo$"]) };
|
||||||
|
const summary = parseDiff(diff, opts);
|
||||||
|
expect(summary.files.map((f) => f.path)).toEqual(["src/foo.ts"]);
|
||||||
|
expect(summary.excluded.map((f) => [f.path, f.reason])).toEqual([
|
||||||
|
["src/data.foo", "extra ignore pattern"],
|
||||||
|
]);
|
||||||
|
expect(filterNoise(diff, opts)).not.toContain("data.foo");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("ignorePaths keeps an excluded-by-default file in scope", () => {
|
||||||
|
const diff = [
|
||||||
|
"diff --git a/package-lock.json b/package-lock.json",
|
||||||
|
"--- a/package-lock.json",
|
||||||
|
"+++ b/package-lock.json",
|
||||||
|
"+a",
|
||||||
|
"+b",
|
||||||
|
"+c",
|
||||||
|
].join("\n");
|
||||||
|
const opts = { ignorePaths: ["package-lock.json"] };
|
||||||
|
const summary = parseDiff(diff, opts);
|
||||||
|
expect(summary.files).toHaveLength(1);
|
||||||
|
expect(summary.files[0].path).toBe("package-lock.json");
|
||||||
|
expect(summary.excluded).toHaveLength(0);
|
||||||
|
expect(summary.totalAdded).toBe(3);
|
||||||
|
expect(filterNoise(diff, opts)).toContain("package-lock.json");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("default behavior unchanged when overrides are unset", () => {
|
||||||
|
const summary = parseDiff(SYNTH_DIFF);
|
||||||
|
expect(summary.files[0].path).toBe("src/index.ts");
|
||||||
|
expect(summary.totalAdded).toBe(3);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("compileIgnorePatterns skips invalid regexes", () => {
|
||||||
|
const compiled = compileIgnorePatterns(["\\.foo$", "(", "ok$"]);
|
||||||
|
expect(compiled.length).toBe(2);
|
||||||
|
});
|
||||||
|
});
|
||||||
96
tests/gitignore-hygiene.test.ts
Normal file
96
tests/gitignore-hygiene.test.ts
Normal file
@@ -0,0 +1,96 @@
|
|||||||
|
import { describe, expect, it } from "bun:test";
|
||||||
|
import * as fs from "node:fs";
|
||||||
|
import * as path from "node:path";
|
||||||
|
import { tempDir } from "./helpers";
|
||||||
|
import { ensureRalpiIgnored } from "../src/utils";
|
||||||
|
|
||||||
|
// ─── Gitignore hygiene: ensureRalpiIgnored ──────────────────────────────────
|
||||||
|
|
||||||
|
describe("ensureRalpiIgnored", () => {
|
||||||
|
it("creates .gitignore with .ralpi/ when absent in a git work tree", () => {
|
||||||
|
const { dir, cleanup } = tempDir();
|
||||||
|
try {
|
||||||
|
fs.mkdirSync(path.join(dir, ".git"));
|
||||||
|
expect(ensureRalpiIgnored(dir)).toBe(true);
|
||||||
|
const content = fs.readFileSync(path.join(dir, ".gitignore"), "utf8");
|
||||||
|
expect(content).toContain(".ralpi/");
|
||||||
|
} finally {
|
||||||
|
cleanup();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("appends .ralpi/ to an existing .gitignore without the marker", () => {
|
||||||
|
const { dir, cleanup } = tempDir();
|
||||||
|
try {
|
||||||
|
fs.mkdirSync(path.join(dir, ".git"));
|
||||||
|
fs.writeFileSync(
|
||||||
|
path.join(dir, ".gitignore"),
|
||||||
|
"node_modules/\n*.log\n",
|
||||||
|
"utf8",
|
||||||
|
);
|
||||||
|
expect(ensureRalpiIgnored(dir)).toBe(true);
|
||||||
|
const content = fs.readFileSync(path.join(dir, ".gitignore"), "utf8");
|
||||||
|
expect(content).toContain("node_modules/");
|
||||||
|
expect(content).toContain(".ralpi/");
|
||||||
|
} finally {
|
||||||
|
cleanup();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves a .gitignore with the marker untouched", () => {
|
||||||
|
const { dir, cleanup } = tempDir();
|
||||||
|
try {
|
||||||
|
fs.mkdirSync(path.join(dir, ".git"));
|
||||||
|
fs.writeFileSync(path.join(dir, ".gitignore"), ".ralpi/\n", "utf8");
|
||||||
|
expect(ensureRalpiIgnored(dir)).toBe(false);
|
||||||
|
expect(fs.readFileSync(path.join(dir, ".gitignore"), "utf8")).toBe(
|
||||||
|
".ralpi/\n",
|
||||||
|
);
|
||||||
|
} finally {
|
||||||
|
cleanup();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("is a no-op outside a git work tree", () => {
|
||||||
|
const { dir, cleanup } = tempDir();
|
||||||
|
try {
|
||||||
|
expect(ensureRalpiIgnored(dir)).toBe(false);
|
||||||
|
expect(fs.existsSync(path.join(dir, ".gitignore"))).toBe(false);
|
||||||
|
} finally {
|
||||||
|
cleanup();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("is memoized per project dir", () => {
|
||||||
|
const { dir, cleanup } = tempDir();
|
||||||
|
try {
|
||||||
|
fs.mkdirSync(path.join(dir, ".git"));
|
||||||
|
expect(ensureRalpiIgnored(dir)).toBe(true);
|
||||||
|
// Second call: same dir already handled → no further work.
|
||||||
|
expect(ensureRalpiIgnored(dir)).toBe(false);
|
||||||
|
fs.writeFileSync(path.join(dir, ".gitignore"), "old\n", "utf8");
|
||||||
|
expect(ensureRalpiIgnored(dir)).toBe(false);
|
||||||
|
expect(fs.readFileSync(path.join(dir, ".gitignore"), "utf8")).toBe(
|
||||||
|
"old\n",
|
||||||
|
);
|
||||||
|
} finally {
|
||||||
|
cleanup();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("works when .git is a file (linked git worktree)", () => {
|
||||||
|
const { dir, cleanup } = tempDir();
|
||||||
|
try {
|
||||||
|
fs.writeFileSync(
|
||||||
|
path.join(dir, ".git"),
|
||||||
|
"gitdir: /some/shared/repo\n",
|
||||||
|
"utf8",
|
||||||
|
);
|
||||||
|
expect(ensureRalpiIgnored(dir)).toBe(true);
|
||||||
|
const content = fs.readFileSync(path.join(dir, ".gitignore"), "utf8");
|
||||||
|
expect(content).toContain(".ralpi/");
|
||||||
|
} finally {
|
||||||
|
cleanup();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
303
tests/review-prompt.test.ts
Normal file
303
tests/review-prompt.test.ts
Normal file
@@ -0,0 +1,303 @@
|
|||||||
|
/**
|
||||||
|
* Tests for the review prompt builders (src/prompts.ts).
|
||||||
|
* Covers: per-file summary table, excluded-files section, oversized-diff
|
||||||
|
* read-instruction (never byte-truncates), custom review focus, and the
|
||||||
|
* configurable noise-filter overrides surfacing in the prompt.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { describe, test, expect } from "bun:test";
|
||||||
|
import {
|
||||||
|
buildReviewPrompt,
|
||||||
|
buildReviewPromptUncommitted,
|
||||||
|
} from "../src/prompts";
|
||||||
|
import { compileIgnorePatterns } from "../src/diff";
|
||||||
|
import type { Task, Project } from "../src/types";
|
||||||
|
|
||||||
|
const task: Task = {
|
||||||
|
id: "01",
|
||||||
|
title: "Implement auth",
|
||||||
|
description: "Add a login flow",
|
||||||
|
status: "completed",
|
||||||
|
dependencies: [],
|
||||||
|
};
|
||||||
|
|
||||||
|
const project: Project = {
|
||||||
|
objective: "Build the app",
|
||||||
|
sourcePath: "README.md",
|
||||||
|
sourceDir: "/tmp",
|
||||||
|
tasks: [task],
|
||||||
|
dependencies: {},
|
||||||
|
};
|
||||||
|
|
||||||
|
/** A diff mixing one code file plus lockfile/minified/binary noise. */
|
||||||
|
const MIXED_DIFF = [
|
||||||
|
"diff --git a/src/auth.ts b/src/auth.ts",
|
||||||
|
"index 111..222 100644",
|
||||||
|
"--- a/src/auth.ts",
|
||||||
|
"+++ b/src/auth.ts",
|
||||||
|
"@@ -1,3 +1,5 @@",
|
||||||
|
' import { hash } from "./hash";',
|
||||||
|
"+export function login() {",
|
||||||
|
"+ return hash(secret);",
|
||||||
|
"- return legacy();",
|
||||||
|
"+}",
|
||||||
|
"",
|
||||||
|
"diff --git a/package-lock.json b/package-lock.json",
|
||||||
|
"index 000..111 100644",
|
||||||
|
"--- a/package-lock.json",
|
||||||
|
"+++ b/package-lock.json",
|
||||||
|
"@@ -0,0 +1,3 @@",
|
||||||
|
"+{",
|
||||||
|
'+ "name": "x"',
|
||||||
|
"+}",
|
||||||
|
"",
|
||||||
|
"diff --git a/assets/logo.png b/assets/logo.png",
|
||||||
|
"index 111..222 100644",
|
||||||
|
"Binary files differ",
|
||||||
|
"",
|
||||||
|
"diff --git a/dist/app.min.js b/dist/app.min.js",
|
||||||
|
"index 111..222 100644",
|
||||||
|
"--- a/dist/app.min.js",
|
||||||
|
"+++ b/dist/app.min.js",
|
||||||
|
"@@ -1 +1 @@",
|
||||||
|
"-var a=1;",
|
||||||
|
"+var a=2;",
|
||||||
|
].join("\n");
|
||||||
|
|
||||||
|
function manyFileDiff(n: number): string {
|
||||||
|
const chunks: string[] = [];
|
||||||
|
for (let i = 0; i < n; i++) {
|
||||||
|
chunks.push(
|
||||||
|
`diff --git a/src/f${String(i).padStart(2, "0")}.ts b/src/f${String(i).padStart(2, "0")}.ts`,
|
||||||
|
"--- a/src/f.ts",
|
||||||
|
"+++ b/src/f.ts",
|
||||||
|
`+line ${i}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return chunks.join("\n");
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("buildReviewPrompt", () => {
|
||||||
|
test("emits a per-file +/− summary table with totals, excluding noise", () => {
|
||||||
|
const prompt = buildReviewPrompt(
|
||||||
|
task,
|
||||||
|
project,
|
||||||
|
"abc1234",
|
||||||
|
"feat: auth",
|
||||||
|
MIXED_DIFF,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(prompt).toContain("### Changed Files");
|
||||||
|
expect(prompt).toContain("| `src/auth.ts` | +3/-1 | ts |");
|
||||||
|
expect(prompt).toContain("| **Total** | **+3/-1** | |");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("surfaces an excluded-files section with path, counts, and reason", () => {
|
||||||
|
const prompt = buildReviewPrompt(
|
||||||
|
task,
|
||||||
|
project,
|
||||||
|
"abc1234",
|
||||||
|
"feat: auth",
|
||||||
|
MIXED_DIFF,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(prompt).toContain("### Excluded Files (3)");
|
||||||
|
expect(prompt).toContain("- `package-lock.json` (+3/-0) — lockfile");
|
||||||
|
expect(prompt).toContain(
|
||||||
|
"- `assets/logo.png` (+0/-0) — binary/media asset",
|
||||||
|
);
|
||||||
|
expect(prompt).toContain("- `dist/app.min.js` (+1/-1) — minified asset");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("never inlines excluded (noise) chunks into the diff block", () => {
|
||||||
|
const prompt = buildReviewPrompt(
|
||||||
|
task,
|
||||||
|
project,
|
||||||
|
"abc1234",
|
||||||
|
"feat: auth",
|
||||||
|
MIXED_DIFF,
|
||||||
|
);
|
||||||
|
|
||||||
|
// The noise chunks themselves are never inlined — only the excluded-files
|
||||||
|
// section names them (as `- path (+x/-y) — reason`, no `diff --git` header).
|
||||||
|
expect(prompt).not.toContain("diff --git a/package-lock.json");
|
||||||
|
expect(prompt).not.toContain("diff --git a/assets/logo.png");
|
||||||
|
expect(prompt).not.toContain("diff --git a/dist/app.min.js");
|
||||||
|
// The cleaned diff block is present with the code file.
|
||||||
|
expect(prompt).toContain("```diff");
|
||||||
|
expect(prompt).toContain("diff --git a/src/auth.ts");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("omits the excluded section entirely when nothing is excluded", () => {
|
||||||
|
const clean = [
|
||||||
|
"diff --git a/src/auth.ts b/src/auth.ts",
|
||||||
|
"--- a/src/auth.ts",
|
||||||
|
"+++ b/src/auth.ts",
|
||||||
|
"+export const x = 1;",
|
||||||
|
].join("\n");
|
||||||
|
const prompt = buildReviewPrompt(
|
||||||
|
task,
|
||||||
|
project,
|
||||||
|
"abc1234",
|
||||||
|
"feat: auth",
|
||||||
|
clean,
|
||||||
|
);
|
||||||
|
expect(prompt).not.toContain("### Excluded Files");
|
||||||
|
expect(prompt).toContain("| `src/auth.ts` | +1/-0 | ts |");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("switches to a file-list + read instruction for >20 files, no truncation", () => {
|
||||||
|
const prompt = buildReviewPrompt(
|
||||||
|
task,
|
||||||
|
project,
|
||||||
|
"abc1234",
|
||||||
|
"feat: many",
|
||||||
|
manyFileDiff(21),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(prompt).toContain("Diff too large");
|
||||||
|
expect(prompt).toContain("Use `read` to inspect the changed files");
|
||||||
|
// No byte-truncated inline diff for oversized inputs.
|
||||||
|
expect(prompt).not.toContain("```diff");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("switches to a file-list + read instruction for a >50KB diff, no truncation", () => {
|
||||||
|
// One file but a huge cleaned diff — crosses MAX_DIFF_BYTES (50_000).
|
||||||
|
const huge = [
|
||||||
|
"diff --git a/src/auth.ts b/src/auth.ts",
|
||||||
|
"--- a/src/auth.ts",
|
||||||
|
"+++ b/src/auth.ts",
|
||||||
|
...Array.from(
|
||||||
|
{ length: 26000 },
|
||||||
|
() => "+padding line to blow past the size threshold",
|
||||||
|
),
|
||||||
|
].join("\n");
|
||||||
|
const prompt = buildReviewPrompt(
|
||||||
|
task,
|
||||||
|
project,
|
||||||
|
"abc1234",
|
||||||
|
"feat: auth",
|
||||||
|
huge,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(prompt).toContain("Diff too large");
|
||||||
|
expect(prompt).toContain("Use `read` to inspect the changed files");
|
||||||
|
expect(prompt).toContain("src/auth.ts");
|
||||||
|
// No byte-truncated inline diff for the oversized input.
|
||||||
|
expect(prompt).not.toContain("```diff");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a small diff over the file-count branch still inlines under size threshold", () => {
|
||||||
|
// 5 files, small diff — under MAX_REVIEW_FILES and MAX_DIFF_BYTES → inlined.
|
||||||
|
const prompt = buildReviewPrompt(
|
||||||
|
task,
|
||||||
|
project,
|
||||||
|
"abc1234",
|
||||||
|
"feat: small",
|
||||||
|
manyFileDiff(5),
|
||||||
|
);
|
||||||
|
expect(prompt).toContain("```diff");
|
||||||
|
expect(prompt).not.toContain("Diff too large");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("inlines a small diff normally (no read-instruction)", () => {
|
||||||
|
const prompt = buildReviewPrompt(
|
||||||
|
task,
|
||||||
|
project,
|
||||||
|
"abc1234",
|
||||||
|
"feat: auth",
|
||||||
|
MIXED_DIFF,
|
||||||
|
);
|
||||||
|
expect(prompt).not.toContain("Diff too large");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("emits a Custom Review Focus section only when focus is set", () => {
|
||||||
|
const withFocus = buildReviewPrompt(
|
||||||
|
task,
|
||||||
|
project,
|
||||||
|
"abc1234",
|
||||||
|
"feat: auth",
|
||||||
|
MIXED_DIFF,
|
||||||
|
{ focus: "check security only" },
|
||||||
|
);
|
||||||
|
expect(withFocus).toContain("## Custom Review Focus");
|
||||||
|
expect(withFocus).toContain("check security only");
|
||||||
|
|
||||||
|
const withoutFocus = buildReviewPrompt(
|
||||||
|
task,
|
||||||
|
project,
|
||||||
|
"abc1234",
|
||||||
|
"feat: auth",
|
||||||
|
MIXED_DIFF,
|
||||||
|
);
|
||||||
|
expect(withoutFocus).not.toContain("## Custom Review Focus");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("surfaces extra ignore patterns and ignorePaths overrides in the prompt", () => {
|
||||||
|
const diff = [
|
||||||
|
"diff --git a/src/keep.ts b/src/keep.ts",
|
||||||
|
"--- a/src/keep.ts",
|
||||||
|
"+++ b/src/keep.ts",
|
||||||
|
"+keep",
|
||||||
|
"diff --git a/package-lock.json b/package-lock.json",
|
||||||
|
"--- a/package-lock.json",
|
||||||
|
"+++ b/package-lock.json",
|
||||||
|
"+a",
|
||||||
|
"+b",
|
||||||
|
"+c",
|
||||||
|
"+d",
|
||||||
|
].join("\n");
|
||||||
|
|
||||||
|
// ignorePaths keeps the lockfile in scope → it shows in the table,
|
||||||
|
// and no excluded section is emitted.
|
||||||
|
const kept = buildReviewPrompt(task, project, "abc1234", "x", diff, {
|
||||||
|
diffOptions: { ignorePaths: ["package-lock.json"] },
|
||||||
|
});
|
||||||
|
expect(kept).toContain("| `package-lock.json` | +4/-0 | json |");
|
||||||
|
expect(kept).not.toContain("### Excluded Files");
|
||||||
|
|
||||||
|
// Without ignorePaths, the lockfile is excluded.
|
||||||
|
const excluded = buildReviewPrompt(task, project, "abc1234", "x", diff);
|
||||||
|
expect(excluded).not.toContain("| `package-lock.json` |");
|
||||||
|
expect(excluded).toContain("### Excluded Files (1)");
|
||||||
|
|
||||||
|
// extraPatterns drops a matching file from scope.
|
||||||
|
const dropped = buildReviewPrompt(task, project, "abc1234", "x", diff, {
|
||||||
|
diffOptions: {
|
||||||
|
extraPatterns: compileIgnorePatterns(["\\.ts$"]),
|
||||||
|
ignorePaths: [],
|
||||||
|
},
|
||||||
|
});
|
||||||
|
expect(dropped).not.toContain("| `src/keep.ts` |");
|
||||||
|
expect(dropped).toContain("### Excluded Files (2)");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("buildReviewPromptUncommitted", () => {
|
||||||
|
test("emits summary table, excluded section, and cleaned diff", () => {
|
||||||
|
const prompt = buildReviewPromptUncommitted(
|
||||||
|
task,
|
||||||
|
project,
|
||||||
|
"M src/auth.ts",
|
||||||
|
MIXED_DIFF,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(prompt).toContain("### Changed Files");
|
||||||
|
expect(prompt).toContain("| `src/auth.ts` | +3/-1 | ts |");
|
||||||
|
expect(prompt).toContain("### Excluded Files (3)");
|
||||||
|
expect(prompt).not.toContain("diff --git a/package-lock.json");
|
||||||
|
expect(prompt).toContain("### Current Tracked Diff (git diff)");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("supports custom focus", () => {
|
||||||
|
const prompt = buildReviewPromptUncommitted(
|
||||||
|
task,
|
||||||
|
project,
|
||||||
|
"M src/auth.ts",
|
||||||
|
MIXED_DIFF,
|
||||||
|
{ focus: "review performance" },
|
||||||
|
);
|
||||||
|
expect(prompt).toContain("## Custom Review Focus");
|
||||||
|
expect(prompt).toContain("review performance");
|
||||||
|
});
|
||||||
|
});
|
||||||
53
tests/review-severity.test.ts
Normal file
53
tests/review-severity.test.ts
Normal file
@@ -0,0 +1,53 @@
|
|||||||
|
/**
|
||||||
|
* Tests for the severity taxonomy alignment in review verdict parsing
|
||||||
|
* (src/review.ts): the `critical` token is accepted and normalized to
|
||||||
|
* ralpi's `blocker` severity, mirroring @piex-dev/review's grading.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { describe, test, expect } from "bun:test";
|
||||||
|
import { extractReview } from "../src/review";
|
||||||
|
|
||||||
|
/** Build a full review-agent output ending in a REVIEW VERDICT block. */
|
||||||
|
function reviewOutput(findings: string[]): string {
|
||||||
|
return [
|
||||||
|
"Prose: looks mostly fine, a few issues to fix.",
|
||||||
|
"## REVIEW VERDICT",
|
||||||
|
"VERDICT: fail",
|
||||||
|
"SUMMARY: Needs fixes.",
|
||||||
|
"FINDINGS:",
|
||||||
|
...findings,
|
||||||
|
].join("\n");
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("extractReview severity normalization", () => {
|
||||||
|
test("maps critical → blocker, keeps warning/nit/info", () => {
|
||||||
|
const out = reviewOutput([
|
||||||
|
"- [critical] src/auth.ts:12 hardcoded secret",
|
||||||
|
"- [warning] src/auth.ts:30 unused import",
|
||||||
|
"- [nit] src/auth.ts:5 style",
|
||||||
|
"- [info] src/auth.ts:1 note",
|
||||||
|
]);
|
||||||
|
const review = extractReview(out, "01", "abc1234");
|
||||||
|
expect(review).not.toBeNull();
|
||||||
|
const severities = review!.findings.map((f) => f.severity);
|
||||||
|
expect(severities).toEqual(["blocker", "warning", "nit", "info"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("normalizes the warn synonym to warning", () => {
|
||||||
|
const out = reviewOutput(["- [warn] src/a.ts:2 thing"]);
|
||||||
|
const review = extractReview(out, "01", "abc1234");
|
||||||
|
expect(review!.findings[0].severity).toBe("warning");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("uppercase CRITICAL token also maps to blocker", () => {
|
||||||
|
const out = reviewOutput(["- [CRITICAL] src/a.ts:2 thing"]);
|
||||||
|
const review = extractReview(out, "01", "abc1234");
|
||||||
|
expect(review!.findings[0].severity).toBe("blocker");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("findings without a severity are still parsed", () => {
|
||||||
|
const out = reviewOutput(["- src/a.ts:2 plain line"]);
|
||||||
|
const review = extractReview(out, "01", "abc1234");
|
||||||
|
expect(review!.findings[0].severity).toBe("info");
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user