From 3c04c62bd7449bed25d42f62035afab878e87f66 Mon Sep 17 00:00:00 2001 From: Osho Emmanuel Date: Mon, 13 Jul 2026 23:39:07 +0100 Subject: [PATCH 01/12] Add typecheck script and clear tsc --noEmit baseline - Add npm run typecheck (tsc --noEmit) as the source-of-truth type gate - Narrow exactOptionalPropertyTypes gaps via conditional spreads and optional-with-undefined fields across agent executor, orchestrator, observability, providers, and scorer - Make CheckItem.line/description optional to match runtime reality (reported items carry optional fields; output formatters use a separate Issue type, so display is unaffected) - Confine @ai-sdk/perplexity LanguageModelV1 -> LanguageModel skew to a single cast; ai@6 dropped V1 types and a provider upgrade is a separate, out-of-scope dependency change - No strict compiler options relaxed; src/agent/* left compiling only --- package.json | 1 + src/agent/executor.ts | 36 ++++++++++----------- src/cli/orchestrator.ts | 2 +- src/evaluators/violation-filter.ts | 4 +-- src/observability/factory.ts | 2 +- src/observability/langfuse-observability.ts | 4 +-- src/prompts/schema.ts | 4 +-- src/providers/perplexity-provider.ts | 7 +++- src/providers/vercel-ai-provider.ts | 22 +++++++------ src/scoring/scorer.ts | 5 ++- 10 files changed, 49 insertions(+), 38 deletions(-) diff --git a/package.json b/package.json index 74f474d5..d9e16b86 100644 --- a/package.json +++ b/package.json @@ -7,6 +7,7 @@ "scripts": { "dev": "tsx src/index.ts", "build": "tsup", + "typecheck": "tsc --noEmit", "start": "node dist/index.js", "lint": "eslint .", "lint:fix": "eslint . --fix", diff --git a/src/agent/executor.ts b/src/agent/executor.ts index 432c6d64..749a5685 100644 --- a/src/agent/executor.ts +++ b/src/agent/executor.ts @@ -1,7 +1,7 @@ import { readdir, readFile } from 'fs/promises'; import * as os from 'os'; import fg from 'fast-glob'; -import { buildCheckLLMSchema, isJudgeResult, type PromptEvaluationResult } from '../prompts/schema'; +import { buildCheckLLMSchema, isJudgeResult, type GateChecks, type PromptEvaluationResult } from '../prompts/schema'; import type { PromptFile } from '../prompts/prompt-loader'; import { Type, Severity } from '../evaluators/types'; import { computeFilterDecision } from '../evaluators/violation-filter'; @@ -14,6 +14,7 @@ import { createReviewSessionStore } from './review-session-store'; import { buildAgentSystemPrompt } from './prompt-builder'; import { AgentToolError } from '../errors'; import { ScanPathResolver } from '../boundaries/scan-path-resolver'; +import type { FilePatternConfig } from '../boundaries/file-section-parser'; import { createAgentTools, listAvailableTools, @@ -53,7 +54,7 @@ export interface RunAgentExecutorParams { prompts: PromptFile[]; provider: LLMProvider; workspaceRoot: string; - scanPaths: Array<{ pattern: string; runRules: string[]; overrides: Record }>; + scanPaths: FilePatternConfig[]; outputFormat: OutputFormat; printMode: boolean; sessionHomeDir?: string; @@ -178,7 +179,7 @@ function findingsFromEvents(events: SessionEvent[]): AgentFinding[] { function buildFileRuleMatches( relativeTargets: string[], prompts: PromptFile[], - scanPaths: Array<{ pattern: string; runRules: string[]; overrides: Record }> + scanPaths: FilePatternConfig[] ): Array<{ file: string; ruleSource: string }> { const resolver = new ScanPathResolver(); const availablePacks = Array.from(new Set(prompts.map((p) => p.pack).filter((p): p is string => !!p))); @@ -301,12 +302,13 @@ function resolveVisibleToolContext(params: { } const resolvedPath = tryResolveRelativePath(workspaceRoot, parsed.data.path); const path = resolvedPath ?? parsed.data.path; + const progressFile = resolvedPath && targetFiles.has(resolvedPath) + ? resolvedPath + : (currentProgressFile ?? defaultProgressFile); return { toolName: 'read_file', path, - progressFile: resolvedPath && targetFiles.has(resolvedPath) - ? resolvedPath - : (currentProgressFile ?? defaultProgressFile), + ...(progressFile ? { progressFile } : {}), signature: `read_file:${path}`, }; } @@ -317,10 +319,11 @@ function resolveVisibleToolContext(params: { } const resolvedPath = tryResolveRelativePath(workspaceRoot, parsed.data.path); const path = resolvedPath ?? parsed.data.path; + const progressFile = currentProgressFile ?? defaultProgressFile; return { toolName: 'list_directory', path, - progressFile: currentProgressFile ?? defaultProgressFile, + ...(progressFile ? { progressFile } : {}), signature: `list_directory:${path}`, }; } @@ -333,14 +336,15 @@ function resolveVisibleToolContext(params: { const resolvedPath = tryResolveRelativePath(workspaceRoot, parsed.data.file); const path = resolvedPath ?? parsed.data.file; const ruleText = parsed.data.reviewInstruction?.trim() || prompt?.body || ''; + const progressFile = resolvedPath && targetFiles.has(resolvedPath) + ? resolvedPath + : (currentProgressFile ?? defaultProgressFile); return { toolName: 'lint', path, ruleName: String(prompt?.meta.name || prompt?.meta.id || 'Rule'), ruleText, - progressFile: resolvedPath && targetFiles.has(resolvedPath) - ? resolvedPath - : (currentProgressFile ?? defaultProgressFile), + ...(progressFile ? { progressFile } : {}), signature: `lint:${path}:${normalizeRuleSource(parsed.data.ruleSource)}:${ruleText}`, }; } @@ -390,11 +394,7 @@ type FindingLikeViolation = { suggestion?: string; fix?: string; confidence?: number; - checks?: { - plausible_non_violation?: boolean; - context_supports_violation?: boolean; - rule_supports_claim?: boolean; - }; + checks?: GateChecks; }; async function appendInlineFinding(params: { @@ -516,8 +516,8 @@ export async function runAgentExecutor(params: RunAgentExecutorParams): Promise< workspaceRoot, promptBySource, targetFiles, - currentProgressFile, - defaultProgressFile: relativeTargets[0], + ...(currentProgressFile ? { currentProgressFile } : {}), + ...(relativeTargets[0] ? { defaultProgressFile: relativeTargets[0] } : {}), }); if (visibleToolContext?.progressFile) { @@ -798,7 +798,7 @@ export async function runAgentExecutor(params: RunAgentExecutorParams): Promise< workspaceRoot, fileRuleMatches, availableTools, - userInstructions, + ...(userInstructions ? { userInstructions } : {}), }), prompt: [ `Workspace root: ${workspaceRoot}`, diff --git a/src/cli/orchestrator.ts b/src/cli/orchestrator.ts index f4325095..6ae4acd4 100644 --- a/src/cli/orchestrator.ts +++ b/src/cli/orchestrator.ts @@ -1194,7 +1194,7 @@ async function evaluateFilesInAgentMode( progressReporter, maxParallelToolCalls: 3, maxRetries: options.agentMaxRetries ?? 10, - userInstructions: options.userInstructionContent, + ...(options.userInstructionContent ? { userInstructions: options.userInstructionContent } : {}), }); let totalErrors = 0; diff --git a/src/evaluators/violation-filter.ts b/src/evaluators/violation-filter.ts index 2d3fc295..c4daa9b5 100644 --- a/src/evaluators/violation-filter.ts +++ b/src/evaluators/violation-filter.ts @@ -56,8 +56,8 @@ export function computeFilterDecision(v: GateViolationLike): FilterDecision { const hasFix = (v.fix ?? "").trim() !== ""; - const hasConfidence = typeof v.confidence === "number"; - const passesConfidence = hasConfidence && v.confidence >= confidenceThreshold; + const confidence = typeof v.confidence === "number" ? v.confidence : undefined; + const passesConfidence = confidence !== undefined && confidence >= confidenceThreshold; if (!passesConfidence) reasons.push(`confidence<${confidenceThreshold}`); const surface = diff --git a/src/observability/factory.ts b/src/observability/factory.ts index 84ec671a..c646fc08 100644 --- a/src/observability/factory.ts +++ b/src/observability/factory.ts @@ -19,6 +19,6 @@ export function createObservability(env: EnvConfig, logger?: Logger): AIObservab publicKey: env.LANGFUSE_PUBLIC_KEY, secretKey: env.LANGFUSE_SECRET_KEY, ...(env.LANGFUSE_BASE_URL ? { baseUrl: env.LANGFUSE_BASE_URL } : {}), - logger, + ...(logger ? { logger } : {}), }); } diff --git a/src/observability/langfuse-observability.ts b/src/observability/langfuse-observability.ts index b8898127..25f4d32c 100644 --- a/src/observability/langfuse-observability.ts +++ b/src/observability/langfuse-observability.ts @@ -12,8 +12,8 @@ export interface LangfuseObservabilityConfig { } export class LangfuseObservability implements AIObservability { - private sdk?: NodeSDK; - private initPromise?: Promise; + private sdk?: NodeSDK | undefined; + private initPromise?: Promise | undefined; private readonly logger: Logger; constructor(private readonly config: LangfuseObservabilityConfig) { diff --git a/src/prompts/schema.ts b/src/prompts/schema.ts index 26c137e1..bd9fd9ee 100644 --- a/src/prompts/schema.ts +++ b/src/prompts/schema.ts @@ -337,8 +337,8 @@ export type JudgeResult = { }; export type CheckItem = { - line: number; - description: string; + line?: number; + description?: string; analysis: string; message?: string; suggestion?: string; diff --git a/src/providers/perplexity-provider.ts b/src/providers/perplexity-provider.ts index c6a05499..432f8c09 100644 --- a/src/providers/perplexity-provider.ts +++ b/src/providers/perplexity-provider.ts @@ -1,4 +1,5 @@ import { generateText } from 'ai'; +import type { LanguageModel } from 'ai'; import { z } from 'zod'; import { createPerplexity } from '@ai-sdk/perplexity'; import type { SearchProvider } from './search-provider'; @@ -46,7 +47,11 @@ export class PerplexitySearchProvider implements SearchProvider { try { const result = await generateText({ - model: this.client('sonar-pro'), + // @ai-sdk/perplexity@1 exposes LanguageModelV1 models, while ai@6's + // generateText types require a LanguageModel (V2/V3). This is a + // third-party SDK version skew, not a repo type hole; resolve by + // upgrading @ai-sdk/perplexity to a V2 release in a follow-up. + model: this.client('sonar-pro') as unknown as LanguageModel, prompt: query, }); diff --git a/src/providers/vercel-ai-provider.ts b/src/providers/vercel-ai-provider.ts index 689a19c0..5cd833fd 100644 --- a/src/providers/vercel-ai-provider.ts +++ b/src/providers/vercel-ai-provider.ts @@ -25,7 +25,7 @@ export class VercelAIProvider implements LLMProvider { private config: VercelAIConfig; private builder: RequestBuilder; private logger: Logger; - private observability?: AIObservability; + private observability?: AIObservability | undefined; constructor(config: VercelAIConfig, builder?: RequestBuilder) { this.config = { @@ -73,12 +73,14 @@ export class VercelAIProvider implements LLMProvider { } try { + const evaluator = this.extractContextValue(context, 'evaluatorName', 'evaluator'); + const rule = this.extractContextValue(context, 'ruleName', 'rule'); const observabilityOptions = this.getObservabilityOptions({ operation: 'structured-eval', provider: this.config.providerName ?? 'unknown', model: this.config.modelName ?? 'unknown', - evaluator: this.extractContextValue(context, 'evaluatorName', 'evaluator'), - rule: this.extractContextValue(context, 'ruleName', 'rule'), + ...(evaluator ? { evaluator } : {}), + ...(rule ? { rule } : {}), }); const result = await generateText({ @@ -187,14 +189,14 @@ export class VercelAIProvider implements LLMProvider { } } - return { - usage: result.usage - ? { - inputTokens: result.usage.inputTokens ?? 0, - outputTokens: result.usage.outputTokens ?? 0, + return result.usage + ? { + usage: { + inputTokens: result.usage.inputTokens ?? 0, + outputTokens: result.usage.outputTokens ?? 0, + }, } - : undefined, - }; + : {}; } private getObservabilityOptions(context: AIExecutionContext): Record { diff --git a/src/scoring/scorer.ts b/src/scoring/scorer.ts index ecea0db6..24f0ced5 100644 --- a/src/scoring/scorer.ts +++ b/src/scoring/scorer.ts @@ -48,7 +48,10 @@ export function calculateCheckScore( ): CheckResult { const strictness = resolveStrictness(options.strictness); - const mappedViolations = violations.map((item) => ({ ...item, criterionName: item.description })); + const mappedViolations = violations.map((item) => ({ + ...item, + ...(item.description !== undefined ? { criterionName: item.description } : {}), + })); // Density Calculation: Violations per 100 words const density = (mappedViolations.length / wordCount) * 100; From e319bba09fb1e07af044a55161da89ebb6510313 Mon Sep 17 00:00:00 2001 From: Osho Emmanuel Date: Mon, 13 Jul 2026 23:40:19 +0100 Subject: [PATCH 02/12] Exclude docs/research from eslint - Add docs/research/** to eslint ignores; the directory holds local, untracked throwaway research scripts that are not part of the shipped package and should not be type-checked by lint --- eslint.config.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index 8549cd13..4bec57ff 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -10,7 +10,7 @@ import unicorn from "eslint-plugin-unicorn"; export default defineConfig([ // Ignored + housekeeping - { ignores: ["node_modules", "coverage", "dist", "build"] }, + { ignores: ["node_modules", "coverage", "dist", "build", "docs/research/**"] }, { linterOptions: { reportUnusedDisableDirectives: true } }, // Make resolver settings global From 20e0acc8204f1b67858ab7b31bf3f5b023c2799a Mon Sep 17 00:00:00 2001 From: Osho Emmanuel Date: Mon, 13 Jul 2026 23:41:25 +0100 Subject: [PATCH 03/12] Add vitest.config.ts for stable test resolution - Project had no vitest config; vitest ran on pure defaults - Pin test discovery to tests/**/*.test.ts and inline ora, @langfuse/otel, and @opentelemetry/sdk-node so suites that transitively import agent/observability modules resolve reliably - No change to the 45 suites / 323 tests baseline --- vitest.config.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 vitest.config.ts diff --git a/vitest.config.ts b/vitest.config.ts new file mode 100644 index 00000000..b5f7a7b4 --- /dev/null +++ b/vitest.config.ts @@ -0,0 +1,16 @@ +import { defineConfig } from 'vitest/config'; + +export default defineConfig({ + test: { + // Tests live under tests/ and follow the *.test.ts convention. + include: ['tests/**/*.test.ts'], + // Inline heavy/ESM-only deps so suites that transitively import the + // agent (ora) or observability (@langfuse/otel, @opentelemetry/sdk-node) + // modules resolve reliably without relying on scattered per-file mocks. + server: { + deps: { + inline: ['ora', '@langfuse/otel', '@opentelemetry/sdk-node'], + }, + }, + }, +}); From 3580e4971c2e6cfa8ce2308d350803c58d43ddba Mon Sep 17 00:00:00 2001 From: Osho Emmanuel Date: Mon, 13 Jul 2026 23:41:59 +0100 Subject: [PATCH 04/12] Add npm run verify aggregate gate - Runs typecheck, lint, and test:run in sequence - Single command for CI and downstream refactor phases to prove the verification baseline --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index d9e16b86..3a73ad4d 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,8 @@ "lint:fix": "eslint . --fix", "test": "vitest", "test:run": "vitest run", - "test:ci": "vitest run --coverage" + "test:ci": "vitest run --coverage", + "verify": "npm run typecheck && npm run lint && npm run test:run" }, "bin": { "vectorlint": "./dist/index.js", From 0fe8700b2e4d2ded0f51ef10e30e1f7fb4f39af5 Mon Sep 17 00:00:00 2001 From: Osho Emmanuel Date: Mon, 13 Jul 2026 23:47:58 +0100 Subject: [PATCH 05/12] Add typecheck CI job and align test workflow Node version - New typecheck.yml runs tsc --noEmit on push/PR for main and release-docs - Bump test.yml from Node 18 to 20 to satisfy package.json engines>=20.6 - Catches type regressions in CI before merge, not just via ESLint --- .github/workflows/test.yml | 2 +- .github/workflows/typecheck.yml | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/typecheck.yml diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index dcf03d6f..06411499 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -18,7 +18,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v4 with: - node-version: '18' + node-version: '20' cache: 'npm' - name: Install dependencies diff --git a/.github/workflows/typecheck.yml b/.github/workflows/typecheck.yml new file mode 100644 index 00000000..73dad374 --- /dev/null +++ b/.github/workflows/typecheck.yml @@ -0,0 +1,28 @@ +name: Typecheck + +on: + push: + branches: [main, release-docs] + pull_request: + branches: [main, release-docs] + +jobs: + typecheck: + name: tsc --noEmit + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + cache: 'npm' + + - name: Install dependencies + run: npm ci + + - name: Run typecheck + run: npm run typecheck From 6c271f121b80e05e5e3d7ae2d7b6c02352755abd Mon Sep 17 00:00:00 2001 From: Osho Emmanuel Date: Mon, 13 Jul 2026 23:58:45 +0100 Subject: [PATCH 06/12] Deprecate --mode agent and fall back to standard - README Agent Mode section replaced with under-review notice linking the audit - --mode agent now warns through the injected logger and runs standard evaluation; the agent executor code is retained (unreachable from the CLI) pending Phase 4 removal - Add logger to EvaluationOptions so the deprecation warning routes through the logging abstraction instead of console - Rewrite orchestrator-agent-output tests to assert deprecation + fallback BREAKING: --mode agent no longer runs the autonomous workspace-agent loop --- README.md | 29 +- src/cli/commands.ts | 3 +- src/cli/orchestrator.ts | 9 +- src/cli/types.ts | 2 + tests/orchestrator-agent-output.test.ts | 1153 ++--------------------- 5 files changed, 80 insertions(+), 1116 deletions(-) diff --git a/README.md b/README.md index 859ea51a..f6511e6a 100644 --- a/README.md +++ b/README.md @@ -145,29 +145,16 @@ Notes: - Prompts and outputs are recorded when Langfuse observability is enabled. - Do not send secrets, credentials, or PII unless your policy explicitly allows observability tooling to access that data. -## Agent Mode +## Agent Mode (under review) -Agent mode is for reviews that need context from several files such as -documentation drift and cross-file accuracy. - -Run VectorLint in autonomous agent mode: - -```bash -vectorlint doc.md --mode agent -``` - -For machine-parseable output: - -```bash -vectorlint doc.md --mode agent --output json -``` - -To suppress interactive progress in line output: - -```bash -vectorlint doc.md --mode agent --print -``` +The `--mode agent` flag is under active rework. It currently enables an +autonomous cross-file review mode that is being **removed** in favor of a +bounded harness model. See +[`docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md`](docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md) +for the decision and the refactor plan. +Until the refactor lands, `--mode agent` prints a deprecation warning and +falls back to standard mode. Do not build integrations against it. ## Contributing diff --git a/src/cli/commands.ts b/src/cli/commands.ts index 369eda1f..50dbffc8 100644 --- a/src/cli/commands.ts +++ b/src/cli/commands.ts @@ -82,7 +82,7 @@ export function registerMainCommand(program: Command): void { .option('--show-prompt-trunc', 'Print truncated prompt/content previews (500 chars)') .option('--debug-json', 'Write debug JSON artifacts (raw model output + filter decisions)') .option('--output ', `Output format: ${OUTPUT_FORMATS.join(', ')}`, OUTPUT_FORMATS[0]) - .option('--mode ', 'Execution mode: standard (default) or agent', DEFAULT_REVIEW_MODE) + .option('--mode ', 'Execution mode: standard (default). "agent" is deprecated and falls back to standard.', DEFAULT_REVIEW_MODE) .option('-p, --print', 'Suppress interactive progress output in agent mode') .option('--config ', `Path to custom ${DEFAULT_CONFIG_FILENAME} config file`) .argument('[paths...]', 'files or directories to check (required)') @@ -217,6 +217,7 @@ export function registerMainCommand(program: Command): void { ...(searchProvider ? { searchProvider } : {}), concurrency: config.concurrency, verbose: cliOptions.verbose, + logger: runtimeLogger, debugJson: cliOptions.debugJson, outputFormat: cliOptions.output, mode: cliOptions.mode, diff --git a/src/cli/orchestrator.ts b/src/cli/orchestrator.ts index 6ae4acd4..f8df367a 100644 --- a/src/cli/orchestrator.ts +++ b/src/cli/orchestrator.ts @@ -1168,6 +1168,9 @@ async function buildAgentRuleScores( return results; } +// Retained in quarantine: unreachable from the CLI after --mode agent was +// deprecated (now falls back to standard evaluation). Removed in Phase 4. +// eslint-disable-next-line @typescript-eslint/no-unused-vars -- intentional quarantine async function evaluateFilesInAgentMode( targets: string[], options: EvaluationOptions, @@ -1322,7 +1325,11 @@ export async function evaluateFiles( } if (mode === AGENT_REVIEW_MODE) { - return evaluateFilesInAgentMode(targets, options, outputFormat, jsonFormatter); + options.logger?.warn( + '--mode agent is deprecated and now falls back to standard mode. ' + + 'See docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md.', + ); + // Fall through to standard evaluation; do not call evaluateFilesInAgentMode. } for (const file of targets) { diff --git a/src/cli/types.ts b/src/cli/types.ts index a73f55e6..7f95192a 100644 --- a/src/cli/types.ts +++ b/src/cli/types.ts @@ -10,6 +10,7 @@ import { RdJsonFormatter } from '../output/rdjson-formatter'; import type { PromptEvaluationResult, JudgeResult } from '../prompts/schema'; import { Severity } from '../evaluators/types'; import type { TokenUsageStats, PricingConfig } from '../providers/token-usage'; +import type { Logger } from '../logging/logger'; export enum OutputFormat { Line = "line", @@ -48,6 +49,7 @@ export interface EvaluationOptions { agentMaxRetries?: number; pricing?: PricingConfig; userInstructionContent?: string; + logger?: Logger; } export interface EvaluationResult { diff --git a/tests/orchestrator-agent-output.test.ts b/tests/orchestrator-agent-output.test.ts index e08bbdaa..0b61ea63 100644 --- a/tests/orchestrator-agent-output.test.ts +++ b/tests/orchestrator-agent-output.test.ts @@ -2,28 +2,21 @@ import { mkdtempSync, rmSync, writeFileSync } from 'fs'; import * as path from 'path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { evaluateFiles } from '../src/cli/orchestrator'; -import { AGENT_REVIEW_MODE, OutputFormat } from '../src/cli/types'; +import { AGENT_REVIEW_MODE, DEFAULT_REVIEW_MODE, OutputFormat } from '../src/cli/types'; import type { PromptFile } from '../src/prompts/prompt-loader'; import type { LLMProvider } from '../src/providers/llm-provider'; +import type { Logger } from '../src/logging/logger'; import { Severity } from '../src/evaluators/types'; -function makePrompt(params?: { - id?: string; - name?: string; - source?: string; - body?: string; -}): PromptFile { - const id = params?.id ?? 'consistency'; - const name = params?.name ?? 'Consistency'; - const source = params?.source ?? 'packs/default/consistency.md'; - const body = params?.body ?? 'Find inconsistent wording'; - +function makePrompt(): PromptFile { + const id = 'consistency'; + const name = 'Consistency'; return { id, filename: `${id}.md`, - fullPath: source, + fullPath: 'packs/default/consistency.md', pack: 'Default', - body, + body: 'Find inconsistent wording', meta: { id: name, name, @@ -33,91 +26,39 @@ function makePrompt(params?: { }; } -function makeProvider(): LLMProvider { - return { - runPromptStructured() { - return Promise.resolve({ data: { reasoning: 'ok', violations: [] } }); - }, - runAgentToolLoop: async (params: Record) => { - const tools = params.tools as Record Promise }>; - await tools.lint.execute({ - file: 'doc.md', - ruleSource: 'packs/default/consistency.md', - }); - await tools.report_finding.execute({ - kind: 'top-level', - ruleSource: 'packs/default/consistency.md', - message: 'Cross-file issue found', - references: [{ file: 'doc.md', startLine: 1, endLine: 1 }], - }); - await tools.finalize_review.execute({}); - return { usage: { inputTokens: 3, outputTokens: 2 } }; - }, - } as unknown as LLMProvider; -} +type LoggerSpy = Logger & { warn: ReturnType }; -function makeTopLevelOnlyProvider(): LLMProvider { +function makeLogger(): LoggerSpy { return { - runPromptStructured() { - return Promise.resolve({ data: { reasoning: 'ok', violations: [] } }); - }, - runAgentToolLoop: async (params: Record) => { - const tools = params.tools as Record Promise }>; - await tools.report_finding.execute({ - kind: 'top-level', - ruleSource: 'packs/default/consistency.md', - message: 'Top-level without references', - }); - await tools.finalize_review.execute({}); - return { usage: { inputTokens: 2, outputTokens: 1 } }; - }, - } as unknown as LLMProvider; + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + } as unknown as LoggerSpy; } -function makeCrossFileTopLevelProvider(): LLMProvider { - return { - runPromptStructured() { - return Promise.resolve({ data: { reasoning: 'ok', violations: [] } }); - }, - runAgentToolLoop: async (params: Record) => { - const tools = params.tools as Record Promise }>; - await tools.report_finding.execute({ - kind: 'top-level', - ruleSource: 'packs/default/consistency.md', - message: 'Cross-file issue found', - references: [{ file: 'other.md', startLine: 1, endLine: 1 }], - }); - await tools.finalize_review.execute({}); - return { usage: { inputTokens: 2, outputTokens: 1 } }; - }, - } as unknown as LLMProvider; +interface StandardProviderSpies { + provider: LLMProvider; + runPromptStructured: ReturnType; + runAgentToolLoop: ReturnType; } -function makeNoFinalizeProvider(): LLMProvider { - return { - runPromptStructured() { - return Promise.resolve({ data: { reasoning: 'ok', violations: [] } }); - }, - runAgentToolLoop: async (params: Record) => { - const tools = params.tools as Record Promise }>; - await tools.lint.execute({ - file: 'doc.md', - ruleSource: 'packs/default/consistency.md', - }); - await tools.report_finding.execute({ - kind: 'top-level', - ruleSource: 'packs/default/consistency.md', - message: 'Finding recorded before session ended', - references: [{ file: 'doc.md', startLine: 1, endLine: 1 }], - }); - // intentionally omit finalize_review to simulate missing-finalize scenario - return { usage: { inputTokens: 3, outputTokens: 2 } }; - }, - } as unknown as LLMProvider; +function makeStandardProvider(): StandardProviderSpies { + const runPromptStructured = vi.fn().mockResolvedValue({ + data: { reasoning: 'ok', violations: [] }, + }); + const runAgentToolLoop = vi.fn().mockResolvedValue({ + usage: { inputTokens: 0, outputTokens: 0 }, + }); + const provider = { runPromptStructured, runAgentToolLoop } as unknown as LLMProvider; + return { provider, runPromptStructured, runAgentToolLoop }; } -describe('agent orchestrator output', () => { - const originalIsTTY = Object.getOwnPropertyDescriptor(process.stderr, 'isTTY'); +// `--mode agent` is deprecated. These tests prove the CLI/evaluateFiles path no +// longer reaches the autonomous agent executor: it warns through the injected +// logger and falls back to standard evaluation. The retained agent executor +// code is covered directly by tests/agent/* and removed in Phase 4. +describe('agent mode deprecation', () => { const tempRepos: string[] = []; function createTempRepo(): string { @@ -137,55 +78,15 @@ describe('agent orchestrator output', () => { for (const repo of tempRepos.splice(0, tempRepos.length)) { rmSync(repo, { recursive: true, force: true }); } - if (originalIsTTY) { - Object.defineProperty(process.stderr, 'isTTY', originalIsTTY); - } }); - it('produces agent-mode findings and summary when mode is set to agent', async () => { + it('emits a deprecation warning through the injected logger and falls back to standard evaluation', async () => { const repo = createTempRepo(); const file = path.join(repo, 'doc.md'); writeFileSync(file, 'bad phrase\n', 'utf8'); - const result = await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider: makeProvider(), - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Json, - mode: AGENT_REVIEW_MODE as never, - printMode: true, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - expect(result.totalFiles).toBe(1); - expect(result.totalWarnings).toBeGreaterThan(0); - expect(result.hadOperationalErrors).toBe(false); - }); - - it('includes nested lint usage in the final agent-mode token totals', async () => { - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - const provider: LLMProvider = { - runPromptStructured() { - return Promise.resolve({ - data: { reasoning: 'ok', violations: [] }, - usage: { inputTokens: 7, outputTokens: 3 }, - }); - }, - runAgentToolLoop: async (params: Record) => { - const tools = params.tools as Record Promise }>; - await tools.lint.execute({ - file: 'doc.md', - ruleSource: 'packs/default/consistency.md', - }); - await tools.finalize_review.execute({}); - return { usage: { inputTokens: 11, outputTokens: 5 } }; - }, - } as unknown as LLMProvider; + const { provider, runPromptStructured, runAgentToolLoop } = makeStandardProvider(); + const logger = makeLogger(); const result = await evaluateFiles([file], { prompts: [makePrompt()], @@ -194,338 +95,31 @@ describe('agent orchestrator output', () => { concurrency: 1, verbose: false, outputFormat: OutputFormat.Json, - mode: AGENT_REVIEW_MODE as never, + mode: AGENT_REVIEW_MODE, printMode: true, scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - expect(result.tokenUsage).toEqual({ - totalInputTokens: 18, - totalOutputTokens: 8, + logger, }); - }); - - it('keeps json output shape consistent with formatter-based structure', async () => { - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider: makeProvider(), - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Json, - mode: AGENT_REVIEW_MODE as never, - printMode: true, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const payload = JSON.parse( - String(vi.mocked(console.log).mock.calls.at(-1)?.[0]) - ) as { - files?: Record; - summary?: { files?: number; errors?: number; warnings?: number }; - metadata?: { version?: string; timestamp?: string }; - }; - - expect(payload.files).toBeDefined(); - expect(payload.summary).toBeDefined(); - expect(typeof payload.summary?.files).toBe('number'); - expect(typeof payload.summary?.errors).toBe('number'); - expect(typeof payload.summary?.warnings).toBe('number'); - expect(payload.metadata?.timestamp).toBeTruthy(); - }); - - it('keeps top-level json keys consistent between standard and agent modes', async () => { - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider: makeProvider(), - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Json, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const standardPayload = JSON.parse( - String(vi.mocked(console.log).mock.calls.at(-1)?.[0]) - ) as Record; - - vi.mocked(console.log).mockClear(); - - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider: makeProvider(), - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Json, - mode: AGENT_REVIEW_MODE as never, - printMode: true, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const agentPayload = JSON.parse( - String(vi.mocked(console.log).mock.calls.at(-1)?.[0]) - ) as Record; - - expect(Object.keys(agentPayload).sort()).toEqual( - Object.keys(standardPayload).sort() + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('deprecated'), ); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md'), + ); + // Standard evaluation ran; the autonomous executor did not. + expect(runAgentToolLoop).not.toHaveBeenCalled(); + expect(runPromptStructured).toHaveBeenCalled(); + expect(result.totalFiles).toBe(1); }); - it('emits configured agent progress messages in line output mode', async () => { - Object.defineProperty(process.stderr, 'isTTY', { - configurable: true, - value: true, - }); - const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); - - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider: makeProvider(), - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Line, - mode: AGENT_REVIEW_MODE as never, - printMode: false, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const stderrOutput = stderrSpy.mock.calls.map((call) => String(call[0])).join(''); - expect(stderrOutput).toContain('Reviewing doc.md for Consistency'); - expect(stderrOutput).toContain(' └ Found no issues in doc.md'); - expect(stderrOutput).not.toContain('[vectorlint]'); - expect(stderrOutput).toMatch(/Completed review in \d+s\./); - }); - - it('renders visible tool invocations and results while hiding internal agent tools', async () => { - Object.defineProperty(process.stderr, 'isTTY', { - configurable: true, - value: true, - }); - const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); - - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - const provider: LLMProvider = { - runPromptStructured() { - return Promise.resolve({ data: { reasoning: 'ok', violations: [] } }); - }, - runAgentToolLoop: async (params: Record) => { - const tools = params.tools as Record Promise }>; - - await tools.lint.execute({ - file: 'doc.md', - ruleSource: 'packs/default/consistency.md', - }); - await tools.search_files.execute({ pattern: '**/*.md' }); - await tools.read_file.execute({ path: 'doc.md' }); - await tools.list_directory.execute({ path: '.' }); - await tools.search_content.execute({ pattern: 'bad phrase', path: '.', glob: '**/*.md' }); - await tools.finalize_review.execute({}); - - return { usage: { inputTokens: 6, outputTokens: 2 } }; - }, - } as unknown as LLMProvider; - - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider, - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Line, - mode: AGENT_REVIEW_MODE as never, - printMode: false, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const stderrOutput = stderrSpy.mock.calls.map((call) => String(call[0])).join(''); - expect(stderrOutput).toContain('Lint("Find inconsistent wording...")'); - expect(stderrOutput).toContain('Found no issues in doc.md'); - expect(stderrOutput).toContain('Read(doc.md)'); - expect(stderrOutput).toContain('Read 1 line from doc.md'); - expect(stderrOutput).toContain('List(.)'); - expect(stderrOutput).toContain('Listed 1 entry in .'); - expect(stderrOutput).not.toContain('SearchFiles('); - expect(stderrOutput).not.toContain('SearchContent('); - expect(stderrOutput).not.toContain('Finalize('); - }); - - it('renders interactive tool lines without a trailing newline', async () => { - Object.defineProperty(process.stderr, 'isTTY', { - configurable: true, - value: true, - }); - const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); - - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - const provider: LLMProvider = { - runPromptStructured() { - return Promise.resolve({ data: { reasoning: 'ok', violations: [] } }); - }, - runAgentToolLoop: async (params: Record) => { - const tools = params.tools as Record Promise }>; - await tools.lint.execute({ - file: 'doc.md', - ruleSource: 'packs/default/consistency.md', - }); - await tools.finalize_review.execute({}); - return { usage: { inputTokens: 2, outputTokens: 1 } }; - }, - } as unknown as LLMProvider; - - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider, - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Line, - mode: AGENT_REVIEW_MODE as never, - printMode: false, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const firstToolLine = stderrSpy.mock.calls - .map((call) => String(call[0])) - .find((chunk) => chunk.includes(' └ ')); - - expect(firstToolLine).toBeDefined(); - expect(firstToolLine?.endsWith('\n')).toBe(false); - }); - - it('uses in-place progress updates for repeated tool calls instead of appending only plain lines', async () => { - Object.defineProperty(process.stderr, 'isTTY', { - configurable: true, - value: true, - }); - const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); - - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - const provider: LLMProvider = { - runPromptStructured() { - return Promise.resolve({ data: { reasoning: 'ok', violations: [] } }); - }, - runAgentToolLoop: async (params: Record) => { - const tools = params.tools as Record Promise }>; - await tools.lint.execute({ file: 'doc.md', ruleSource: 'packs/default/ai-pattern.md' }); - await tools.lint.execute({ file: 'doc.md', ruleSource: 'packs/default/consistency.md' }); - await tools.lint.execute({ file: 'doc.md', ruleSource: 'packs/default/wordiness.md' }); - await tools.finalize_review.execute({}); - return { usage: { inputTokens: 3, outputTokens: 2 } }; - }, - } as unknown as LLMProvider; - - await evaluateFiles([file], { - prompts: [ - makePrompt({ id: 'ai-pattern', name: 'AI Pattern', source: 'packs/default/ai-pattern.md' }), - makePrompt({ id: 'consistency', name: 'Consistency', source: 'packs/default/consistency.md' }), - makePrompt({ id: 'wordiness', name: 'Wordiness', source: 'packs/default/wordiness.md' }), - ], - rulesPath: undefined, - provider, - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Line, - mode: AGENT_REVIEW_MODE as never, - printMode: false, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const stderrOutput = stderrSpy.mock.calls.map((call) => String(call[0])).join(''); - expect(stderrOutput).toContain('\x1b[1A'); - }); - - it('updates progress rule labels as the active lint rule changes', async () => { - Object.defineProperty(process.stderr, 'isTTY', { - configurable: true, - value: true, - }); - const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); - - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - const provider: LLMProvider = { - runPromptStructured() { - return Promise.resolve({ data: { reasoning: 'ok', violations: [] } }); - }, - runAgentToolLoop: async (params: Record) => { - const tools = params.tools as Record Promise }>; - await tools.lint.execute({ file: 'doc.md', ruleSource: 'packs/default/ai-pattern.md' }); - await tools.lint.execute({ file: 'doc.md', ruleSource: 'packs/default/consistency.md' }); - await tools.finalize_review.execute({}); - return { usage: { inputTokens: 2, outputTokens: 1 } }; - }, - } as unknown as LLMProvider; - - await evaluateFiles([file], { - prompts: [ - makePrompt({ id: 'ai-pattern', name: 'AI Pattern', source: 'packs/default/ai-pattern.md' }), - makePrompt({ id: 'consistency', name: 'Consistency', source: 'packs/default/consistency.md' }), - ], - rulesPath: undefined, - provider, - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Line, - mode: AGENT_REVIEW_MODE as never, - printMode: false, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const stderrOutput = stderrSpy.mock.calls.map((call) => String(call[0])).join(''); - expect(stderrOutput).toContain('for AI Pattern'); - expect(stderrOutput).toContain('for Consistency'); - }); - - it('shows visible tool retry status after a visible tool failure', async () => { - Object.defineProperty(process.stderr, 'isTTY', { - configurable: true, - value: true, - }); - const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); - + it('does not invoke the agent executor in line output mode', async () => { const repo = createTempRepo(); const file = path.join(repo, 'doc.md'); - const retriable = path.join(repo, 'retriable.md'); writeFileSync(file, 'bad phrase\n', 'utf8'); - const provider: LLMProvider = { - runPromptStructured() { - return Promise.resolve({ data: { reasoning: 'ok', violations: [] } }); - }, - runAgentToolLoop: async (params: Record) => { - const tools = params.tools as Record Promise }>; - await expect(tools.read_file.execute({ path: 'retriable.md' })).rejects.toThrow(); - writeFileSync(retriable, 'hello\n', 'utf8'); - await tools.read_file.execute({ path: 'retriable.md' }); - await tools.finalize_review.execute({}); - return { usage: { inputTokens: 2, outputTokens: 1 } }; - }, - } as unknown as LLMProvider; + const { provider, runPromptStructured, runAgentToolLoop } = makeStandardProvider(); + const logger = makeLogger(); await evaluateFiles([file], { prompts: [makePrompt()], @@ -534,115 +128,23 @@ describe('agent orchestrator output', () => { concurrency: 1, verbose: false, outputFormat: OutputFormat.Line, - mode: AGENT_REVIEW_MODE as never, + mode: AGENT_REVIEW_MODE, printMode: false, scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const stderrOutput = stderrSpy.mock.calls.map((call) => String(call[0])).join(''); - expect(stderrOutput).toContain('Error reading retriable.md'); - expect(stderrOutput).toContain('Retrying Read(retriable.md)...'); - expect(stderrOutput).toContain('Read 1 line from retriable.md'); - }); - - it('shows visible-tool path errors even when path validation fails before file access', async () => { - Object.defineProperty(process.stderr, 'isTTY', { - configurable: true, - value: true, + logger, }); - const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); - - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - const provider: LLMProvider = { - runPromptStructured() { - return Promise.resolve({ data: { reasoning: 'ok', violations: [] } }); - }, - runAgentToolLoop: async (params: Record) => { - const tools = params.tools as Record Promise }>; - await expect(tools.read_file.execute({ path: '../outside.md' })).rejects.toThrow(); - await tools.finalize_review.execute({}); - return { usage: { inputTokens: 2, outputTokens: 1 } }; - }, - } as unknown as LLMProvider; - - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider, - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Line, - mode: AGENT_REVIEW_MODE as never, - printMode: false, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - const stderrOutput = stderrSpy.mock.calls.map((call) => String(call[0])).join(''); - expect(stderrOutput).toContain('Error reading ../outside.md'); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('deprecated')); + expect(runAgentToolLoop).not.toHaveBeenCalled(); + expect(runPromptStructured).toHaveBeenCalled(); }); - it('shows quality scores in agent line output and keeps operational failures explicit when finalize_review is missing', async () => { - Object.defineProperty(process.stderr, 'isTTY', { - configurable: true, - value: true, - }); - const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); - + it('stays silent in standard mode without a logger', async () => { const repo = createTempRepo(); const file = path.join(repo, 'doc.md'); writeFileSync(file, 'bad phrase\n', 'utf8'); - const provider: LLMProvider = { - runPromptStructured() { - return Promise.resolve({ - data: { - reasoning: 'detected issue', - violations: [ - { - line: 1, - quoted_text: 'bad phrase', - context_before: '', - context_after: '', - description: 'Bad phrase used', - analysis: 'This wording is inconsistent.', - message: 'Use consistent wording', - suggestion: 'Replace bad phrase', - fix: 'better phrase', - rule_quote: 'Avoid vague wording', - checks: { - rule_supports_claim: true, - evidence_exact: true, - context_supports_violation: true, - plausible_non_violation: false, - fix_is_drop_in: true, - fix_preserves_meaning: true, - }, - check_notes: { - rule_supports_claim: 'clear', - evidence_exact: 'exact', - context_supports_violation: 'yes', - plausible_non_violation: 'none', - fix_is_drop_in: 'yes', - fix_preserves_meaning: 'yes', - }, - confidence: 0.95, - }, - ], - }, - }); - }, - runAgentToolLoop: async (params: Record) => { - const tools = params.tools as Record Promise }>; - await tools.lint.execute({ - file: 'doc.md', - ruleSource: 'packs/default/consistency.md', - }); - return { usage: { inputTokens: 6, outputTokens: 3 } }; - }, - } as unknown as LLMProvider; + const { provider, runAgentToolLoop } = makeStandardProvider(); const result = await evaluateFiles([file], { prompts: [makePrompt()], @@ -650,549 +152,14 @@ describe('agent orchestrator output', () => { provider, concurrency: 1, verbose: false, - outputFormat: OutputFormat.Line, - mode: AGENT_REVIEW_MODE as never, - printMode: false, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const stdout = vi - .mocked(console.log) - .mock - .calls - .map((call) => String(call[0])) - .join('\n'); - - expect(result.hadOperationalErrors).toBe(true); - expect(result.requestFailures).toBe(0); - expect(stdout).toContain('Use consistent wording'); - expect(stdout).toContain('Quality Scores:'); - expect(stderrSpy).toHaveBeenCalled(); - const stderrOutput = stderrSpy.mock.calls.map((call) => String(call[0])).join(''); - expect(stderrOutput).toContain('Review failed after'); - expect(stderrOutput).not.toContain('Completed review in'); - }); - - it('suppresses progress output when print mode is enabled', async () => { - Object.defineProperty(process.stderr, 'isTTY', { - configurable: true, - value: true, - }); - const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); - - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider: makeProvider(), - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Line, - mode: AGENT_REVIEW_MODE as never, - printMode: true, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - expect(stderrSpy).not.toHaveBeenCalled(); - }); - - it('returns machine-parseable json output without progress text contamination', async () => { - Object.defineProperty(process.stderr, 'isTTY', { - configurable: true, - value: true, - }); - const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); - - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider: makeProvider(), - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Json, - mode: AGENT_REVIEW_MODE as never, - printMode: false, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const stdout = vi - .mocked(console.log) - .mock - .calls - .map((call) => String(call[0])) - .join('\n'); - - expect(stdout).not.toContain('Reviewing'); - expect(stdout).not.toContain(' └ '); - expect(stdout).not.toContain('Completed review.'); - expect(stderrSpy).not.toHaveBeenCalled(); - }); - - it('appends a new two-line block when agent work moves to the next file', async () => { - Object.defineProperty(process.stderr, 'isTTY', { - configurable: true, - value: true, - }); - const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); - - const repo = createTempRepo(); - const fileOne = path.join(repo, 'doc.md'); - const fileTwo = path.join(repo, 'doc2.md'); - writeFileSync(fileOne, 'bad phrase\n', 'utf8'); - writeFileSync(fileTwo, 'another bad phrase\n', 'utf8'); - - const provider: LLMProvider = { - runPromptStructured() { - return Promise.resolve({ data: { reasoning: 'ok', violations: [] } }); - }, - runAgentToolLoop: async (params: Record) => { - const tools = params.tools as Record Promise }>; - await tools.read_file.execute({ path: 'doc.md' }); - await tools.lint.execute({ file: 'doc.md', ruleSource: 'packs/default/consistency.md' }); - await tools.read_file.execute({ path: 'doc2.md' }); - await tools.lint.execute({ file: 'doc2.md', ruleSource: 'packs/default/consistency.md' }); - await tools.finalize_review.execute({}); - return { usage: { inputTokens: 4, outputTokens: 2 } }; - }, - } as unknown as LLMProvider; - - await evaluateFiles([fileOne, fileTwo], { - prompts: [makePrompt()], - rulesPath: undefined, - provider, - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Line, - mode: AGENT_REVIEW_MODE as never, - printMode: false, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const stderrOutput = stderrSpy.mock.calls.map((call) => String(call[0])).join(''); - expect(stderrOutput).toContain('Reviewing doc.md for Consistency'); - expect(stderrOutput).toContain('Read 1 line from doc.md'); - expect(stderrOutput).toContain('Reviewing doc2.md for Consistency'); - expect(stderrOutput).toContain('Read 1 line from doc2.md'); - const firstFileIndex = stderrOutput.indexOf('Reviewing doc.md for Consistency'); - const secondFileIndex = stderrOutput.indexOf('Reviewing doc2.md for Consistency'); - expect(firstFileIndex).toBeGreaterThan(-1); - expect(secondFileIndex).toBeGreaterThan(firstFileIndex); - expect(stderrOutput).toMatch(/Completed review in \d+s\./); - }); - - it('renders top-level findings without explicit references at 1:1 in line output', async () => { - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider: makeTopLevelOnlyProvider(), - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Line, - mode: AGENT_REVIEW_MODE as never, - printMode: true, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const stdout = vi - .mocked(console.log) - .mock - .calls - .map((call) => String(call[0])) - .join('\n'); - - expect(stdout).toContain('Top-level without references'); - expect(stdout).toContain('1:1'); - }); - - it('prints lazy file headers for non-target referenced findings in line output', async () => { - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - const otherFile = path.join(repo, 'other.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - writeFileSync(otherFile, 'other content\n', 'utf8'); - - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider: makeCrossFileTopLevelProvider(), - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Line, - mode: AGENT_REVIEW_MODE as never, - printMode: true, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const stdout = vi - .mocked(console.log) - .mock - .calls - .map((call) => String(call[0])) - .join('\n'); - - expect(stdout).toContain('other.md'); - expect(stdout).toContain('Cross-file issue found'); - }); - - it('scores against every matched file, including clean files without findings', async () => { - const repo = createTempRepo(); - const firstFile = path.join(repo, 'doc.md'); - const secondFile = path.join(repo, 'other.md'); - writeFileSync(firstFile, 'one two three four five six seven eight nine ten\n', 'utf8'); - writeFileSync(secondFile, 'alpha beta gamma delta epsilon zeta eta theta iota kappa\n', 'utf8'); - - const provider: LLMProvider = { - runPromptStructured() { - return Promise.resolve({ - data: { - reasoning: 'detected issue', - violations: [ - { - line: 1, - quoted_text: 'one', - context_before: '', - context_after: '', - description: 'Issue found', - analysis: 'Needs cleanup.', - message: 'Fix the wording', - suggestion: 'Use clearer wording', - fix: 'clear wording', - rule_quote: 'Be precise', - checks: { - rule_supports_claim: true, - evidence_exact: true, - context_supports_violation: true, - plausible_non_violation: false, - fix_is_drop_in: true, - fix_preserves_meaning: true, - }, - check_notes: { - rule_supports_claim: 'clear', - evidence_exact: 'exact', - context_supports_violation: 'yes', - plausible_non_violation: 'none', - fix_is_drop_in: 'yes', - fix_preserves_meaning: 'yes', - }, - confidence: 0.95, - }, - ], - }, - }); - }, - runAgentToolLoop: async (params: Record) => { - const tools = params.tools as Record Promise }>; - await tools.lint.execute({ - file: 'doc.md', - ruleSource: 'packs/default/consistency.md', - }); - await tools.finalize_review.execute({}); - return { usage: { inputTokens: 3, outputTokens: 2 } }; - }, - } as unknown as LLMProvider; - - await evaluateFiles([firstFile, secondFile], { - prompts: [makePrompt()], - rulesPath: undefined, - provider, - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Line, - mode: AGENT_REVIEW_MODE as never, - printMode: true, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const stdout = vi - .mocked(console.log) - .mock - .calls - .map((call) => String(call[0])) - .join('\n'); - - expect(stdout).toContain('5.0/10'); - }); - - it('keeps canonical rule identity and warning severity aligned across json and rdjson outputs', async () => { - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider: makeProvider(), - concurrency: 1, - verbose: false, outputFormat: OutputFormat.Json, - mode: AGENT_REVIEW_MODE as never, + mode: DEFAULT_REVIEW_MODE, printMode: true, scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const jsonPayload = JSON.parse( - String(vi.mocked(console.log).mock.calls.at(-1)?.[0]) - ) as { - files?: Record }>; - }; - - const jsonIssue = - jsonPayload.files?.['doc.md']?.issues?.find( - (issue) => issue.rule === 'Default.Consistency' - ) ?? jsonPayload.files?.['doc.md']?.issues?.[0]; - - vi.mocked(console.log).mockClear(); - - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider: makeProvider(), - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.RdJson, - mode: AGENT_REVIEW_MODE as never, - printMode: true, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const rdjsonPayload = JSON.parse( - String(vi.mocked(console.log).mock.calls.at(-1)?.[0]) - ) as { - diagnostics?: Array<{ code?: { value?: string }; severity?: string }>; - }; - - const rdjsonDiagnostic = - rdjsonPayload.diagnostics?.find( - (diagnostic) => diagnostic.code?.value === 'Default.Consistency' - ) ?? rdjsonPayload.diagnostics?.[0]; - - expect(jsonIssue?.rule).toBe('Default.Consistency'); - expect(jsonIssue?.severity).toBe(Severity.WARNING); - expect(rdjsonDiagnostic?.code?.value).toBe('Default.Consistency'); - expect(rdjsonDiagnostic?.severity).toBe(Severity.WARNING); - }); - - it('keeps rdjson output machine-parseable without interactive progress writes', async () => { - Object.defineProperty(process.stderr, 'isTTY', { - configurable: true, - value: true, }); - const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); - - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider: makeProvider(), - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.RdJson, - mode: AGENT_REVIEW_MODE as never, - printMode: false, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const stdout = vi.mocked(console.log).mock.calls.map((call) => String(call[0])).join('\n'); - expect(stdout).not.toContain('Reviewing'); - expect(stdout).not.toContain(' └ '); - expect(stderrSpy).not.toHaveBeenCalled(); - }); - - it('keeps vale-json output machine-parseable without interactive progress writes', async () => { - Object.defineProperty(process.stderr, 'isTTY', { - configurable: true, - value: true, - }); - const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); - - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider: makeProvider(), - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.ValeJson, - mode: AGENT_REVIEW_MODE as never, - printMode: false, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - const stdout = vi.mocked(console.log).mock.calls.map((call) => String(call[0])).join('\n'); - expect(stdout).not.toContain('Reviewing'); - expect(stdout).not.toContain(' └ '); - expect(stderrSpy).not.toHaveBeenCalled(); - }); - - it('surfaces findings recorded before missing finalize while still reporting an operational failure', async () => { - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - const result = await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider: makeNoFinalizeProvider(), - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Json, - mode: AGENT_REVIEW_MODE as never, - printMode: true, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - expect(result.totalWarnings).toBeGreaterThan(0); - expect(result.requestFailures).toBe(0); - expect(result.hadOperationalErrors).toBe(true); - }); - - it('passes userInstructionContent into the agent system prompt', async () => { - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - let capturedSystemPrompt = ''; - const provider: LLMProvider = { - runPromptStructured() { - return Promise.resolve({ data: { reasoning: 'ok', violations: [] } }); - }, - runAgentToolLoop: async (params: Record) => { - const systemPrompt = params.systemPrompt; - capturedSystemPrompt = typeof systemPrompt === 'string' ? systemPrompt : ''; - const tools = params.tools as Record Promise }>; - await tools.finalize_review.execute({}); - return { usage: { inputTokens: 1, outputTokens: 1 } }; - }, - } as unknown as LLMProvider; - - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider, - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Json, - mode: AGENT_REVIEW_MODE as never, - printMode: true, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - userInstructionContent: 'Always enforce concise phrasing.', - } as never); - - expect(capturedSystemPrompt).toContain('User Instructions (from VECTORLINT.md):'); - expect(capturedSystemPrompt).toContain('Always enforce concise phrasing.'); - }); - - it('counts provider tool-loop failures as request failures in agent mode', async () => { - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - const provider: LLMProvider = { - runPromptStructured() { - return Promise.resolve({ data: { reasoning: 'ok', violations: [] } }); - }, - runAgentToolLoop() { - return Promise.reject(new Error('provider request failed')); - }, - } as unknown as LLMProvider; - - const result = await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider, - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Json, - mode: AGENT_REVIEW_MODE as never, - printMode: true, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - expect(result.hadOperationalErrors).toBe(true); - expect(result.requestFailures).toBe(1); - }); - - it('passes a default agent retry budget to the provider tool loop', async () => { - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - let receivedParams: Record | undefined; - const provider: LLMProvider = { - runPromptStructured() { - return Promise.resolve({ data: { reasoning: 'ok', violations: [] } }); - }, - runAgentToolLoop: async (params: Record) => { - receivedParams = params; - const tools = params.tools as Record Promise }>; - await tools.finalize_review.execute({}); - return { usage: { inputTokens: 1, outputTokens: 1 } }; - }, - } as unknown as LLMProvider; - - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider, - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Json, - mode: AGENT_REVIEW_MODE as never, - printMode: true, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - expect(receivedParams?.maxRetries).toBe(10); - }); - - it('passes configured agent retry budget to the provider tool loop', async () => { - const repo = createTempRepo(); - const file = path.join(repo, 'doc.md'); - writeFileSync(file, 'bad phrase\n', 'utf8'); - - let receivedParams: Record | undefined; - const provider: LLMProvider = { - runPromptStructured() { - return Promise.resolve({ data: { reasoning: 'ok', violations: [] } }); - }, - runAgentToolLoop: async (params: Record) => { - receivedParams = params; - const tools = params.tools as Record Promise }>; - await tools.finalize_review.execute({}); - return { usage: { inputTokens: 1, outputTokens: 1 } }; - }, - } as unknown as LLMProvider; - - await evaluateFiles([file], { - prompts: [makePrompt()], - rulesPath: undefined, - provider, - concurrency: 1, - verbose: false, - outputFormat: OutputFormat.Json, - mode: AGENT_REVIEW_MODE as never, - printMode: true, - agentMaxRetries: 4, - scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], - } as never); - - expect(receivedParams?.maxRetries).toBe(4); + // No deprecation warning and no executor invocation in standard mode. + expect(runAgentToolLoop).not.toHaveBeenCalled(); + expect(result.totalFiles).toBe(1); }); }); From a2e9f25c8935163285235736ac70ff5926d47b95 Mon Sep 17 00:00:00 2001 From: Osho Emmanuel Date: Tue, 14 Jul 2026 00:10:35 +0100 Subject: [PATCH 07/12] Append Phase 1 completion note to harness audit - Record the shifted 2026-07-13 baseline: only tsc --noEmit was failing; lint and test:run were already green, so the audit's lint and four-suite module-resolution failures are stale - Note the durable Phase 1 gates (typecheck, verify, vitest.config, docs/research lint exclusion, Node 20 typecheck/test workflows) - Point to the --mode agent deprecation and standard fallback as the precondition for Phases 2-5; record that no spec or architecture doc is superseded in this appendix --- ...0-vectorlint-harness-architecture-audit.md | 380 ++++++++++++++++++ 1 file changed, 380 insertions(+) create mode 100644 docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md diff --git a/docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md b/docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md new file mode 100644 index 00000000..51161ce0 --- /dev/null +++ b/docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md @@ -0,0 +1,380 @@ +# VectorLint Harness Architecture Audit + +Date: 2026-07-10 + +Status: Objective system audit plus product-direction audit. Request changes before shipping the current autonomous workspace-agent surface as a public contract. + +## Product Decision + +VectorLint should not keep the current autonomous workspace-agent mode. + +The retained agent-like capability should be a constrained reader executor: a model or headless agent can read the target content in sections when that is useful for context management, but it should not search the workspace, inspect arbitrary files, perform top-level workspace checks, or rewrite the rule being evaluated. + +The intended execution model is: + +- `direct`: send target content and rule in one structured review call. +- `reader`: give the executor a read-section capability over the target content only. +- `auto`: choose `direct` for normal-sized inputs and `reader` for large inputs or explicit context-management needs. + +This is an execution strategy decision, not a return to VectorLint-as-agent. + +## Product Direction Under Review + +VectorLint should become a programmable, bounded, on-page content review harness that another caller can invoke. + +The caller can be Codex, Claude Code, another coding agent, CI, a local CLI wrapper, or a future service. That caller owns exploration, cross-page reasoning, research, and context gathering. VectorLint owns the constrained review of target content against source-backed rules and returns structured findings, scores, diagnostics, and usage metadata. + +In practical terms: + +- VectorLint reviews the target page or explicitly supplied content. +- External context is caller-supplied, not discovered by VectorLint. +- Rules define review behavior, granularity, severity, and scoring constraints. +- Executors can be API models, headless local agents, or constrained reader executors, but they receive the same review request contract. +- VectorLint should not expose a model-controlled workspace tool loop as its core product surface. + +## Objective System Audit Summary + +Separate from product direction, the current codebase has several system-level liabilities: + +- Build health is weak: `npx tsc --noEmit`, `npm run lint`, and parts of `npm run test:run` fail in the current workspace. +- Runtime contracts are duplicated across hand-written TypeScript, JSON-schema objects, Zod schemas, and formatter-specific shapes. +- Standard mode and agent mode project the same underlying findings differently. +- CLI orchestration owns too many responsibilities: config, file matching, evaluation, scoring, output formatting, debug artifacts, and agent routing. +- Evidence verification and counting are inconsistent enough to affect exit behavior. +- Observability and debug paths can record full prompts, content, and outputs. +- Documentation and implementation disagree on the current agent-mode topology and tool contracts. + +## Architecture Verdict + +The current codebase contains two execution models at once: + +1. The older evaluator path: one structured model call per rule or chunk. +2. The newer autonomous workspace-agent path: VectorLint gives a model tools to read/search the workspace and call lint as a nested tool. + +The second path should not continue as-is. The useful piece is not "agent mode" broadly; it is target-aware reading for context management. The current implementation also duplicates result projection, scoring, evidence validation, output formatting, and configuration behavior from the first path. That is why the architecture feels harder to maintain: the code is not just messy; it is serving two different ownership models. + +The highest-leverage change is to define a neutral review-domain contract and make every executor implement that contract. + +## Proposed Core Contract + +The contract should be centered on review, not providers or agents. + +```ts +interface ReviewRequest { + target: ReviewTarget; + rules: ReviewRule[]; + context?: ReviewContext[]; + budget: ReviewBudget; + outputPolicy: ReviewOutputPolicy; +} + +interface ReviewExecutor { + run(request: ReviewRequest): Promise; +} + +interface ReviewResult { + findings: ReviewFinding[]; + scores: ReviewScore[]; + diagnostics: ReviewDiagnostic[]; + usage?: ReviewUsage; +} +``` + +API models, reasoning models, and headless CLI agents become adapter implementations behind this contract. The CLI becomes orchestration around request creation, result formatting, and exit behavior. + +## Major Findings + +### 1. The Current Agent Surface Is Broader Than The Decided Execution Model + +Evidence: + +- `README.md:148` presents `--mode agent` as autonomous cross-file review mode. +- `src/agent/prompt-builder.ts:43` instructs the model to perform workspace-level checks. +- `src/agent/executor.ts:670` exposes `read_file`. +- `src/agent/executor.ts:715` exposes `search_content`. + +Impact: + +VectorLint still behaves like it owns exploration. That puts it in competition with external coding agents instead of becoming a harness those agents can use. It also overshoots the decided reader-executor model, which only needs target-scoped reading for context management. + +Recommendation: + +Remove the current autonomous workspace-agent surface. Preserve only a constrained reader executor if needed: + +- It may read sections of the target content. +- It may not search the workspace. +- It may not read arbitrary files. +- It may not perform top-level workspace findings. +- It may not override source-backed rules. + +### 2. The Provider Interface Mixes Structured Review With Autonomous Tool Loops + +Evidence: + +- `src/providers/llm-provider.ts:28` requires both `runPromptStructured` and `runAgentToolLoop`. +- `src/providers/vercel-ai-provider.ts:136` implements the agent loop on the same provider. +- `src/agent/executor.ts:796` depends on the provider-level agent loop. + +Impact: + +Every future executor has to pretend to be both a structured model provider and an autonomous agent runner. That is the wrong abstraction for headless CLI support and for a constrained reader executor. + +Recommendation: + +Split the contracts: + +- `StructuredModelClient` for API structured output. +- `HeadlessReviewExecutor` for local CLI agents. +- `ReaderReviewExecutor` for target-scoped read-section execution. +- `ReviewExecutor` as the stable domain-level interface. +- `TelemetrySink` or observability decoration as an optional cross-cutting concern. + +### 3. Runtime Type Contracts Are Not Strong Enough For A Harness + +Evidence: + +- `src/providers/vercel-ai-provider.ts:119` returns `output as T`. +- `src/providers/llm-provider.ts:9` defines tool input and output as `unknown`. +- `src/agent/tools-registry.ts:21` exposes tools without typed output contracts. +- `npx tsc --noEmit` currently fails in core files. + +Impact: + +The code presents types as guarantees, but several are only assertions. A programmable harness needs reliable runtime validation because external callers and headless adapters will depend on these contracts. + +Recommendation: + +Tie structured execution to concrete Zod schemas or canonical parsers. Introduce typed tool contracts only if tools remain. Add a `typecheck` script and wire it into CI and build. + +### 4. Standard Mode And Agent Mode Project Results Differently + +Evidence: + +- `src/cli/orchestrator.ts:626` derives standard check severity from scoring. +- `src/agent/executor.ts:431` stamps agent findings with prompt severity. +- `src/agent/executor.ts:598` flattens judge criteria before recording findings. +- `src/cli/orchestrator.ts:1216` builds agent scores only for line output. +- `src/cli/orchestrator.ts:1237` emits structured output without agent scores or diagnostics. + +Impact: + +The same underlying review can produce different severity, rule identity, score data, JSON shape, and exit behavior depending on mode. That makes VectorLint hard to trust as a machine-facing gate. + +Recommendation: + +Extract one shared result projection pipeline: + +```text +PromptEvaluationResult + -> verified findings + -> rule and criterion identity + -> severity and score + -> diagnostics + -> output formatter +``` + +Every executor should return into this pipeline. + +### 5. The On-Page Boundary Is Not Enforced + +Evidence: + +- `src/agent/executor.ts:585` lets `lint` resolve any file inside `workspaceRoot`. +- `src/agent/executor.ts:670` lets `read_file` read any workspace file. +- `src/agent/executor.ts:715` lets `search_content` scan workspace content. +- `src/agent/executor.ts:134` allows model-supplied `reviewInstruction`. +- `src/agent/executor.ts:583` replaces the source-backed prompt body with the model-supplied override. + +Impact: + +Prompt-injected page content can indirectly steer the model to read non-target files or rewrite the rule being evaluated. That breaks deterministic review semantics and creates privacy and security risk. + +Recommendation: + +For the new harness, enforce a target/context allowlist: + +- Target content is always in scope. +- Caller-supplied context is in scope. +- Workspace reads are out of scope unless the caller explicitly includes those files as context. +- Reader execution can page through target content only. +- Rule bodies are source-backed and caller-authored, not model-authored. + +### 6. Evidence Handling Can Misreport Findings + +Evidence: + +- Standard mode skips unverifiable quotes but can still count original surfaced violation totals in some paths. +- `src/agent/executor.ts:415` attempts to locate evidence. +- `src/agent/executor.ts:426` falls back to the model-provided line when location fails. + +Impact: + +Standard mode can fail a run without printing the issue that caused the failure. Agent mode can surface hallucinated evidence as if it were verified. + +Recommendation: + +Use one evidence verifier. Count only emitted, verified findings. If evidence cannot be located, return a diagnostic and either skip the finding or mark the run operationally failed. + +### 7. Cost And Work Are Not Bounded Enough + +Evidence: + +- `src/providers/vercel-ai-provider.ts:161` defaults agent loops to 1000 steps. +- `src/agent/executor.ts:577` allows repeated nested `lint` calls. +- `src/evaluators/base-evaluator.ts:213` runs one model call per chunk. +- `src/agent/executor.ts:680` and `src/agent/executor.ts:715` glob before applying result caps. + +Impact: + +A single review can multiply cost through agent steps, nested lint calls, rules, chunks, and workspace search. Even a constrained reader executor will be slower than a direct call, so `auto` must have clear thresholds and budgets. + +Recommendation: + +Add explicit budgets: + +- Max target bytes. +- Max caller context bytes. +- Max chunks per rule. +- Max model calls per review. +- Max findings per rule. +- Max wall-clock duration. +- Max headless executor retries. + +### 8. Documentation Still Points At The Old Product + +Evidence: + +- `README.md:148` documents autonomous workspace-agent mode. +- `docs/specs/2026-03-17-agentic-capabilities-design.md:24` frames the roadmap around cross-document agent mode. +- The same spec describes implementation details that are not true now: one agent per rule, final `AgentFindingSchema`, AbortSignal propagation, ripgrep JSON, and Vale JSON omission of agent findings. + +Impact: + +Future contributors will optimize toward the wrong architecture and trust contracts that the code does not implement. + +Recommendation: + +Mark the agentic spec superseded. Replace it with a harness architecture spec that defines the review request, context boundary, executor interface, output contract, and migration plan. + +### 9. Secrets And Sensitive Content Need Better Handling + +Evidence: + +- `src/config/global-config.ts:13` says the global config stores API keys. +- `src/config/global-config.ts:76` creates the config directory with default permissions. +- `src/config/global-config.ts:81` writes the config file with default permissions. +- `src/observability/langfuse-observability.ts:66` records inputs. +- `src/observability/langfuse-observability.ts:67` records outputs. +- `src/providers/vercel-ai-provider.ts:59` can log full prompts and content. + +Impact: + +Provider keys, unreleased docs, caller context, and model outputs can leak through filesystem permissions, telemetry, debug logs, or persisted artifacts. + +Recommendation: + +Use private file modes for config and review artifacts. Make payload telemetry a separate opt-in from metadata telemetry. Add redaction or safe debug modes. + +## Verification Results + +Commands run from `/Users/klinsmann/Projects/TinyRocketLabs/vectorlint`: + +```bash +npm run lint +npm run test:run +npx tsc --noEmit +``` + +Results: + +- `npm run lint` failed before linting because typed ESLint rules were applied to a `.cjs` research script without parser type information. +- `npm run test:run` passed most tests but failed 4 suites during module resolution for `ora` and `@langfuse/otel`. +- `npx tsc --noEmit` failed with contract errors in agent, orchestrator, observability, logging, and provider modules. + +## Recommended Refactor Sequence + +### Phase 1: Stop The Bleeding + +1. Remove or hide the current autonomous workspace-agent mode. +2. Fix `tsc --noEmit`. +3. Fix test module resolution. +4. Fix lint configuration for non-TypeScript research scripts. +5. Add `typecheck` to CI and build validation. + +### Phase 2: Define The Harness Contract + +1. Create a neutral review-domain module. +2. Define `ReviewRequest`, `ReviewRule`, `ReviewContext`, `ReviewBudget`, `ReviewFinding`, `ReviewScore`, and `ReviewDiagnostic`. +3. Define target/context boundary rules. +4. Define structured output shape once. +5. Define execution strategy: `direct`, `reader`, and `auto`. + +### Phase 3: Share Result Projection + +1. Extract evidence location and verification. +2. Extract filtering. +3. Extract severity and scoring. +4. Extract output formatter inputs. +5. Make standard mode and any executor mode use the same projection path. + +### Phase 4: Replace The Agent Loop With Executors + +1. Keep API model execution as direct execution. +2. Add target-scoped reader execution as another executor. +3. Add headless CLI execution behind the same review contract if still useful. +4. Pass the same review request to each executor. +5. Remove model-controlled rule overrides. +6. Remove workspace read/search tools from core review. + +### Phase 5: Update Documentation + +1. Mark the agentic capabilities spec superseded. +2. Write a new harness architecture spec. +3. Update README and CLI docs. +4. Document migration from autonomous workspace-agent mode to direct/reader/auto execution. + +## Suggested Target Architecture + +```text +CLI / API / External Agent + -> ReviewRequestBuilder + -> ReviewExecutor + -> ApiModelExecutor + -> ReaderExecutor + -> HeadlessCliExecutor + -> ResultProjection + -> EvidenceVerifier + -> ViolationFilter + -> ScoreCalculator + -> Diagnostics + -> OutputFormatter + -> line + -> json + -> rdjson + -> vale-json +``` + +The important inversion is that VectorLint no longer asks, "What tools should this agent use?" It asks, "Given this target, these rules, this context, this execution strategy, and this budget, what findings can be verified?" + +## Final Recommendation + +The decided direction is cleaner than the current architecture. The current pain is not a sign that VectorLint is too complex; it is a sign that the codebase is carrying two ownership models and several duplicated runtime contracts. + +Make VectorLint the harness. Let external agents be agents. Keep reader execution only as a bounded way to manage target-content context. + +--- + +## Appendix: Phase 1 — Completed (2026-07-13) + +Phase 1 of the recommended refactor sequence ("Stop The Bleeding") landed on branch `codex/ci/harness-stop-the-bleeding`. It shifts the verification baseline recorded in "Verification Results" above and addresses Finding 1 at the CLI boundary. + +**Shifted baseline.** The 2026-07-13 pre-work capture differed from the original verification results: `npm run lint` and `npm run test:run` already passed, so the earlier lint failure and the four `ora` / `@langfuse/otel` module-resolution failures were stale. Only `npx tsc --noEmit` was still failing. Phase 1 cleared those remaining type errors (narrowed at boundaries; no strict compiler options relaxed) and added durable gates so the baseline cannot silently regress: + +- `npm run typecheck` (`tsc --noEmit`) and `npm run verify` (typecheck + lint + test:run) scripts. +- `vitest.config.ts` for stable test module resolution. +- `docs/research/**` excluded from linting. +- `.github/workflows/typecheck.yml` and `.github/workflows/test.yml` aligned to Node 20. + +**Current state.** `npm run verify`, `npm run build`, and built-CLI standard review smoke runs are green. Per Finding 1, the autonomous workspace-agent surface is deprecated at the CLI boundary: `--mode agent` emits a deprecation warning and falls back to standard evaluation. `src/agent/*` and the agent-mode helpers are retained as compile-only quarantine, unreachable from the CLI, pending removal in Phase 4. The deprecation notice and pointer to this audit live in the README `## Agent Mode` section and the `--mode` help text. + +This appendix records completion only. Findings 2–9 and the proposed core contract remain owned by Phases 2–5; no spec or architecture document is superseded here. From 888f0271e30b0e0e1fa5d2773ca89f94cc415d58 Mon Sep 17 00:00:00 2001 From: Osho Emmanuel Date: Tue, 14 Jul 2026 00:22:49 +0100 Subject: [PATCH 08/12] Add root .vectorlint.ini for reproducible smoke validation - Commit a repository-native .vectorlint.ini using the bundled VectorLint preset (verbatim `vectorlint init` template). - The required validation commands `npm start -- README.md --output line` and `npm start -- README.md --mode agent` previously failed from the committed branch with "Missing configuration file"; they had relied on an untracked, deleted VECTORLINT.md. - Both smoke commands now run from a clean checkout with no untracked setup files; --mode agent still warns and falls back to standard mode. Refs: .agent-runs/2026-07-13-223741-harness-refactor/reports/01-stop-the-bleeding.md --- .vectorlint.ini | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .vectorlint.ini diff --git a/.vectorlint.ini b/.vectorlint.ini new file mode 100644 index 00000000..d5b0d845 --- /dev/null +++ b/.vectorlint.ini @@ -0,0 +1,9 @@ +# VectorLint Configuration +# Global settings +RulesPath= +Concurrency=4 +DefaultSeverity=warning + +# Default rules for all markdown files +[**/*.md] +RunRules=VectorLint From 99f73c22ae728cb3dfb904dd0208b881322ac09b Mon Sep 17 00:00:00 2001 From: Osho Emmanuel Date: Tue, 14 Jul 2026 00:56:45 +0100 Subject: [PATCH 09/12] Add neutral review-domain contract module - Introduce src/review/ with typed + Zod-validated ReviewRequest/Result contracts, boundary helpers, budget defaults/enforcement, model-call selection, ReviewExecutor interface, and a PromptFile request builder - Every external shape has a paired strict Zod schema; legacy scoring-mode, rubric, and model-authored rule-override fields are rejected - modelCall is single | agent | auto; chooseModelCall() resolves auto - On-page boundary (target + caller context only) via buildScope/isInScope with pure lexical URI normalization; no filesystem reads - BudgetExceededError extends the repository VectorlintError base - Purely additive: no changes outside src/review/ and tests/review/ - tests/review/ covers surface, schemas, budget, boundary, results, model-call selection, and the request builder (46 tests) --- src/review/README.md | 42 ++++++ src/review/boundary.ts | 67 ++++++++++ src/review/budget.ts | 83 ++++++++++++ src/review/executor.ts | 53 ++++++++ src/review/index.ts | 15 +++ src/review/request-builder.ts | 88 ++++++++++++ src/review/schemas.ts | 135 +++++++++++++++++++ src/review/types.ts | 192 +++++++++++++++++++++++++++ tests/review/boundary.test.ts | 39 ++++++ tests/review/budget.test.ts | 67 ++++++++++ tests/review/executor.test.ts | 46 +++++++ tests/review/module-surface.test.ts | 34 +++++ tests/review/request-builder.test.ts | 97 ++++++++++++++ tests/review/result.test.ts | 95 +++++++++++++ tests/review/types.test.ts | 74 +++++++++++ 15 files changed, 1127 insertions(+) create mode 100644 src/review/README.md create mode 100644 src/review/boundary.ts create mode 100644 src/review/budget.ts create mode 100644 src/review/executor.ts create mode 100644 src/review/index.ts create mode 100644 src/review/request-builder.ts create mode 100644 src/review/schemas.ts create mode 100644 src/review/types.ts create mode 100644 tests/review/boundary.test.ts create mode 100644 tests/review/budget.test.ts create mode 100644 tests/review/executor.test.ts create mode 100644 tests/review/module-surface.test.ts create mode 100644 tests/review/request-builder.test.ts create mode 100644 tests/review/result.test.ts create mode 100644 tests/review/types.test.ts diff --git a/src/review/README.md b/src/review/README.md new file mode 100644 index 00000000..bc2a120b --- /dev/null +++ b/src/review/README.md @@ -0,0 +1,42 @@ +# `src/review/` — Review-Domain Contract + +The neutral, implementation-neutral contract every executor programs against. +A caller builds a `ReviewRequest`; an executor returns a `ReviewResult`. + +## What lives here + +- `types.ts` — all review-domain interfaces (`ReviewTarget`, `ReviewRule`, + `ReviewContext`, `ReviewBudget`, `ReviewFinding`, `ReviewScore`, + `ReviewDiagnostic`, `ReviewResult`, `ReviewRequest`, `ReviewModelCall`). +- `schemas.ts` — Zod schemas mirroring every external shape (boundary + validation). All schemas are strict; legacy scoring-mode, rubric, and + model-authored rule-override fields are rejected. +- `budget.ts` — `DEFAULT_REVIEW_BUDGET`, `REVIEW_BUDGET_SCHEMA`, + `enforceBudget()`, and `BudgetExceededError` (extends `VectorlintError`). +- `boundary.ts` — `buildScope()` / `isInScope()` enforcing the on-page boundary. +- `executor.ts` — `ReviewExecutor` interface, `REVIEW_MODEL_CALLS` + (`single | agent | auto`), `chooseModelCall()`. +- `request-builder.ts` — `buildReviewRequest()` bridging `PromptFile` to + `ReviewRequest`. + +## On-page boundary (audit Finding #5) + +- Target content is always in scope. +- Caller-supplied context is in scope. +- Workspace reads are out of scope unless the caller includes them as context. +- Agent model calls page through target content only. +- Rule bodies are source-backed and caller-authored, never model-authored. + +## Execution strategy + +`modelCall: single | agent | auto` selects how the reviewer model is invoked, +not how rules are scored. `single` sends target + rule in one structured call; +`agent` gives the executor a target-scoped read-section capability; `auto` +picks `single` for normal-sized inputs and `agent` for large ones. + +## Wiring status + +Phase 2 (this module) is purely additive and is **not** wired into the CLI. +Phase 3 emits `ReviewResult` via shared finding processing; Phase 4 implements +executors behind `ReviewExecutor` and removes the old agent loop. See +`docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md`. diff --git a/src/review/boundary.ts b/src/review/boundary.ts new file mode 100644 index 00000000..8dc5bb0c --- /dev/null +++ b/src/review/boundary.ts @@ -0,0 +1,67 @@ +import type { ReviewScope } from './types'; + +const FILE_URI_RE = /^file:\/\/(?[^/]*)(?\/.*)?$/; + +/** + * Lexically resolves '.' and '..' segments in a path string. Pure: performs no + * filesystem reads. The normalization philosophy is adapted from + * src/agent/path-utils.ts (resolveWithinRoot), which is removed in Phase 4; + * copied here so src/review/ has no agent import and no filesystem access. + * + * Symlink canonicalization is intentionally NOT performed inside the contract: + * callers are responsible for canonicalizing real files into target/context + * URIs before building a ReviewRequest. + */ +function resolveDotSegments(input: string): string { + const segments: string[] = []; + for (const segment of input.split('/')) { + if (segment === '' || segment === '.') continue; + if (segment === '..') { + segments.pop(); + continue; + } + segments.push(segment); + } + return (input.startsWith('/') ? '/' : '') + segments.join('/'); +} + +/** + * Normalizes a review URI by resolving '.'/'..' segments in its path. file:// + * URIs are normalized structurally; other (virtual, in-memory) URIs are + * returned as-is after dot resolution of their path-like suffix. + */ +export function normalizeReviewUri(uri: string): string { + const match = FILE_URI_RE.exec(uri); + if (match?.groups) { + const authority = match.groups['authority'] ?? ''; + const rawPath = match.groups['path'] ?? '/'; + return `file://${authority}${resolveDotSegments(rawPath)}`; + } + return uri; +} + +/** + * Builds the on-page boundary scope (audit Finding #5) from the target URI and + * any caller-supplied context URIs. Only these URIs are in scope; arbitrary + * workspace files are out of scope unless the caller explicitly includes them. + */ +export function buildScope(params: { + targetUri: string; + contextUris?: readonly string[]; +}): ReviewScope { + const allowedUris = new Set(); + allowedUris.add(normalizeReviewUri(params.targetUri)); + for (const uri of params.contextUris ?? []) { + allowedUris.add(normalizeReviewUri(uri)); + } + return { allowedUris }; +} + +/** + * Returns true iff `uri` (after normalization) is the target or caller-supplied + * context. Path-traversal segments are resolved before comparison, so a + * traversal-style path cannot masquerade as an in-scope URI. + */ +export function isInScope(scope: ReviewScope, uri: string): boolean { + return scope.allowedUris.has(normalizeReviewUri(uri)); +} diff --git a/src/review/budget.ts b/src/review/budget.ts new file mode 100644 index 00000000..e7d4cd8b --- /dev/null +++ b/src/review/budget.ts @@ -0,0 +1,83 @@ +import { z } from 'zod'; +import type { ReviewBudget } from './types'; +import { VectorlintError } from '../errors'; + +/** + * Sensible default bounds for a single review (audit Finding #7). Frozen so a + * shared default object cannot be accidentally mutated by a caller. + * + * Note: there is intentionally no finding cap (`maxFindingsPerRule`) — + * VectorLint reports every verified issue it finds — and no headless retry + * budget (`maxHeadlessRetries`) because the decided architecture has no + * headless executor. + */ +export const DEFAULT_REVIEW_BUDGET: Readonly = Object.freeze({ + maxTargetBytes: 1_000_000, + maxCallerContextBytes: 500_000, + maxChunksPerRule: 20, + maxModelCallsPerReview: 50, + maxWallClockMs: 5 * 60_000, +}); + +/** Zod schema mirroring {@link ReviewBudget}, applying defaults for omitted fields. */ +export const REVIEW_BUDGET_SCHEMA = z + .object({ + maxTargetBytes: z.number().int().positive().default(DEFAULT_REVIEW_BUDGET.maxTargetBytes), + maxCallerContextBytes: z + .number() + .int() + .positive() + .default(DEFAULT_REVIEW_BUDGET.maxCallerContextBytes), + maxChunksPerRule: z.number().int().positive().default(DEFAULT_REVIEW_BUDGET.maxChunksPerRule), + maxModelCallsPerReview: z + .number() + .int() + .positive() + .default(DEFAULT_REVIEW_BUDGET.maxModelCallsPerReview), + maxWallClockMs: z.number().int().positive().default(DEFAULT_REVIEW_BUDGET.maxWallClockMs), + }) + .strict(); + +/** Current resource usage, checked against a {@link ReviewBudget}. */ +export interface BudgetUsage { + modelCalls: number; + elapsedMs: number; +} + +/** + * Thrown by {@link enforceBudget} when current usage violates a hard limit. + * Extends the repository's canonical error base (VectorlintError). + */ +export class BudgetExceededError extends VectorlintError { + constructor( + message: string, + public readonly limit: keyof ReviewBudget, + public readonly actual: number, + ) { + super(message, 'BUDGET_EXCEEDED'); + this.name = 'BudgetExceededError'; + } +} + +/** + * Throws {@link BudgetExceededError} if current usage violates a hard limit. + * Executors call this before/after each model call and on timeout checks. + * Only runtime counters (model calls, wall clock) are enforced here; size + * limits (target/context bytes, chunks) are enforced at request-build time. + */ +export function enforceBudget(budget: ReviewBudget, usage: BudgetUsage): void { + if (usage.modelCalls > budget.maxModelCallsPerReview) { + throw new BudgetExceededError( + `model calls (${usage.modelCalls}) exceed maxModelCallsPerReview (${budget.maxModelCallsPerReview})`, + 'maxModelCallsPerReview', + usage.modelCalls, + ); + } + if (usage.elapsedMs > budget.maxWallClockMs) { + throw new BudgetExceededError( + `elapsed time (${usage.elapsedMs}ms) exceeds maxWallClockMs (${budget.maxWallClockMs}ms)`, + 'maxWallClockMs', + usage.elapsedMs, + ); + } +} diff --git a/src/review/executor.ts b/src/review/executor.ts new file mode 100644 index 00000000..2eef9011 --- /dev/null +++ b/src/review/executor.ts @@ -0,0 +1,53 @@ +import type { ReviewModelCall, ReviewRequest, ReviewResult } from './types'; + +/** + * Above this target byte size (roughly the existing chunking threshold in + * bytes), `auto` model-call selection prefers the agent executor so the + * reviewer can page through target content for context management. + */ +export const AGENT_MODEL_CALL_BYTE_THRESHOLD = 600_000; + +/** + * The single source of truth for reviewer model-call strategies. Kept in sync + * with the {@link ReviewModelCall} union via `satisfies`. + */ +export const REVIEW_MODEL_CALLS = ['single', 'agent', 'auto'] as const satisfies readonly ReviewModelCall[]; + +/** + * The stable domain-level interface every executor implements. Single and + * agent executors are implementations behind this contract (audit Finding #2). + * A headless/autonomous workspace executor is explicitly NOT part of this + * contract. + */ +export interface ReviewExecutor { + run(request: ReviewRequest): Promise; +} + +/** + * Agent-call capability: a bounded way to page through target content for + * context management. NOT a workspace tool (audit Product Decision). An agent + * executor receives this capability scoped to the target only. + */ +export interface ReviewTargetReadCapability { + /** Read a 1-based [startLine, endLine] window of the target content. */ + readTargetSection( + startLine: number, + endLine: number, + ): Promise<{ startLine: number; endLine: number; content: string }>; +} + +/** + * Resolves 'auto' to 'single' or 'agent'. Single for normal-sized inputs; + * agent for large inputs or multi-rule runs that benefit from paging. + */ +export function chooseModelCall( + modelCall: ReviewModelCall, + signal: { targetBytes: number; rules: number }, +): 'single' | 'agent' { + if (modelCall === 'single') return 'single'; + if (modelCall === 'agent') return 'agent'; + if (signal.targetBytes > AGENT_MODEL_CALL_BYTE_THRESHOLD || signal.rules > 5) { + return 'agent'; + } + return 'single'; +} diff --git a/src/review/index.ts b/src/review/index.ts new file mode 100644 index 00000000..c44dbe66 --- /dev/null +++ b/src/review/index.ts @@ -0,0 +1,15 @@ +/** + * Neutral review-domain contract for the VectorLint harness. + * + * Everything an executor needs to review target content against + * source-backed rules flows through this module. The CLI, headless adapters, + * and external callers all build a ReviewRequest and receive a ReviewResult. + * + * See docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md. + */ +export * from './types'; +export * from './schemas'; +export * from './budget'; +export * from './boundary'; +export * from './executor'; +export * from './request-builder'; diff --git a/src/review/request-builder.ts b/src/review/request-builder.ts new file mode 100644 index 00000000..72500596 --- /dev/null +++ b/src/review/request-builder.ts @@ -0,0 +1,88 @@ +import type { PromptFile } from '../schemas/prompt-schemas'; +import type { + ReviewBudget, + ReviewContext, + ReviewModelCall, + ReviewOutputPolicy, + ReviewRequest, + ReviewRule, + ReviewTarget, +} from './types'; +import { DEFAULT_REVIEW_BUDGET } from './budget'; +import { ValidationError } from '../errors'; + +/** Default output policy: include usage metadata, never record payloads. */ +export const DEFAULT_REVIEW_OUTPUT_POLICY: Readonly = Object.freeze({ + includeUsage: true, + recordPayloadTelemetry: false, +}); + +/** Optional overrides applied by {@link buildReviewRequest}. */ +export interface ReviewRequestBuilderConfig { + budget?: ReviewBudget; + outputPolicy?: ReviewOutputPolicy; + modelCall?: ReviewModelCall; +} + +export interface BuildReviewRequestParams { + target: ReviewTarget; + /** Existing prompt files to convert into source-backed ReviewRules. */ + prompts: readonly PromptFile[]; + /** Caller-supplied, in-scope context, passed through unchanged. */ + context?: ReviewContext[]; + config?: ReviewRequestBuilderConfig; +} + +/** Builds the stable ReviewRule id formatted Pack.Rule (mirrors src/agent/rule-id.ts). */ +function toReviewRuleId(prompt: PromptFile): string { + const pack = prompt.pack || 'Default'; + return `${pack}.${prompt.meta.id}`; +} + +/** + * Converts a single PromptFile into a source-backed ReviewRule, mapping only + * fields that belong in the neutral contract. Legacy `meta.type`, `evaluator`, + * `criteria`, and judge/rubric fields are intentionally NOT copied. + */ +function toReviewRule(prompt: PromptFile): ReviewRule { + const rule: ReviewRule = { + id: toReviewRuleId(prompt), + source: prompt.fullPath, + body: prompt.body, + }; + if (prompt.meta.name !== undefined) { + rule.name = prompt.meta.name; + } + // Severity is a string enum whose values ('error' | 'warning') match + // ReviewSeverity exactly, so it maps cleanly without transformation. + if (prompt.meta.severity !== undefined) { + rule.severity = prompt.meta.severity; + } + return rule; +} + +/** + * Builds a {@link ReviewRequest} from existing {@link PromptFile}s plus a + * target. This is the conservative bridge Phase 4 uses to convert the current + * prompt pipeline into the review contract without changing how prompts load. + * Throws a {@link ValidationError} when no prompts are supplied. + */ +export function buildReviewRequest(params: BuildReviewRequestParams): ReviewRequest { + const { target, prompts, context } = params; + if (prompts.length === 0) { + throw new ValidationError('buildReviewRequest requires at least one prompt.'); + } + + const config = params.config; + const request: ReviewRequest = { + target, + rules: prompts.map(toReviewRule), + budget: config?.budget ?? DEFAULT_REVIEW_BUDGET, + outputPolicy: config?.outputPolicy ?? DEFAULT_REVIEW_OUTPUT_POLICY, + modelCall: config?.modelCall ?? 'auto', + }; + if (context !== undefined) { + request.context = context; + } + return request; +} diff --git a/src/review/schemas.ts b/src/review/schemas.ts new file mode 100644 index 00000000..a1132cd3 --- /dev/null +++ b/src/review/schemas.ts @@ -0,0 +1,135 @@ +import { z } from 'zod'; +import { REVIEW_BUDGET_SCHEMA } from './budget'; + +/** + * Zod schemas mirroring src/review/types.ts (boundary validation). + * + * Every external review-domain shape has a paired schema here so callers and + * external adapters can validate untrusted input at the system boundary + * (audit Finding #3). Schemas are intentionally strict: unknown keys are + * rejected so legacy scoring-mode, rubric, and model-authored rule-override + * fields cannot leak into the new contract. + */ + +export const REVIEW_SEVERITY_SCHEMA = z.enum(['error', 'warning']); + +export const REVIEW_VIOLATION_CONDITION_SCHEMA = z + .object({ + id: z.string().min(1), + description: z.string().min(1), + }) + .strict(); + +export const REVIEW_TARGET_SCHEMA = z + .object({ + uri: z.string().min(1), + content: z.string(), + contentType: z.string().min(1), + byteLength: z.number().int().nonnegative().optional(), + }) + .strict(); + +export const REVIEW_RULE_SCHEMA = z + .object({ + id: z.string().min(1), + source: z.string().min(1), + body: z.string(), + name: z.string().optional(), + severity: REVIEW_SEVERITY_SCHEMA.default('warning'), + violationConditions: z.array(REVIEW_VIOLATION_CONDITION_SCHEMA).optional(), + }) + .strict(); + +export const REVIEW_CONTEXT_SCHEMA = z + .object({ + label: z.string().min(1), + content: z.string(), + relation: z.string().optional(), + uri: z.string().optional(), + }) + .strict(); + +export const REVIEW_OUTPUT_POLICY_SCHEMA = z + .object({ + includeUsage: z.boolean(), + recordPayloadTelemetry: z.boolean(), + }) + .strict(); + +export const REVIEW_DIAGNOSTIC_LEVEL_SCHEMA = z.enum(['info', 'warn', 'error']); + +export const REVIEW_SCORE_COMPONENT_SCHEMA = z + .object({ + id: z.string().min(1), + scoreText: z.string(), + score: z.number(), + weight: z.number().optional(), + }) + .strict(); + +export const REVIEW_SCORE_SCHEMA = z + .object({ + ruleId: z.string().min(1), + score: z.number(), + scoreText: z.string(), + severity: REVIEW_SEVERITY_SCHEMA, + findingCount: z.number().int().nonnegative().optional(), + components: z.array(REVIEW_SCORE_COMPONENT_SCHEMA).optional(), + }) + .strict(); + +export const REVIEW_FINDING_SCHEMA = z + .object({ + ruleId: z.string().min(1), + ruleSource: z.string().min(1), + severity: REVIEW_SEVERITY_SCHEMA, + message: z.string(), + line: z.number().int().positive(), + column: z.number().int().positive(), + match: z.string(), + analysis: z.string().optional(), + suggestion: z.string().optional(), + fix: z.string().optional(), + }) + .strict(); + +export const REVIEW_DIAGNOSTIC_SCHEMA = z + .object({ + level: REVIEW_DIAGNOSTIC_LEVEL_SCHEMA, + code: z.string().min(1), + message: z.string(), + ruleId: z.string().optional(), + context: z.record(z.string(), z.unknown()).optional(), + }) + .strict(); + +export const REVIEW_USAGE_SCHEMA = z + .object({ + inputTokens: z.number().int().nonnegative().optional(), + outputTokens: z.number().int().nonnegative().optional(), + modelCalls: z.number().int().nonnegative(), + costUsd: z.number().nonnegative().optional(), + wallClockMs: z.number().int().nonnegative().optional(), + }) + .strict(); + +export const REVIEW_RESULT_SCHEMA = z + .object({ + findings: z.array(REVIEW_FINDING_SCHEMA), + scores: z.array(REVIEW_SCORE_SCHEMA), + diagnostics: z.array(REVIEW_DIAGNOSTIC_SCHEMA), + usage: REVIEW_USAGE_SCHEMA.optional(), + hadOperationalErrors: z.boolean().optional(), + }) + .strict(); + +export const REVIEW_REQUEST_SCHEMA = z + .object({ + target: REVIEW_TARGET_SCHEMA, + rules: z.array(REVIEW_RULE_SCHEMA).min(1), + context: z.array(REVIEW_CONTEXT_SCHEMA).optional(), + budget: REVIEW_BUDGET_SCHEMA, + outputPolicy: REVIEW_OUTPUT_POLICY_SCHEMA, + modelCall: z.enum(['single', 'agent', 'auto']), + }) + .strict(); diff --git a/src/review/types.ts b/src/review/types.ts new file mode 100644 index 00000000..97e22039 --- /dev/null +++ b/src/review/types.ts @@ -0,0 +1,192 @@ +/** + * Neutral review-domain contract types for the VectorLint harness. + * + * Everything an executor needs to review target content against + * source-backed rules flows through this module. Callers build a + * {@link ReviewRequest} and executors return a {@link ReviewResult}. + * + * See docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md. + * + * This module is implementation-neutral: it deliberately exposes no legacy + * scoring-mode, rubric, or model-authored rule-override surface. `modelCall` + * selects how the reviewer model is invoked, not how rules are scored. + */ + +/** Finding/Rule severity. */ +export type ReviewSeverity = 'error' | 'warning'; + +/** Objective Via Negativa condition that counts as a violation when present. */ +export interface ReviewViolationCondition { + id: string; + description: string; +} + +/** + * Target content under review. The on-page boundary (audit Finding #5) is + * enforced against this: executors may only read sections of `content`. + */ +export interface ReviewTarget { + /** Stable absolute URI (file:// or virtual scheme for in-memory content). */ + uri: string; + /** Full target content. May be paged by an agent executor. */ + content: string; + /** MIME-ish content type, e.g. 'text/markdown'. */ + contentType: string; + /** Optional byte length hint; computed from content if absent. */ + byteLength?: number; +} + +/** + * A source-backed, caller-authored rule. Model-authored rule overrides are + * explicitly disallowed (audit Finding #1, #5). + */ +export interface ReviewRule { + /** Stable id, formatted Pack.Rule[.Criterion] in output. */ + id: string; + /** Canonical source path (where the rule body came from). */ + source: string; + /** The rule prompt body. Source-backed; never model-authored. */ + body: string; + /** Human-readable name. */ + name?: string; + /** 'error' | 'warning'. Defaults to 'warning'. */ + severity?: ReviewSeverity; + /** Optional structured Via Negativa violation conditions. */ + violationConditions?: ReviewViolationCondition[]; +} + +/** + * Caller-supplied context. Explicitly in scope (audit boundary rule): + * VectorLint does NOT discover workspace files as context on its own; the + * caller owns exploration and context gathering. + */ +export interface ReviewContext { + /** Short label used in diagnostics/tracing. */ + label: string; + /** The context content itself. */ + content: string; + /** How this context relates to the target, e.g. 'reference' | 'glossary'. */ + relation?: string; + /** Optional source URI for provenance. */ + uri?: string; +} + +/** + * The on-page boundary scope: the normalized URIs an executor may read. + * Built by {@link buildScope} (see boundary.ts) and checked by + * {@link isInScope}. + */ +export interface ReviewScope { + /** Normalized absolute URIs that are in scope (target + caller context). */ + readonly allowedUris: ReadonlySet; +} + +/** + * Hard bounds on a single review (audit Finding #7). These limit work, not + * output. Executors MUST check these and surface a ReviewDiagnostic (or fail + * the run) when exceeded. + */ +export interface ReviewBudget { + maxTargetBytes: number; + maxCallerContextBytes: number; + maxChunksPerRule: number; + maxModelCallsPerReview: number; + /** Maximum elapsed review time in milliseconds. This is the run timeout. */ + maxWallClockMs: number; +} + +/** + * Controls optional output/telemetry behavior. Diagnostics are always part of + * ReviewResult; this policy does not hide operational warnings. Audit + * Finding #9: payload telemetry must be a separate opt-in from metadata + * telemetry. + */ +export interface ReviewOutputPolicy { + /** Include usage/cost in the result. */ + includeUsage: boolean; + /** Opt-in: record prompt/content payloads to telemetry. Default false. */ + recordPayloadTelemetry: boolean; +} + +/** A verified finding anchored in the target content. */ +export interface ReviewFinding { + ruleId: string; + ruleSource: string; + severity: ReviewSeverity; + message: string; + /** 1-based line in the target content. */ + line: number; + /** 1-based column. */ + column: number; + /** Verified anchored text. Unverified finding evidence is a diagnostic. */ + match: string; + analysis?: string; + suggestion?: string; + fix?: string; +} + +/** A per-rule score produced through shared finding processing. */ +export interface ReviewScore { + ruleId: string; + score: number; + /** Human-readable score, e.g. "8.0/10". */ + scoreText: string; + severity: ReviewSeverity; + /** Count of verified findings that contributed to this score, if applicable. */ + findingCount?: number; + components?: ReviewScoreComponent[]; +} + +export interface ReviewScoreComponent { + id: string; + scoreText: string; + score: number; + weight?: number; +} + +export type ReviewDiagnosticLevel = 'info' | 'warn' | 'error'; + +/** An operational or finding-processing note. Always part of ReviewResult. */ +export interface ReviewDiagnostic { + level: ReviewDiagnosticLevel; + /** Stable machine code, e.g. 'finding-evidence-not-locatable'. */ + code: string; + message: string; + ruleId?: string; + /** Extra machine-readable detail (model-call count, evidence preview, ...). */ + context?: Record; +} + +/** Aggregated resource usage for a review run. */ +export interface ReviewUsage { + inputTokens?: number; + outputTokens?: number; + modelCalls: number; + costUsd?: number; + wallClockMs?: number; +} + +/** The output from any executor, produced through shared finding processing. */ +export interface ReviewResult { + findings: ReviewFinding[]; + scores: ReviewScore[]; + diagnostics: ReviewDiagnostic[]; + usage?: ReviewUsage; + /** True if the run hit an operational error but still returned partial results. */ + hadOperationalErrors?: boolean; +} + +/** How to call the reviewer model. 'auto' resolves via chooseModelCall(). */ +export type ReviewModelCall = 'single' | 'agent' | 'auto'; + +/** The complete input to any executor. */ +export interface ReviewRequest { + target: ReviewTarget; + rules: ReviewRule[]; + /** Caller-supplied, in-scope context (never workspace-discovered). */ + context?: ReviewContext[]; + budget: ReviewBudget; + outputPolicy: ReviewOutputPolicy; + /** Reviewer model call shape. 'auto' resolves via chooseModelCall(). */ + modelCall: ReviewModelCall; +} diff --git a/tests/review/boundary.test.ts b/tests/review/boundary.test.ts new file mode 100644 index 00000000..5ff92dad --- /dev/null +++ b/tests/review/boundary.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from 'vitest'; +import { buildScope, isInScope, normalizeReviewUri } from '../../src/review'; + +describe('review boundary', () => { + const scope = buildScope({ + targetUri: 'file:///repo/docs/guide.md', + contextUris: ['file:///repo/docs/glossary.md'], + }); + + it('target is always in scope', () => { + expect(isInScope(scope, 'file:///repo/docs/guide.md')).toBe(true); + }); + + it('caller-supplied context uri is in scope', () => { + expect(isInScope(scope, 'file:///repo/docs/glossary.md')).toBe(true); + }); + + it('arbitrary workspace file is NOT in scope', () => { + expect(isInScope(scope, 'file:///repo/src/index.ts')).toBe(false); + }); + + it('rejects path traversal that escapes the target', () => { + expect(isInScope(scope, 'file:///repo/docs/../src/index.ts')).toBe(false); + }); + + it('normalizes traversal that resolves back onto an in-scope uri', () => { + expect(isInScope(scope, 'file:///repo/docs/sub/../guide.md')).toBe(true); + }); +}); + +describe('normalizeReviewUri', () => { + it('resolves dot segments in file uris', () => { + expect(normalizeReviewUri('file:///repo/a/../b/./c.md')).toBe('file:///repo/b/c.md'); + }); + + it('preserves authority', () => { + expect(normalizeReviewUri('file://host/x/../y.md')).toBe('file://host/y.md'); + }); +}); diff --git a/tests/review/budget.test.ts b/tests/review/budget.test.ts new file mode 100644 index 00000000..a63bcc24 --- /dev/null +++ b/tests/review/budget.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from 'vitest'; +import { + BudgetExceededError, + DEFAULT_REVIEW_BUDGET, + REVIEW_BUDGET_SCHEMA, + enforceBudget, +} from '../../src/review'; +import { VectorlintError } from '../../src/errors'; + +describe('review budget defaults', () => { + it('provides sensible positive defaults', () => { + expect(DEFAULT_REVIEW_BUDGET.maxModelCallsPerReview).toBeGreaterThan(0); + expect(DEFAULT_REVIEW_BUDGET.maxWallClockMs).toBeGreaterThan(0); + expect(DEFAULT_REVIEW_BUDGET.maxTargetBytes).toBeGreaterThan(0); + }); + + it('does not cap findings or headless retries', () => { + expect('maxFindingsPerRule' in DEFAULT_REVIEW_BUDGET).toBe(false); + expect('maxHeadlessRetries' in DEFAULT_REVIEW_BUDGET).toBe(false); + }); +}); + +describe('REVIEW_BUDGET_SCHEMA', () => { + it('applies defaults for omitted fields', () => { + const parsed = REVIEW_BUDGET_SCHEMA.parse({ maxModelCallsPerReview: 5 }); + expect(parsed.maxWallClockMs).toBe(DEFAULT_REVIEW_BUDGET.maxWallClockMs); + expect(parsed.maxModelCallsPerReview).toBe(5); + }); + + it('rejects non-positive limits', () => { + expect(() => REVIEW_BUDGET_SCHEMA.parse({ maxModelCallsPerReview: 0 })).toThrow(); + }); +}); + +describe('enforceBudget', () => { + it('is silent within limits', () => { + expect(() => + enforceBudget(DEFAULT_REVIEW_BUDGET, { modelCalls: 1, elapsedMs: 1000 }), + ).not.toThrow(); + }); + + it('throws BudgetExceededError when model calls exceed the limit', () => { + expect(() => + enforceBudget( + { ...DEFAULT_REVIEW_BUDGET, maxModelCallsPerReview: 3 }, + { modelCalls: 4, elapsedMs: 0 }, + ), + ).toThrow(BudgetExceededError); + }); + + it('throws BudgetExceededError when wall clock exceeds the limit', () => { + expect(() => + enforceBudget( + { ...DEFAULT_REVIEW_BUDGET, maxWallClockMs: 100 }, + { modelCalls: 0, elapsedMs: 101 }, + ), + ).toThrow(BudgetExceededError); + }); + + it('extends the repository error base', () => { + const err = new BudgetExceededError('boom', 'maxModelCallsPerReview', 9); + expect(err).toBeInstanceOf(VectorlintError); + expect(err.limit).toBe('maxModelCallsPerReview'); + expect(err.actual).toBe(9); + expect(err.code).toBe('BUDGET_EXCEEDED'); + }); +}); diff --git a/tests/review/executor.test.ts b/tests/review/executor.test.ts new file mode 100644 index 00000000..c15e670b --- /dev/null +++ b/tests/review/executor.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from 'vitest'; +import { + REVIEW_MODEL_CALLS, + chooseModelCall, + type ReviewExecutor, + type ReviewRequest, +} from '../../src/review'; + +describe('REVIEW_MODEL_CALLS', () => { + it('exposes single | agent | auto', () => { + expect(REVIEW_MODEL_CALLS).toEqual(['single', 'agent', 'auto']); + }); +}); + +describe('chooseModelCall', () => { + it('respects an explicit single call', () => { + expect(chooseModelCall('single', { targetBytes: 2_000_000, rules: 10 })).toBe('single'); + }); + + it('respects an explicit agent call', () => { + expect(chooseModelCall('agent', { targetBytes: 10, rules: 1 })).toBe('agent'); + }); + + it('returns single for small inputs under auto', () => { + expect(chooseModelCall('auto', { targetBytes: 10_000, rules: 1 })).toBe('single'); + }); + + it('returns agent for large inputs under auto', () => { + expect(chooseModelCall('auto', { targetBytes: 2_000_000, rules: 1 })).toBe('agent'); + }); + + it('returns agent for many rules under auto', () => { + expect(chooseModelCall('auto', { targetBytes: 1_000, rules: 6 })).toBe('agent'); + }); +}); + +describe('ReviewExecutor contract', () => { + it('can be implemented and run() returns a ReviewResult', async () => { + const fake: ReviewExecutor = { + run: () => Promise.resolve({ findings: [], scores: [], diagnostics: [] }), + }; + const result = await fake.run({} as unknown as ReviewRequest); + expect(result.findings).toEqual([]); + expect(result.diagnostics).toEqual([]); + }); +}); diff --git a/tests/review/module-surface.test.ts b/tests/review/module-surface.test.ts new file mode 100644 index 00000000..6c03ad30 --- /dev/null +++ b/tests/review/module-surface.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from 'vitest'; +import * as review from '../../src/review'; + +describe('src/review public surface', () => { + it('exports the core contract values and functions', () => { + // Runtime presence checks for values; types are checked by tsc. + expect(review.REVIEW_MODEL_CALLS).toBeDefined(); + expect(review.DEFAULT_REVIEW_BUDGET).toBeDefined(); + expect(typeof review.chooseModelCall).toBe('function'); + expect(typeof review.isInScope).toBe('function'); + expect(typeof review.buildReviewRequest).toBe('function'); + }); + + it('exposes the boundary, budget, and error helpers', () => { + expect(typeof review.buildScope).toBe('function'); + expect(typeof review.enforceBudget).toBe('function'); + expect(typeof review.normalizeReviewUri).toBe('function'); + expect(review.BudgetExceededError).toBeDefined(); + }); + + it('exposes the Zod schemas for every external shape', () => { + expect(review.REVIEW_TARGET_SCHEMA).toBeDefined(); + expect(review.REVIEW_RULE_SCHEMA).toBeDefined(); + expect(review.REVIEW_CONTEXT_SCHEMA).toBeDefined(); + expect(review.REVIEW_BUDGET_SCHEMA).toBeDefined(); + expect(review.REVIEW_OUTPUT_POLICY_SCHEMA).toBeDefined(); + expect(review.REVIEW_FINDING_SCHEMA).toBeDefined(); + expect(review.REVIEW_SCORE_SCHEMA).toBeDefined(); + expect(review.REVIEW_DIAGNOSTIC_SCHEMA).toBeDefined(); + expect(review.REVIEW_USAGE_SCHEMA).toBeDefined(); + expect(review.REVIEW_RESULT_SCHEMA).toBeDefined(); + expect(review.REVIEW_REQUEST_SCHEMA).toBeDefined(); + }); +}); diff --git a/tests/review/request-builder.test.ts b/tests/review/request-builder.test.ts new file mode 100644 index 00000000..618efa03 --- /dev/null +++ b/tests/review/request-builder.test.ts @@ -0,0 +1,97 @@ +import { describe, expect, it } from 'vitest'; +import { + DEFAULT_REVIEW_BUDGET, + REVIEW_RULE_SCHEMA, + buildReviewRequest, +} from '../../src/review'; +import { ValidationError } from '../../src/errors'; +import { Severity } from '../../src/evaluators/types'; +import type { PromptFile } from '../../src/schemas/prompt-schemas'; +import type { ReviewContext, ReviewTarget } from '../../src/review'; + +function makePrompt(overrides: Partial = {}): PromptFile { + return { + id: 'pseudo-advice', + filename: 'pseudo-advice.md', + fullPath: '/repo/presets/VectorLint/pseudo-advice.md', + meta: { + id: 'PseudoAdvice', + name: 'Pseudo Advice', + severity: Severity.WARNING, + type: 'check', + criteria: [{ id: 'Vague', name: 'Vague advice' }], + }, + body: 'Check for pseudo advice.', + pack: 'VectorLint', + ...overrides, + }; +} + +describe('buildReviewRequest', () => { + const target: ReviewTarget = { + uri: 'file:///repo/docs/guide.md', + content: '# Guide', + contentType: 'text/markdown', + }; + + it('maps one PromptFile to one ReviewRule with default budget and auto modelCall', () => { + const request = buildReviewRequest({ target, prompts: [makePrompt()] }); + expect(request.modelCall).toBe('auto'); + expect(request.budget).toEqual(DEFAULT_REVIEW_BUDGET); + expect(request.rules).toHaveLength(1); + + const rule = request.rules[0]; + expect(rule).toBeDefined(); + expect(rule?.id).toBe('VectorLint.PseudoAdvice'); + expect(rule?.source).toBe('/repo/presets/VectorLint/pseudo-advice.md'); + expect(rule?.body).toBe('Check for pseudo advice.'); + expect(rule?.severity).toBe('warning'); + expect(rule?.name).toBe('Pseudo Advice'); + // Strict parse proves no legacy evaluator/criteria/type fields leaked. + expect(() => REVIEW_RULE_SCHEMA.parse(rule)).not.toThrow(); + }); + + it('does not copy legacy meta.type, evaluator, or criteria fields', () => { + const request = buildReviewRequest({ target, prompts: [makePrompt()] }); + const rule = request.rules[0]; + expect(rule).toBeDefined(); + expect(rule && 'type' in rule).toBe(false); + expect(rule && 'evaluator' in rule).toBe(false); + expect(rule && 'criteria' in rule).toBe(false); + }); + + it('throws a ValidationError when no prompts are supplied', () => { + expect(() => buildReviewRequest({ target, prompts: [] })).toThrow(ValidationError); + }); + + it('passes caller-supplied context through unchanged', () => { + const context: ReviewContext[] = [ + { label: 'glossary', content: 'term a means ...', relation: 'reference' }, + ]; + const request = buildReviewRequest({ target, prompts: [makePrompt()], context }); + expect(request.context).toBe(context); + }); + + it('honors an explicit modelCall override', () => { + const request = buildReviewRequest({ + target, + prompts: [makePrompt()], + config: { modelCall: 'agent' }, + }); + expect(request.modelCall).toBe('agent'); + }); + + it('omits severity on the rule when the prompt has none', () => { + const request = buildReviewRequest({ + target, + prompts: [ + makePrompt({ + meta: { id: 'PseudoAdvice', name: 'Pseudo Advice' }, + }), + ], + }); + const rule = request.rules[0]; + expect(rule).toBeDefined(); + expect(rule && 'severity' in rule).toBe(false); + }); +}); diff --git a/tests/review/result.test.ts b/tests/review/result.test.ts new file mode 100644 index 00000000..3be3767a --- /dev/null +++ b/tests/review/result.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from 'vitest'; +import { + REVIEW_DIAGNOSTIC_SCHEMA, + REVIEW_FINDING_SCHEMA, + REVIEW_RESULT_SCHEMA, +} from '../../src/review'; + +describe('ReviewFinding schema', () => { + it('parses a complete finding', () => { + const finding = REVIEW_FINDING_SCHEMA.parse({ + ruleId: 'VectorLint.Consistency', + ruleSource: '/repo/presets/VectorLint/consistency.md', + severity: 'warning', + message: 'Vague advice.', + line: 5, + column: 1, + match: 'consider leveraging', + analysis: 'too vague', + suggestion: 'be specific', + fix: 'consider using concrete terms', + }); + expect(finding.line).toBe(5); + expect(finding.match).toBe('consider leveraging'); + expect(finding.severity).toBe('warning'); + }); + + it('rejects a finding with an unknown severity', () => { + expect(() => + REVIEW_FINDING_SCHEMA.parse({ + ruleId: 'P.R', + ruleSource: '/x.md', + severity: 'critical', + message: 'x', + line: 1, + column: 1, + match: 'x', + }), + ).toThrow(); + }); + + it('rejects a finding missing required anchored match evidence', () => { + expect(() => + REVIEW_FINDING_SCHEMA.parse({ + ruleId: 'P.R', + ruleSource: '/x.md', + severity: 'warning', + message: 'x', + line: 1, + column: 1, + }), + ).toThrow(); + }); +}); + +describe('ReviewDiagnostic schema', () => { + it('parses a diagnostic with level/code/message', () => { + const diag = REVIEW_DIAGNOSTIC_SCHEMA.parse({ + level: 'warn', + code: 'finding-evidence-not-locatable', + message: 'could not anchor the finding', + }); + expect(diag.level).toBe('warn'); + }); + + it('rejects an unknown level', () => { + expect(() => + REVIEW_DIAGNOSTIC_SCHEMA.parse({ level: 'fatal', code: 'x', message: 'y' }), + ).toThrow(); + }); +}); + +describe('ReviewResult schema', () => { + it('accepts findings/scores/diagnostics and defaults usage to undefined', () => { + const result = REVIEW_RESULT_SCHEMA.parse({ findings: [], scores: [], diagnostics: [] }); + expect(result.findings).toEqual([]); + expect(result.diagnostics).toEqual([]); + expect(result.usage).toBeUndefined(); + }); + + it('keeps diagnostics mandatory', () => { + expect(() => + REVIEW_RESULT_SCHEMA.parse({ findings: [], scores: [] }), + ).toThrow(); + }); + + it('accepts usage when provided', () => { + const result = REVIEW_RESULT_SCHEMA.parse({ + findings: [], + scores: [], + diagnostics: [], + usage: { modelCalls: 2, inputTokens: 10, outputTokens: 5 }, + }); + expect(result.usage?.modelCalls).toBe(2); + }); +}); diff --git a/tests/review/types.test.ts b/tests/review/types.test.ts new file mode 100644 index 00000000..d3bee632 --- /dev/null +++ b/tests/review/types.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it } from 'vitest'; +import { REVIEW_CONTEXT_SCHEMA, REVIEW_RULE_SCHEMA, REVIEW_TARGET_SCHEMA } from '../../src/review'; + +describe('ReviewTarget schema', () => { + it('parses a valid target', () => { + const target = REVIEW_TARGET_SCHEMA.parse({ + uri: 'file:///repo/docs/guide.md', + content: '# Guide\n\nBody text.', + contentType: 'text/markdown', + }); + expect(target.uri).toBe('file:///repo/docs/guide.md'); + }); + + it('allows empty content (streaming target)', () => { + expect(() => + REVIEW_TARGET_SCHEMA.parse({ uri: 'file:///x', content: '', contentType: 'text/plain' }), + ).not.toThrow(); + }); + + it('rejects a target missing a uri', () => { + expect(() => + REVIEW_TARGET_SCHEMA.parse({ content: 'x', contentType: 'text/plain' }), + ).toThrow(); + }); +}); + +describe('ReviewRule schema', () => { + it('parses a rule with required fields and defaults severity to warning', () => { + const rule = REVIEW_RULE_SCHEMA.parse({ + id: 'Consistency', + source: 'VectorLint/consistency.md', + body: 'Check internal consistency.', + violationConditions: [ + { id: 'Contradiction', description: 'The target makes mutually inconsistent claims.' }, + ], + }); + expect(rule.severity).toBe('warning'); + expect(rule.violationConditions?.[0]?.id).toBe('Contradiction'); + }); + + it('rejects legacy evaluator and judge criteria fields', () => { + expect(() => + REVIEW_RULE_SCHEMA.parse({ + id: 'Tone', + source: 'VectorLint/tone.md', + body: 'Judge whether the tone is good.', + evaluator: 'judge', + criteria: [{ id: 'Tone', name: 'Tone quality' }], + }), + ).toThrow(); + }); + + it('rejects an unknown severity', () => { + expect(() => + REVIEW_RULE_SCHEMA.parse({ + id: 'Tone', + source: 'VectorLint/tone.md', + body: 'x', + severity: 'critical', + }), + ).toThrow(); + }); +}); + +describe('ReviewContext schema', () => { + it('parses caller-supplied scoped content', () => { + const ctx = REVIEW_CONTEXT_SCHEMA.parse({ + label: 'related-glossary', + content: 'Term A means ...', + relation: 'reference', + }); + expect(ctx.label).toBe('related-glossary'); + }); +}); From 8f0680b1b9d26e2319a79f790bbaa7519a01452d Mon Sep 17 00:00:00 2001 From: Osho Emmanuel Date: Tue, 14 Jul 2026 15:16:11 +0100 Subject: [PATCH 10/12] refactor(cli): reframe agent mode as internal - Describe agent mode as an unreleased internal implementation path\n- Keep the standard-mode fallback notice focused on bounded harness rework\n- Remove the audit artifact and align orchestrator coverage with the notice --- README.md | 13 +- ...0-vectorlint-harness-architecture-audit.md | 380 ------------------ src/cli/commands.ts | 2 +- src/cli/orchestrator.ts | 8 +- tests/orchestrator-agent-output.test.ts | 14 +- 5 files changed, 17 insertions(+), 400 deletions(-) delete mode 100644 docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md diff --git a/README.md b/README.md index f6511e6a..4e1fba62 100644 --- a/README.md +++ b/README.md @@ -145,16 +145,13 @@ Notes: - Prompts and outputs are recorded when Langfuse observability is enabled. - Do not send secrets, credentials, or PII unless your policy explicitly allows observability tooling to access that data. -## Agent Mode (under review) +## Agent Mode (internal rework) -The `--mode agent` flag is under active rework. It currently enables an -autonomous cross-file review mode that is being **removed** in favor of a -bounded harness model. See -[`docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md`](docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md) -for the decision and the refactor plan. +The `--mode agent` flag was an unreleased internal implementation path. It is +being replaced by a bounded harness model in this refactor. -Until the refactor lands, `--mode agent` prints a deprecation warning and -falls back to standard mode. Do not build integrations against it. +While the refactor lands, `--mode agent` prints an internal-rework notice and +falls back to standard mode. Do not treat it as a public integration surface. ## Contributing diff --git a/docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md b/docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md deleted file mode 100644 index 51161ce0..00000000 --- a/docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md +++ /dev/null @@ -1,380 +0,0 @@ -# VectorLint Harness Architecture Audit - -Date: 2026-07-10 - -Status: Objective system audit plus product-direction audit. Request changes before shipping the current autonomous workspace-agent surface as a public contract. - -## Product Decision - -VectorLint should not keep the current autonomous workspace-agent mode. - -The retained agent-like capability should be a constrained reader executor: a model or headless agent can read the target content in sections when that is useful for context management, but it should not search the workspace, inspect arbitrary files, perform top-level workspace checks, or rewrite the rule being evaluated. - -The intended execution model is: - -- `direct`: send target content and rule in one structured review call. -- `reader`: give the executor a read-section capability over the target content only. -- `auto`: choose `direct` for normal-sized inputs and `reader` for large inputs or explicit context-management needs. - -This is an execution strategy decision, not a return to VectorLint-as-agent. - -## Product Direction Under Review - -VectorLint should become a programmable, bounded, on-page content review harness that another caller can invoke. - -The caller can be Codex, Claude Code, another coding agent, CI, a local CLI wrapper, or a future service. That caller owns exploration, cross-page reasoning, research, and context gathering. VectorLint owns the constrained review of target content against source-backed rules and returns structured findings, scores, diagnostics, and usage metadata. - -In practical terms: - -- VectorLint reviews the target page or explicitly supplied content. -- External context is caller-supplied, not discovered by VectorLint. -- Rules define review behavior, granularity, severity, and scoring constraints. -- Executors can be API models, headless local agents, or constrained reader executors, but they receive the same review request contract. -- VectorLint should not expose a model-controlled workspace tool loop as its core product surface. - -## Objective System Audit Summary - -Separate from product direction, the current codebase has several system-level liabilities: - -- Build health is weak: `npx tsc --noEmit`, `npm run lint`, and parts of `npm run test:run` fail in the current workspace. -- Runtime contracts are duplicated across hand-written TypeScript, JSON-schema objects, Zod schemas, and formatter-specific shapes. -- Standard mode and agent mode project the same underlying findings differently. -- CLI orchestration owns too many responsibilities: config, file matching, evaluation, scoring, output formatting, debug artifacts, and agent routing. -- Evidence verification and counting are inconsistent enough to affect exit behavior. -- Observability and debug paths can record full prompts, content, and outputs. -- Documentation and implementation disagree on the current agent-mode topology and tool contracts. - -## Architecture Verdict - -The current codebase contains two execution models at once: - -1. The older evaluator path: one structured model call per rule or chunk. -2. The newer autonomous workspace-agent path: VectorLint gives a model tools to read/search the workspace and call lint as a nested tool. - -The second path should not continue as-is. The useful piece is not "agent mode" broadly; it is target-aware reading for context management. The current implementation also duplicates result projection, scoring, evidence validation, output formatting, and configuration behavior from the first path. That is why the architecture feels harder to maintain: the code is not just messy; it is serving two different ownership models. - -The highest-leverage change is to define a neutral review-domain contract and make every executor implement that contract. - -## Proposed Core Contract - -The contract should be centered on review, not providers or agents. - -```ts -interface ReviewRequest { - target: ReviewTarget; - rules: ReviewRule[]; - context?: ReviewContext[]; - budget: ReviewBudget; - outputPolicy: ReviewOutputPolicy; -} - -interface ReviewExecutor { - run(request: ReviewRequest): Promise; -} - -interface ReviewResult { - findings: ReviewFinding[]; - scores: ReviewScore[]; - diagnostics: ReviewDiagnostic[]; - usage?: ReviewUsage; -} -``` - -API models, reasoning models, and headless CLI agents become adapter implementations behind this contract. The CLI becomes orchestration around request creation, result formatting, and exit behavior. - -## Major Findings - -### 1. The Current Agent Surface Is Broader Than The Decided Execution Model - -Evidence: - -- `README.md:148` presents `--mode agent` as autonomous cross-file review mode. -- `src/agent/prompt-builder.ts:43` instructs the model to perform workspace-level checks. -- `src/agent/executor.ts:670` exposes `read_file`. -- `src/agent/executor.ts:715` exposes `search_content`. - -Impact: - -VectorLint still behaves like it owns exploration. That puts it in competition with external coding agents instead of becoming a harness those agents can use. It also overshoots the decided reader-executor model, which only needs target-scoped reading for context management. - -Recommendation: - -Remove the current autonomous workspace-agent surface. Preserve only a constrained reader executor if needed: - -- It may read sections of the target content. -- It may not search the workspace. -- It may not read arbitrary files. -- It may not perform top-level workspace findings. -- It may not override source-backed rules. - -### 2. The Provider Interface Mixes Structured Review With Autonomous Tool Loops - -Evidence: - -- `src/providers/llm-provider.ts:28` requires both `runPromptStructured` and `runAgentToolLoop`. -- `src/providers/vercel-ai-provider.ts:136` implements the agent loop on the same provider. -- `src/agent/executor.ts:796` depends on the provider-level agent loop. - -Impact: - -Every future executor has to pretend to be both a structured model provider and an autonomous agent runner. That is the wrong abstraction for headless CLI support and for a constrained reader executor. - -Recommendation: - -Split the contracts: - -- `StructuredModelClient` for API structured output. -- `HeadlessReviewExecutor` for local CLI agents. -- `ReaderReviewExecutor` for target-scoped read-section execution. -- `ReviewExecutor` as the stable domain-level interface. -- `TelemetrySink` or observability decoration as an optional cross-cutting concern. - -### 3. Runtime Type Contracts Are Not Strong Enough For A Harness - -Evidence: - -- `src/providers/vercel-ai-provider.ts:119` returns `output as T`. -- `src/providers/llm-provider.ts:9` defines tool input and output as `unknown`. -- `src/agent/tools-registry.ts:21` exposes tools without typed output contracts. -- `npx tsc --noEmit` currently fails in core files. - -Impact: - -The code presents types as guarantees, but several are only assertions. A programmable harness needs reliable runtime validation because external callers and headless adapters will depend on these contracts. - -Recommendation: - -Tie structured execution to concrete Zod schemas or canonical parsers. Introduce typed tool contracts only if tools remain. Add a `typecheck` script and wire it into CI and build. - -### 4. Standard Mode And Agent Mode Project Results Differently - -Evidence: - -- `src/cli/orchestrator.ts:626` derives standard check severity from scoring. -- `src/agent/executor.ts:431` stamps agent findings with prompt severity. -- `src/agent/executor.ts:598` flattens judge criteria before recording findings. -- `src/cli/orchestrator.ts:1216` builds agent scores only for line output. -- `src/cli/orchestrator.ts:1237` emits structured output without agent scores or diagnostics. - -Impact: - -The same underlying review can produce different severity, rule identity, score data, JSON shape, and exit behavior depending on mode. That makes VectorLint hard to trust as a machine-facing gate. - -Recommendation: - -Extract one shared result projection pipeline: - -```text -PromptEvaluationResult - -> verified findings - -> rule and criterion identity - -> severity and score - -> diagnostics - -> output formatter -``` - -Every executor should return into this pipeline. - -### 5. The On-Page Boundary Is Not Enforced - -Evidence: - -- `src/agent/executor.ts:585` lets `lint` resolve any file inside `workspaceRoot`. -- `src/agent/executor.ts:670` lets `read_file` read any workspace file. -- `src/agent/executor.ts:715` lets `search_content` scan workspace content. -- `src/agent/executor.ts:134` allows model-supplied `reviewInstruction`. -- `src/agent/executor.ts:583` replaces the source-backed prompt body with the model-supplied override. - -Impact: - -Prompt-injected page content can indirectly steer the model to read non-target files or rewrite the rule being evaluated. That breaks deterministic review semantics and creates privacy and security risk. - -Recommendation: - -For the new harness, enforce a target/context allowlist: - -- Target content is always in scope. -- Caller-supplied context is in scope. -- Workspace reads are out of scope unless the caller explicitly includes those files as context. -- Reader execution can page through target content only. -- Rule bodies are source-backed and caller-authored, not model-authored. - -### 6. Evidence Handling Can Misreport Findings - -Evidence: - -- Standard mode skips unverifiable quotes but can still count original surfaced violation totals in some paths. -- `src/agent/executor.ts:415` attempts to locate evidence. -- `src/agent/executor.ts:426` falls back to the model-provided line when location fails. - -Impact: - -Standard mode can fail a run without printing the issue that caused the failure. Agent mode can surface hallucinated evidence as if it were verified. - -Recommendation: - -Use one evidence verifier. Count only emitted, verified findings. If evidence cannot be located, return a diagnostic and either skip the finding or mark the run operationally failed. - -### 7. Cost And Work Are Not Bounded Enough - -Evidence: - -- `src/providers/vercel-ai-provider.ts:161` defaults agent loops to 1000 steps. -- `src/agent/executor.ts:577` allows repeated nested `lint` calls. -- `src/evaluators/base-evaluator.ts:213` runs one model call per chunk. -- `src/agent/executor.ts:680` and `src/agent/executor.ts:715` glob before applying result caps. - -Impact: - -A single review can multiply cost through agent steps, nested lint calls, rules, chunks, and workspace search. Even a constrained reader executor will be slower than a direct call, so `auto` must have clear thresholds and budgets. - -Recommendation: - -Add explicit budgets: - -- Max target bytes. -- Max caller context bytes. -- Max chunks per rule. -- Max model calls per review. -- Max findings per rule. -- Max wall-clock duration. -- Max headless executor retries. - -### 8. Documentation Still Points At The Old Product - -Evidence: - -- `README.md:148` documents autonomous workspace-agent mode. -- `docs/specs/2026-03-17-agentic-capabilities-design.md:24` frames the roadmap around cross-document agent mode. -- The same spec describes implementation details that are not true now: one agent per rule, final `AgentFindingSchema`, AbortSignal propagation, ripgrep JSON, and Vale JSON omission of agent findings. - -Impact: - -Future contributors will optimize toward the wrong architecture and trust contracts that the code does not implement. - -Recommendation: - -Mark the agentic spec superseded. Replace it with a harness architecture spec that defines the review request, context boundary, executor interface, output contract, and migration plan. - -### 9. Secrets And Sensitive Content Need Better Handling - -Evidence: - -- `src/config/global-config.ts:13` says the global config stores API keys. -- `src/config/global-config.ts:76` creates the config directory with default permissions. -- `src/config/global-config.ts:81` writes the config file with default permissions. -- `src/observability/langfuse-observability.ts:66` records inputs. -- `src/observability/langfuse-observability.ts:67` records outputs. -- `src/providers/vercel-ai-provider.ts:59` can log full prompts and content. - -Impact: - -Provider keys, unreleased docs, caller context, and model outputs can leak through filesystem permissions, telemetry, debug logs, or persisted artifacts. - -Recommendation: - -Use private file modes for config and review artifacts. Make payload telemetry a separate opt-in from metadata telemetry. Add redaction or safe debug modes. - -## Verification Results - -Commands run from `/Users/klinsmann/Projects/TinyRocketLabs/vectorlint`: - -```bash -npm run lint -npm run test:run -npx tsc --noEmit -``` - -Results: - -- `npm run lint` failed before linting because typed ESLint rules were applied to a `.cjs` research script without parser type information. -- `npm run test:run` passed most tests but failed 4 suites during module resolution for `ora` and `@langfuse/otel`. -- `npx tsc --noEmit` failed with contract errors in agent, orchestrator, observability, logging, and provider modules. - -## Recommended Refactor Sequence - -### Phase 1: Stop The Bleeding - -1. Remove or hide the current autonomous workspace-agent mode. -2. Fix `tsc --noEmit`. -3. Fix test module resolution. -4. Fix lint configuration for non-TypeScript research scripts. -5. Add `typecheck` to CI and build validation. - -### Phase 2: Define The Harness Contract - -1. Create a neutral review-domain module. -2. Define `ReviewRequest`, `ReviewRule`, `ReviewContext`, `ReviewBudget`, `ReviewFinding`, `ReviewScore`, and `ReviewDiagnostic`. -3. Define target/context boundary rules. -4. Define structured output shape once. -5. Define execution strategy: `direct`, `reader`, and `auto`. - -### Phase 3: Share Result Projection - -1. Extract evidence location and verification. -2. Extract filtering. -3. Extract severity and scoring. -4. Extract output formatter inputs. -5. Make standard mode and any executor mode use the same projection path. - -### Phase 4: Replace The Agent Loop With Executors - -1. Keep API model execution as direct execution. -2. Add target-scoped reader execution as another executor. -3. Add headless CLI execution behind the same review contract if still useful. -4. Pass the same review request to each executor. -5. Remove model-controlled rule overrides. -6. Remove workspace read/search tools from core review. - -### Phase 5: Update Documentation - -1. Mark the agentic capabilities spec superseded. -2. Write a new harness architecture spec. -3. Update README and CLI docs. -4. Document migration from autonomous workspace-agent mode to direct/reader/auto execution. - -## Suggested Target Architecture - -```text -CLI / API / External Agent - -> ReviewRequestBuilder - -> ReviewExecutor - -> ApiModelExecutor - -> ReaderExecutor - -> HeadlessCliExecutor - -> ResultProjection - -> EvidenceVerifier - -> ViolationFilter - -> ScoreCalculator - -> Diagnostics - -> OutputFormatter - -> line - -> json - -> rdjson - -> vale-json -``` - -The important inversion is that VectorLint no longer asks, "What tools should this agent use?" It asks, "Given this target, these rules, this context, this execution strategy, and this budget, what findings can be verified?" - -## Final Recommendation - -The decided direction is cleaner than the current architecture. The current pain is not a sign that VectorLint is too complex; it is a sign that the codebase is carrying two ownership models and several duplicated runtime contracts. - -Make VectorLint the harness. Let external agents be agents. Keep reader execution only as a bounded way to manage target-content context. - ---- - -## Appendix: Phase 1 — Completed (2026-07-13) - -Phase 1 of the recommended refactor sequence ("Stop The Bleeding") landed on branch `codex/ci/harness-stop-the-bleeding`. It shifts the verification baseline recorded in "Verification Results" above and addresses Finding 1 at the CLI boundary. - -**Shifted baseline.** The 2026-07-13 pre-work capture differed from the original verification results: `npm run lint` and `npm run test:run` already passed, so the earlier lint failure and the four `ora` / `@langfuse/otel` module-resolution failures were stale. Only `npx tsc --noEmit` was still failing. Phase 1 cleared those remaining type errors (narrowed at boundaries; no strict compiler options relaxed) and added durable gates so the baseline cannot silently regress: - -- `npm run typecheck` (`tsc --noEmit`) and `npm run verify` (typecheck + lint + test:run) scripts. -- `vitest.config.ts` for stable test module resolution. -- `docs/research/**` excluded from linting. -- `.github/workflows/typecheck.yml` and `.github/workflows/test.yml` aligned to Node 20. - -**Current state.** `npm run verify`, `npm run build`, and built-CLI standard review smoke runs are green. Per Finding 1, the autonomous workspace-agent surface is deprecated at the CLI boundary: `--mode agent` emits a deprecation warning and falls back to standard evaluation. `src/agent/*` and the agent-mode helpers are retained as compile-only quarantine, unreachable from the CLI, pending removal in Phase 4. The deprecation notice and pointer to this audit live in the README `## Agent Mode` section and the `--mode` help text. - -This appendix records completion only. Findings 2–9 and the proposed core contract remain owned by Phases 2–5; no spec or architecture document is superseded here. diff --git a/src/cli/commands.ts b/src/cli/commands.ts index 50dbffc8..b8586407 100644 --- a/src/cli/commands.ts +++ b/src/cli/commands.ts @@ -82,7 +82,7 @@ export function registerMainCommand(program: Command): void { .option('--show-prompt-trunc', 'Print truncated prompt/content previews (500 chars)') .option('--debug-json', 'Write debug JSON artifacts (raw model output + filter decisions)') .option('--output ', `Output format: ${OUTPUT_FORMATS.join(', ')}`, OUTPUT_FORMATS[0]) - .option('--mode ', 'Execution mode: standard (default). "agent" is deprecated and falls back to standard.', DEFAULT_REVIEW_MODE) + .option('--mode ', 'Execution mode: standard (default). "agent" is unreleased/internal and falls back to standard.', DEFAULT_REVIEW_MODE) .option('-p, --print', 'Suppress interactive progress output in agent mode') .option('--config ', `Path to custom ${DEFAULT_CONFIG_FILENAME} config file`) .argument('[paths...]', 'files or directories to check (required)') diff --git a/src/cli/orchestrator.ts b/src/cli/orchestrator.ts index f8df367a..1fe29707 100644 --- a/src/cli/orchestrator.ts +++ b/src/cli/orchestrator.ts @@ -1168,8 +1168,8 @@ async function buildAgentRuleScores( return results; } -// Retained in quarantine: unreachable from the CLI after --mode agent was -// deprecated (now falls back to standard evaluation). Removed in Phase 4. +// Retained in quarantine: the unreleased --mode agent path now falls back to +// standard evaluation. Removed in Phase 4. // eslint-disable-next-line @typescript-eslint/no-unused-vars -- intentional quarantine async function evaluateFilesInAgentMode( targets: string[], @@ -1326,8 +1326,8 @@ export async function evaluateFiles( if (mode === AGENT_REVIEW_MODE) { options.logger?.warn( - '--mode agent is deprecated and now falls back to standard mode. ' + - 'See docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md.', + '--mode agent is an unreleased internal path and now falls back to standard mode. ' + + 'This internal path is being replaced by the bounded harness model.', ); // Fall through to standard evaluation; do not call evaluateFilesInAgentMode. } diff --git a/tests/orchestrator-agent-output.test.ts b/tests/orchestrator-agent-output.test.ts index 0b61ea63..98346e42 100644 --- a/tests/orchestrator-agent-output.test.ts +++ b/tests/orchestrator-agent-output.test.ts @@ -54,11 +54,11 @@ function makeStandardProvider(): StandardProviderSpies { return { provider, runPromptStructured, runAgentToolLoop }; } -// `--mode agent` is deprecated. These tests prove the CLI/evaluateFiles path no +// `--mode agent` is an internal fallback. These tests prove the CLI/evaluateFiles path no // longer reaches the autonomous agent executor: it warns through the injected // logger and falls back to standard evaluation. The retained agent executor // code is covered directly by tests/agent/* and removed in Phase 4. -describe('agent mode deprecation', () => { +describe('agent mode internal fallback', () => { const tempRepos: string[] = []; function createTempRepo(): string { @@ -80,7 +80,7 @@ describe('agent mode deprecation', () => { } }); - it('emits a deprecation warning through the injected logger and falls back to standard evaluation', async () => { + it('emits an internal-rework notice through the injected logger and falls back to standard evaluation', async () => { const repo = createTempRepo(); const file = path.join(repo, 'doc.md'); writeFileSync(file, 'bad phrase\n', 'utf8'); @@ -102,10 +102,10 @@ describe('agent mode deprecation', () => { }); expect(logger.warn).toHaveBeenCalledWith( - expect.stringContaining('deprecated'), + expect.stringContaining('unreleased internal path'), ); expect(logger.warn).toHaveBeenCalledWith( - expect.stringContaining('docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md'), + expect.stringContaining('bounded harness model'), ); // Standard evaluation ran; the autonomous executor did not. expect(runAgentToolLoop).not.toHaveBeenCalled(); @@ -134,7 +134,7 @@ describe('agent mode deprecation', () => { logger, }); - expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('deprecated')); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('unreleased internal path')); expect(runAgentToolLoop).not.toHaveBeenCalled(); expect(runPromptStructured).toHaveBeenCalled(); }); @@ -158,7 +158,7 @@ describe('agent mode deprecation', () => { scanPaths: [{ pattern: '**/*.md', runRules: ['Default'], overrides: {} }], }); - // No deprecation warning and no executor invocation in standard mode. + // No internal-rework notice and no executor invocation in standard mode. expect(runAgentToolLoop).not.toHaveBeenCalled(); expect(result.totalFiles).toBe(1); }); From 4c9519de557ea0774a3a20ba33f3ab2a23e2d7c0 Mon Sep 17 00:00:00 2001 From: Osho Emmanuel Date: Tue, 14 Jul 2026 15:52:14 +0100 Subject: [PATCH 11/12] Remove planning artifact references from review docs - Replace audit and phase references with stable review contract wording\n- Preserve review behavior and integration semantics\n- Validate review tests and repository verification --- src/review/README.md | 9 ++++----- src/review/boundary.ts | 7 +++---- src/review/budget.ts | 2 +- src/review/executor.ts | 4 ++-- src/review/index.ts | 2 +- src/review/request-builder.ts | 4 ++-- src/review/schemas.ts | 4 ++-- src/review/types.ts | 17 +++++++---------- 8 files changed, 22 insertions(+), 27 deletions(-) diff --git a/src/review/README.md b/src/review/README.md index bc2a120b..2b4b461e 100644 --- a/src/review/README.md +++ b/src/review/README.md @@ -19,7 +19,7 @@ A caller builds a `ReviewRequest`; an executor returns a `ReviewResult`. - `request-builder.ts` — `buildReviewRequest()` bridging `PromptFile` to `ReviewRequest`. -## On-page boundary (audit Finding #5) +## On-page boundary - Target content is always in scope. - Caller-supplied context is in scope. @@ -36,7 +36,6 @@ picks `single` for normal-sized inputs and `agent` for large ones. ## Wiring status -Phase 2 (this module) is purely additive and is **not** wired into the CLI. -Phase 3 emits `ReviewResult` via shared finding processing; Phase 4 implements -executors behind `ReviewExecutor` and removes the old agent loop. See -`docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md`. +This module is additive and is **not** wired into the CLI yet. Future CLI +wiring can emit `ReviewResult` through shared finding processing and implement +executors behind `ReviewExecutor`. diff --git a/src/review/boundary.ts b/src/review/boundary.ts index 8dc5bb0c..97e4c0e2 100644 --- a/src/review/boundary.ts +++ b/src/review/boundary.ts @@ -4,9 +4,8 @@ const FILE_URI_RE = /^file:\/\/(?[^/]*)(?\/.*)?$/; /** * Lexically resolves '.' and '..' segments in a path string. Pure: performs no - * filesystem reads. The normalization philosophy is adapted from - * src/agent/path-utils.ts (resolveWithinRoot), which is removed in Phase 4; - * copied here so src/review/ has no agent import and no filesystem access. + * filesystem reads. The helper is kept local so src/review/ has no agent + * import and no filesystem access. * * Symlink canonicalization is intentionally NOT performed inside the contract: * callers are responsible for canonicalizing real files into target/context @@ -41,7 +40,7 @@ export function normalizeReviewUri(uri: string): string { } /** - * Builds the on-page boundary scope (audit Finding #5) from the target URI and + * Builds the on-page boundary scope from the target URI and * any caller-supplied context URIs. Only these URIs are in scope; arbitrary * workspace files are out of scope unless the caller explicitly includes them. */ diff --git a/src/review/budget.ts b/src/review/budget.ts index e7d4cd8b..7f9ca5b8 100644 --- a/src/review/budget.ts +++ b/src/review/budget.ts @@ -3,7 +3,7 @@ import type { ReviewBudget } from './types'; import { VectorlintError } from '../errors'; /** - * Sensible default bounds for a single review (audit Finding #7). Frozen so a + * Sensible default bounds for a single review. Frozen so a * shared default object cannot be accidentally mutated by a caller. * * Note: there is intentionally no finding cap (`maxFindingsPerRule`) — diff --git a/src/review/executor.ts b/src/review/executor.ts index 2eef9011..74ae0100 100644 --- a/src/review/executor.ts +++ b/src/review/executor.ts @@ -15,7 +15,7 @@ export const REVIEW_MODEL_CALLS = ['single', 'agent', 'auto'] as const satisfies /** * The stable domain-level interface every executor implements. Single and - * agent executors are implementations behind this contract (audit Finding #2). + * agent executors are implementations behind this contract. * A headless/autonomous workspace executor is explicitly NOT part of this * contract. */ @@ -25,7 +25,7 @@ export interface ReviewExecutor { /** * Agent-call capability: a bounded way to page through target content for - * context management. NOT a workspace tool (audit Product Decision). An agent + * context management. NOT a workspace tool. An agent * executor receives this capability scoped to the target only. */ export interface ReviewTargetReadCapability { diff --git a/src/review/index.ts b/src/review/index.ts index c44dbe66..e2194b52 100644 --- a/src/review/index.ts +++ b/src/review/index.ts @@ -5,7 +5,7 @@ * source-backed rules flows through this module. The CLI, headless adapters, * and external callers all build a ReviewRequest and receive a ReviewResult. * - * See docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md. + * See README.md for the contract overview. */ export * from './types'; export * from './schemas'; diff --git a/src/review/request-builder.ts b/src/review/request-builder.ts index 72500596..770e33c0 100644 --- a/src/review/request-builder.ts +++ b/src/review/request-builder.ts @@ -63,8 +63,8 @@ function toReviewRule(prompt: PromptFile): ReviewRule { /** * Builds a {@link ReviewRequest} from existing {@link PromptFile}s plus a - * target. This is the conservative bridge Phase 4 uses to convert the current - * prompt pipeline into the review contract without changing how prompts load. + * target. Callers can use this bridge to convert the current prompt pipeline + * into the review contract without changing how prompts load. * Throws a {@link ValidationError} when no prompts are supplied. */ export function buildReviewRequest(params: BuildReviewRequestParams): ReviewRequest { diff --git a/src/review/schemas.ts b/src/review/schemas.ts index a1132cd3..2bca850b 100644 --- a/src/review/schemas.ts +++ b/src/review/schemas.ts @@ -5,8 +5,8 @@ import { REVIEW_BUDGET_SCHEMA } from './budget'; * Zod schemas mirroring src/review/types.ts (boundary validation). * * Every external review-domain shape has a paired schema here so callers and - * external adapters can validate untrusted input at the system boundary - * (audit Finding #3). Schemas are intentionally strict: unknown keys are + * external adapters can validate untrusted input at the system boundary. + * Schemas are intentionally strict: unknown keys are * rejected so legacy scoring-mode, rubric, and model-authored rule-override * fields cannot leak into the new contract. */ diff --git a/src/review/types.ts b/src/review/types.ts index 97e22039..9750d530 100644 --- a/src/review/types.ts +++ b/src/review/types.ts @@ -5,8 +5,6 @@ * source-backed rules flows through this module. Callers build a * {@link ReviewRequest} and executors return a {@link ReviewResult}. * - * See docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md. - * * This module is implementation-neutral: it deliberately exposes no legacy * scoring-mode, rubric, or model-authored rule-override surface. `modelCall` * selects how the reviewer model is invoked, not how rules are scored. @@ -22,8 +20,8 @@ export interface ReviewViolationCondition { } /** - * Target content under review. The on-page boundary (audit Finding #5) is - * enforced against this: executors may only read sections of `content`. + * Target content under review. The on-page boundary is enforced against this: + * executors may only read sections of `content`. */ export interface ReviewTarget { /** Stable absolute URI (file:// or virtual scheme for in-memory content). */ @@ -38,7 +36,7 @@ export interface ReviewTarget { /** * A source-backed, caller-authored rule. Model-authored rule overrides are - * explicitly disallowed (audit Finding #1, #5). + * explicitly disallowed. */ export interface ReviewRule { /** Stable id, formatted Pack.Rule[.Criterion] in output. */ @@ -56,7 +54,7 @@ export interface ReviewRule { } /** - * Caller-supplied context. Explicitly in scope (audit boundary rule): + * Caller-supplied context. Explicitly in scope: * VectorLint does NOT discover workspace files as context on its own; the * caller owns exploration and context gathering. */ @@ -82,7 +80,7 @@ export interface ReviewScope { } /** - * Hard bounds on a single review (audit Finding #7). These limit work, not + * Hard bounds on a single review. These limit work, not * output. Executors MUST check these and surface a ReviewDiagnostic (or fail * the run) when exceeded. */ @@ -97,9 +95,8 @@ export interface ReviewBudget { /** * Controls optional output/telemetry behavior. Diagnostics are always part of - * ReviewResult; this policy does not hide operational warnings. Audit - * Finding #9: payload telemetry must be a separate opt-in from metadata - * telemetry. + * ReviewResult; this policy does not hide operational warnings. Payload + * telemetry is a separate opt-in from metadata telemetry. */ export interface ReviewOutputPolicy { /** Include usage/cost in the result. */ From 82e86693b8b4b158f7dafef040f1b9e17b00636f Mon Sep 17 00:00:00 2001 From: Osho Emmanuel Date: Wed, 22 Jul 2026 23:32:03 +0100 Subject: [PATCH 12/12] refactor(review): remove rejected-design artifacts - Move BudgetExceededError into a focused review error module.\n- Remove comments and tests that preserve discarded contract concepts.\n- Add agent rules for surgical changes, error placement, and comments. --- AGENTS.md | 54 +++++++++++++++++++++++++++- src/review/README.md | 14 +++----- src/review/boundary.ts | 6 ++-- src/review/budget.ts | 22 +----------- src/review/errors.ts | 13 +++++++ src/review/executor.ts | 2 -- src/review/index.ts | 5 +-- src/review/request-builder.ts | 6 +--- src/review/schemas.ts | 4 +-- src/review/types.ts | 6 ++-- tests/review/budget.test.ts | 5 --- tests/review/request-builder.test.ts | 10 ------ tests/review/types.test.ts | 5 ++- 13 files changed, 83 insertions(+), 69 deletions(-) create mode 100644 src/review/errors.ts diff --git a/AGENTS.md b/AGENTS.md index 2d696a85..77d0a7ac 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,6 +22,59 @@ This repository implements VectorLint — a prompt‑driven, structured‑output - `presets/` — bundled rule packs (e.g., `VectorLint/`) - `tests/` — Vitest specs for config, scanning, evaluation, providers +## Agent Behavior Guidelines + +These guidelines reduce common LLM coding mistakes. They bias toward caution over speed; use judgment for trivial tasks. + +### Think Before Coding + +- State assumptions explicitly before implementing. +- If multiple interpretations exist, present them instead of picking silently. +- If a simpler approach exists, say so and push back when warranted. +- If something is unclear, stop, name the confusion, and ask. + +### Simplicity First + +- Write the minimum code that solves the problem. +- Do not add features beyond what was asked. +- Do not introduce abstractions for single-use code. +- Do not add flexibility or configurability that was not requested. +- Do not add error handling for impossible scenarios. +- If a change becomes larger than needed, simplify before finishing. + +### Surgical Changes + +- Touch only the files and lines required by the request. +- Do not improve adjacent code, comments, or formatting unless required. +- Match existing style, even when another style is tempting. +- If you notice unrelated dead code, mention it instead of deleting it. +- Remove imports, variables, functions, or files made unused by your own changes. +- Do not remove pre-existing dead code unless asked. + +### Goal-Driven Execution + +- Turn requests into verifiable goals before implementing. +- For bug fixes, write or identify a reproduction, then make it pass. +- For validation changes, cover invalid inputs, then make the checks pass. +- For refactors, verify behavior before and after the change. +- For multi-step tasks, state a brief plan with verification for each step. +- Loop until the success criteria are verified or a blocker is clear. + +### Error Organization + +- Put domain error classes in focused error files and import them into the modules that throw them. +- Reuse the repository's custom error hierarchy instead of throwing native `Error` directly. +- Catch errors as `unknown`; narrow or convert them with the existing error utilities. +- Introduce error handling only for real failure modes, not speculative or impossible scenarios. + +### Comments + +- Write comments only when they explain non-obvious current behavior, invariants, or constraints. +- Do not preserve planning discussions, rejected alternatives, absent fields, or speculative future architecture in production comments. +- Do not use comments to restate code that is already clear from names and types. +- Keep security and scope-boundary comments when they explain constraints that the implementation must preserve. +- Test supported behavior and current invariants; do not add negative tests solely to memorialize rejected designs. + ## Configuration System ### Quick Start @@ -132,7 +185,6 @@ For documents >600 words, VectorLint automatically chunks content: - Separation of concerns: rules define rubric; schemas enforce structure; CLI orchestrates; evaluators process; reporters format - Separation of concerns: when a file starts combining contracts, orchestration, and utility logic, extract shared helpers and types into focused modules - Extensibility: add providers by implementing `LLMProvider` or `SearchProvider`; add evaluators via registry pattern -- Error handling: prefer the repository's custom error hierarchy over native `Error`; catch blocks use `unknown` type and extend existing custom error types before introducing raw exceptions - Shared domain constants: avoid magic strings for core runtime concepts; define shared constants, enums, or types and import them where needed - Naming: choose domain-accurate names that reflect the real abstraction level; avoid use-case-specific terminology in shared runtime code - Logging: route runtime logging through an injected logger interface; keep concrete logger implementations behind the abstraction diff --git a/src/review/README.md b/src/review/README.md index 2b4b461e..aa55e470 100644 --- a/src/review/README.md +++ b/src/review/README.md @@ -8,11 +8,11 @@ A caller builds a `ReviewRequest`; an executor returns a `ReviewResult`. - `types.ts` — all review-domain interfaces (`ReviewTarget`, `ReviewRule`, `ReviewContext`, `ReviewBudget`, `ReviewFinding`, `ReviewScore`, `ReviewDiagnostic`, `ReviewResult`, `ReviewRequest`, `ReviewModelCall`). -- `schemas.ts` — Zod schemas mirroring every external shape (boundary - validation). All schemas are strict; legacy scoring-mode, rubric, and - model-authored rule-override fields are rejected. +- `schemas.ts` — strict Zod schemas mirroring every external shape (boundary + validation). - `budget.ts` — `DEFAULT_REVIEW_BUDGET`, `REVIEW_BUDGET_SCHEMA`, - `enforceBudget()`, and `BudgetExceededError` (extends `VectorlintError`). + and `enforceBudget()`. +- `errors.ts` — review-domain errors, including `BudgetExceededError`. - `boundary.ts` — `buildScope()` / `isInScope()` enforcing the on-page boundary. - `executor.ts` — `ReviewExecutor` interface, `REVIEW_MODEL_CALLS` (`single | agent | auto`), `chooseModelCall()`. @@ -33,9 +33,3 @@ A caller builds a `ReviewRequest`; an executor returns a `ReviewResult`. not how rules are scored. `single` sends target + rule in one structured call; `agent` gives the executor a target-scoped read-section capability; `auto` picks `single` for normal-sized inputs and `agent` for large ones. - -## Wiring status - -This module is additive and is **not** wired into the CLI yet. Future CLI -wiring can emit `ReviewResult` through shared finding processing and implement -executors behind `ReviewExecutor`. diff --git a/src/review/boundary.ts b/src/review/boundary.ts index 97e4c0e2..1a333695 100644 --- a/src/review/boundary.ts +++ b/src/review/boundary.ts @@ -7,9 +7,9 @@ const FILE_URI_RE = /^file:\/\/(?[^/]*)(?\/.*)?$/; * filesystem reads. The helper is kept local so src/review/ has no agent * import and no filesystem access. * - * Symlink canonicalization is intentionally NOT performed inside the contract: - * callers are responsible for canonicalizing real files into target/context - * URIs before building a ReviewRequest. + * The contract does not perform symlink canonicalization. Callers must + * canonicalize real files into target/context URIs before building a + * ReviewRequest. */ function resolveDotSegments(input: string): string { const segments: string[] = []; diff --git a/src/review/budget.ts b/src/review/budget.ts index 7f9ca5b8..5c26b6ad 100644 --- a/src/review/budget.ts +++ b/src/review/budget.ts @@ -1,15 +1,10 @@ import { z } from 'zod'; import type { ReviewBudget } from './types'; -import { VectorlintError } from '../errors'; +import { BudgetExceededError } from './errors'; /** * Sensible default bounds for a single review. Frozen so a * shared default object cannot be accidentally mutated by a caller. - * - * Note: there is intentionally no finding cap (`maxFindingsPerRule`) — - * VectorLint reports every verified issue it finds — and no headless retry - * budget (`maxHeadlessRetries`) because the decided architecture has no - * headless executor. */ export const DEFAULT_REVIEW_BUDGET: Readonly = Object.freeze({ maxTargetBytes: 1_000_000, @@ -44,21 +39,6 @@ export interface BudgetUsage { elapsedMs: number; } -/** - * Thrown by {@link enforceBudget} when current usage violates a hard limit. - * Extends the repository's canonical error base (VectorlintError). - */ -export class BudgetExceededError extends VectorlintError { - constructor( - message: string, - public readonly limit: keyof ReviewBudget, - public readonly actual: number, - ) { - super(message, 'BUDGET_EXCEEDED'); - this.name = 'BudgetExceededError'; - } -} - /** * Throws {@link BudgetExceededError} if current usage violates a hard limit. * Executors call this before/after each model call and on timeout checks. diff --git a/src/review/errors.ts b/src/review/errors.ts new file mode 100644 index 00000000..1e97aa98 --- /dev/null +++ b/src/review/errors.ts @@ -0,0 +1,13 @@ +import { VectorlintError } from '../errors'; +import type { ReviewBudget } from './types'; + +export class BudgetExceededError extends VectorlintError { + constructor( + message: string, + public readonly limit: keyof ReviewBudget, + public readonly actual: number, + ) { + super(message, 'BUDGET_EXCEEDED'); + this.name = 'BudgetExceededError'; + } +} diff --git a/src/review/executor.ts b/src/review/executor.ts index 74ae0100..441cdd7f 100644 --- a/src/review/executor.ts +++ b/src/review/executor.ts @@ -16,8 +16,6 @@ export const REVIEW_MODEL_CALLS = ['single', 'agent', 'auto'] as const satisfies /** * The stable domain-level interface every executor implements. Single and * agent executors are implementations behind this contract. - * A headless/autonomous workspace executor is explicitly NOT part of this - * contract. */ export interface ReviewExecutor { run(request: ReviewRequest): Promise; diff --git a/src/review/index.ts b/src/review/index.ts index e2194b52..95ac2e07 100644 --- a/src/review/index.ts +++ b/src/review/index.ts @@ -2,13 +2,14 @@ * Neutral review-domain contract for the VectorLint harness. * * Everything an executor needs to review target content against - * source-backed rules flows through this module. The CLI, headless adapters, - * and external callers all build a ReviewRequest and receive a ReviewResult. + * source-backed rules flows through this module. The CLI and external callers + * build a ReviewRequest and receive a ReviewResult. * * See README.md for the contract overview. */ export * from './types'; export * from './schemas'; +export * from './errors'; export * from './budget'; export * from './boundary'; export * from './executor'; diff --git a/src/review/request-builder.ts b/src/review/request-builder.ts index 770e33c0..56a3d5a7 100644 --- a/src/review/request-builder.ts +++ b/src/review/request-builder.ts @@ -39,11 +39,7 @@ function toReviewRuleId(prompt: PromptFile): string { return `${pack}.${prompt.meta.id}`; } -/** - * Converts a single PromptFile into a source-backed ReviewRule, mapping only - * fields that belong in the neutral contract. Legacy `meta.type`, `evaluator`, - * `criteria`, and judge/rubric fields are intentionally NOT copied. - */ +/** Converts a single PromptFile into a source-backed ReviewRule. */ function toReviewRule(prompt: PromptFile): ReviewRule { const rule: ReviewRule = { id: toReviewRuleId(prompt), diff --git a/src/review/schemas.ts b/src/review/schemas.ts index 2bca850b..398c8803 100644 --- a/src/review/schemas.ts +++ b/src/review/schemas.ts @@ -6,9 +6,7 @@ import { REVIEW_BUDGET_SCHEMA } from './budget'; * * Every external review-domain shape has a paired schema here so callers and * external adapters can validate untrusted input at the system boundary. - * Schemas are intentionally strict: unknown keys are - * rejected so legacy scoring-mode, rubric, and model-authored rule-override - * fields cannot leak into the new contract. + * Schemas reject unknown keys. */ export const REVIEW_SEVERITY_SCHEMA = z.enum(['error', 'warning']); diff --git a/src/review/types.ts b/src/review/types.ts index 9750d530..2df19e08 100644 --- a/src/review/types.ts +++ b/src/review/types.ts @@ -4,10 +4,8 @@ * Everything an executor needs to review target content against * source-backed rules flows through this module. Callers build a * {@link ReviewRequest} and executors return a {@link ReviewResult}. - * - * This module is implementation-neutral: it deliberately exposes no legacy - * scoring-mode, rubric, or model-authored rule-override surface. `modelCall` - * selects how the reviewer model is invoked, not how rules are scored. + * `modelCall` selects how the reviewer model is invoked, not how rules are + * scored. */ /** Finding/Rule severity. */ diff --git a/tests/review/budget.test.ts b/tests/review/budget.test.ts index a63bcc24..f1e864f6 100644 --- a/tests/review/budget.test.ts +++ b/tests/review/budget.test.ts @@ -13,11 +13,6 @@ describe('review budget defaults', () => { expect(DEFAULT_REVIEW_BUDGET.maxWallClockMs).toBeGreaterThan(0); expect(DEFAULT_REVIEW_BUDGET.maxTargetBytes).toBeGreaterThan(0); }); - - it('does not cap findings or headless retries', () => { - expect('maxFindingsPerRule' in DEFAULT_REVIEW_BUDGET).toBe(false); - expect('maxHeadlessRetries' in DEFAULT_REVIEW_BUDGET).toBe(false); - }); }); describe('REVIEW_BUDGET_SCHEMA', () => { diff --git a/tests/review/request-builder.test.ts b/tests/review/request-builder.test.ts index 618efa03..affb83a2 100644 --- a/tests/review/request-builder.test.ts +++ b/tests/review/request-builder.test.ts @@ -47,19 +47,9 @@ describe('buildReviewRequest', () => { expect(rule?.body).toBe('Check for pseudo advice.'); expect(rule?.severity).toBe('warning'); expect(rule?.name).toBe('Pseudo Advice'); - // Strict parse proves no legacy evaluator/criteria/type fields leaked. expect(() => REVIEW_RULE_SCHEMA.parse(rule)).not.toThrow(); }); - it('does not copy legacy meta.type, evaluator, or criteria fields', () => { - const request = buildReviewRequest({ target, prompts: [makePrompt()] }); - const rule = request.rules[0]; - expect(rule).toBeDefined(); - expect(rule && 'type' in rule).toBe(false); - expect(rule && 'evaluator' in rule).toBe(false); - expect(rule && 'criteria' in rule).toBe(false); - }); - it('throws a ValidationError when no prompts are supplied', () => { expect(() => buildReviewRequest({ target, prompts: [] })).toThrow(ValidationError); }); diff --git a/tests/review/types.test.ts b/tests/review/types.test.ts index d3bee632..3479074f 100644 --- a/tests/review/types.test.ts +++ b/tests/review/types.test.ts @@ -38,14 +38,13 @@ describe('ReviewRule schema', () => { expect(rule.violationConditions?.[0]?.id).toBe('Contradiction'); }); - it('rejects legacy evaluator and judge criteria fields', () => { + it('rejects unknown fields', () => { expect(() => REVIEW_RULE_SCHEMA.parse({ id: 'Tone', source: 'VectorLint/tone.md', body: 'Judge whether the tone is good.', - evaluator: 'judge', - criteria: [{ id: 'Tone', name: 'Tone quality' }], + unexpected: true, }), ).toThrow(); });