Replace autonomous agent loop with bounded executors#99
Draft
oshorefueled wants to merge 26 commits into
Draft
Conversation
- 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
- 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
- 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
- Runs typecheck, lint, and test:run in sequence - Single command for CI and downstream refactor phases to prove the verification baseline
- 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
- 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
- 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
- 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
- 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)
- Add src/findings/finding-evidence-verifier.ts wrapping locateQuotedText from src/output/location.ts as the single source of truth (audit #6). - Anchored quotes return verified line/column/match; unanchored quotes return a finding-evidence-not-locatable warn diagnostic and no finding. - Never falls back to a model-provided line number the way the old agent path did. - Cover anchored, unanchored, no-line-hint, and truncation cases.
- Add src/findings/types.ts with the FindingProcessingInput contract and strict Zod schemas; no modelCall/mode/agent/evaluator/judge/rubric fields, so legacy judge/evaluator-shaped input is rejected. - Add src/findings/severity.ts: resolveSeverity derives severity from the calculateCheckScore result (one path, no agent stamping); buildRuleId matches Pack.Rule[.Criterion] naming without importing src/agent/. - Add src/findings/scorer.ts as a thin wrapper over calculateCheckScore; the verified finding count drives the score. - Cover schema parse/reject, severity, rule-id, criterion resolution, and score numerics against calculateCheckScore.
- Add src/findings/processor.ts: processFindings runs the single pipeline (filter -> verify -> dedupe -> score -> severity -> diagnostics) and returns the Phase 2 ReviewResult from src/review/types.ts. - Verified finding count, not raw candidate count, drives the score; unanchored evidence becomes a warn diagnostic and emits no finding; hadOperationalErrors is true only for error-level diagnostics. - Add src/findings/index.ts barrel export. - Golden processor test asserts findings/score match the standard check path; cover diagnostics, filtering, verified-count fix, dedupe, rule-id attribution, and ReviewResult contract validation.
- Build FindingProcessingInput and call processFindings in the standard objective-check branch of routePromptResult - Feed returned findings/scores/diagnostics through the existing line/json/rdjson/vale sinks and quality-score output - Remove inline filter/score/severity/quote-location logic from the check path; computeFilterDecision kept only for the debug-run artifact - Verified finding count now drives counts and score (audit Finding #6); unanchored quotes become warn diagnostics and no longer flag operational errors - Add orchestrator regression tests covering finding/score output, JSON/Vale shape, and exit behavior for supported objective-check reviews
- PROMPT_META_SCHEMA now rejects `type: judge` and its deprecated alias `subjective` via a superRefine, so judge-typed prompts fail to load with a descriptive warning instead of reaching evaluation. - superRefine (not refine) keeps the inferred union including "judge" so the legacy BaseEvaluator type-checks unchanged until Phase 4 deletes that path. - Add loader tests covering judge/subjective rejection and that check still loads; subjective and semi-objective are documented as deprecated aliases.
- Drop the judge branch of routePromptResult and the now-dead criterion extraction/reporting helpers (extractAndReportCriterion, validateCriteriaCompleteness, validateScores, locateAndReportViolations, buildRuleName); subjective rubric scoring is not a future-facing review type. - A JudgeResult that still reaches the orchestrator is refused explicitly (operational error, no findings) rather than misprojected as a check result; no adapter is added for old judge metadata. - Standard objective-check orchestration still routes through processFindings unchanged. - Replace the judge-filtering orchestrator test with a judge-rejection test.
- Drop ProcessViolationsParams, ProcessCriterionParams, ProcessCriterionResult, and ValidationParams from src/cli/types.ts. They were orphaned when the orchestrator's inline violation/criterion processing moved to src/findings (Tasks 2-3) and the judge path was removed, and are referenced nowhere else. - Remove the now-unused imports PromptMeta, PromptCriterionSpec, ScoreComponent, and JudgeResult. Severity and PromptEvaluationResult are retained (still used by ReportIssueParams / ProcessPromptResultParams).
- Add tests/findings/README.md mapping the behavior moved into src/findings (filtering, evidence verification, density scoring, rule-id/severity, formatter routing) and the intentionally removed judge/rubric and agent behavior, with agent tests marked out of scope for Phase 3 (Phase 3 Task 8). - Stop describing type: judge/subjective as a valid future-facing prompt type in CREATING_RULES.md, docs/frontmatter-fields.mdx, and docs/style-guide.mdx; note that judge/subjective are now rejected at load. - Drop the stale rubric-based scoring feature blurb from README.md.
- Map meta.criteria to { id, name } before processFindings so legacy
PromptCriterionSpec rubric fields (weight, target) cannot cross the
standard check orchestration -> findings contract
- Add tests/orchestrator-finding-criteria.test.ts mocking processFindings
to assert weight/target are stripped on the production path
- prompt-loader criteria support is unchanged; criterion id/name still
drive output rule-id attribution
- Add StructuredModelClient (runPromptStructured + LLMResult/TokenUsage) as
the permanent single structured-output capability
- Add ToolCallingModelClient: bounded, executor-owned tool transport
(runWithTools) that names no workspace/agent-finding concepts
- Make LLMProvider extend StructuredModelClient; keep runAgentToolLoop only
as a temporary compile bridge for src/agent/ (deleted in a later task)
- VercelAIProvider implements both capabilities via a bounded runWithTools
transport (caller-owned tools + caller-set step budget), no product loop
- Extend AIExecutionContext.operation with a transport-shaped 'tool-calling'
label
- Add provider contract tests asserting the new surfaces and the absence of
legacy workspace-agent concepts
Refs: docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md
(Finding #2; Product Decision)
- Add SingleModelCallExecutor behind the Phase 2 ReviewExecutor contract, reviewing source-backed rules with one structured model call per (rule, chunk) through an injected StructuredModelClient. - Build rule prompts verbatim from request.rules[i].body; no tool surface and no reviewInstruction. - Reuse buildCheckLLMSchema, RecursiveChunker, mergeViolations, prependLineNumbers, and processFindings instead of parallel projection. - Enforce ReviewBudget.maxModelCallsPerReview via enforceBudget before each call; on exhaustion record an error diagnostic + hadOperationalErrors and return partial results rather than throwing past the contract. - Record usage (modelCalls, wallClockMs always; input/output tokens gated on outputPolicy.includeUsage) consistent with ReviewResult.usage. - Add focused tests: one call per rule, projected findings/scores/diagnostics, unanchored-evidence diagnostic, budget exhaustion, no tool calls, and large-target chunking/merging.
- Add AgentModelCallExecutor implementing ReviewExecutor via one bounded ToolCallingModelClient.runWithTools call per rule - Expose exactly one executor-owned tool, read_target_section, through TargetReadCapability bound to in-memory request.target.content (no filesystem or non-target URI access) - Map out-of-range/invalid ranges to model-visible error results so the bounded run continues instead of aborting - Enforce ReviewBudget.maxModelCallsPerReview via enforceBudget before each run, surfacing exhaustion as an error diagnostic with partial results - Pass request.rules[i].body verbatim as the source-backed prompt; no reviewInstruction is accepted or introduced - Project results through src/findings/processor.ts so anchored findings, scores, and diagnostics match the shared pipeline - Extract shared executor helpers (splitRuleId, buildEvalContext, buildReviewUsage, toFindingSeverity, budgetExceededDiagnostic, RunCounters) into src/executors/shared.ts and reuse from the single executor to remove duplication - Add focused tests for the agent executor and target-read adapter
- Reject the removed --mode option without silently remapping it.\n- Cover single, agent, and auto executor dispatch plus ReviewResult routing.\n- Preserve the breaking CLI migration to --model-call single|agent|auto.
- create global config resources with restrictive permissions\n- propagate opt-in payload telemetry through both executors\n- cover CLI, package entrypoints, observability, and provider logging
- Keep observability operation types aligned with the executor-only review surface
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- AgentToolError's only consumers were src/agent/executor.ts, src/agent/path-utils.ts, and tests/agent/agent-executor.test.ts, all deleted with the autonomous workspace-agent surface - Remove the now-unused class so no agent-tied symbol lingers in the error hierarchy Refs: docs/audits/2026-07-10-vectorlint-harness-architecture-audit.md (Product Decision; Finding #2)
oshorefueled
changed the base branch from
codex/refactor/harness-finding-processing
to
main
July 23, 2026 00:21
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The previous internal autonomous review loop mixed provider transport, workspace tools, and finding processing, making the harness hard to bound and review. This PR replaces that implementation path with explicit executor choices and a target-only agent path.
What this PR covers
--model-call single|agent|auto, withautoresolving to the single-call path for normal reviews and the target-paging agent path for larger runs.read_target_section, which pages through only the target content already under review.--modehandling now fails clearly and points contributors to--model-call.Scope
SingleModelCallExecutorandAgentModelCallExecutor.--model-call.src/agent/and obsolete autonomous-loop tests.Behavior Impact
--model-callis the supported strategy selector for this harness path.0700directory and0600file modes.Risk and Mitigations
API / Contract / Schema Changes
StructuredModelClientandToolCallingModelClientprovider capability contracts.recordPayloadTelemetry.--modesurface with--model-call single|agent|auto.Follow-ups
Known Tradeoffs
README.md, but did not receive a model response before bounded termination.How to test / verify
Checks run
npm run test:run -- tests/providers/structured-model-client.test.ts tests/providers/tool-calling-model-client.test.tsnpm run test:run -- tests/executors/npm run test:run -- tests/executors/agent-model-call-executor.test.ts tests/executors/target-read-capability-adapter.test.ts tests/executors/single-model-call-executor.test.tsnpm run test:run -- tests/global-config.test.ts tests/observability/langfuse-observability.test.ts tests/vercel-ai-provider.test.ts tests/package-entrypoint.test.ts tests/main-command-observability.test.ts tests/executors/npm run test:run -- tests/observability/langfuse-observability.test.ts tests/vercel-ai-provider.test.ts tests/executors/agent-model-call-executor.test.ts tests/executors/target-read-capability-adapter.test.tsnpm run verifynpm run buildgit diff --check 4fa07011105f3f2b84df3ae22d41fd774e27749e...HEADrg -n "runAgentToolLoop|AgentToolDefinition|AGENT_REVIEW_MODE|--mode agent|reviewInstruction|agent-tool-loop" src teststest ! -d src/agent && test ! -d tests/agentManual verification
npm start -- README.md --model-call single: command started and resolvedREADME.md; external provider response did not complete within a 90 second bounded wait, so the command exited124.