Repository navigation
Add neutral review-domain contract module - #97
Merged
Merged
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)
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughChangesReview Domain
Agent Guidelines
Estimated code review effort: 3 (Moderate) | ~25 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
- 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
- Replace audit and phase references with stable review contract wording\n- Preserve review behavior and integration semantics\n- Validate review tests and repository verification
…ng' into codex/feat/harness-review-contract
…view-contract # Conflicts: # README.md # src/cli/commands.ts # src/cli/orchestrator.ts # tests/orchestrator-agent-output.test.ts
oshorefueled
changed the base branch from
codex/ci/harness-stop-the-bleeding
to
main
July 17, 2026 00:17
oshorefueled
commented
Jul 22, 2026
- 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.
oshorefueled
marked this pull request as ready for review
July 22, 2026 22:34
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.
Summary
Adds the neutral review-domain contract for the harness refactor. This gives follow-on work a typed, Zod-validated
ReviewRequest->ReviewResultsurface with explicit boundaries, budgets, model-call selection, and a conservative bridge from existing prompt files.Branch Target
release-docsbecause it updates docs for the latest released CLI and should be published now.mainbecause parent Ticket 01 has merged and this branch has been advanced ontomain.codex/ci/harness-stop-the-bleedingbecause it builds on Ticket 01.What this PR covers
src/review/with the contract types, strict Zod schemas, boundary helpers, budget enforcement, model-call selection,ReviewExecutor, andbuildReviewRequest.tests/review/coverage for module surface, schemas, budget behavior, boundary scope, result shapes, model-call selection, and request building.Scope
In scope:
Out of scope:
src/cli,src/agent,src/providers,src/output,src/scoring, orsrc/evaluators.Behavior Impact
Risk and Mitigations
99f73c2and4c9519d) or revert the merged PR.API / Contract / Schema Changes
src/review/.ReviewRequest,ReviewRule,ReviewContext,ReviewBudget,ReviewFinding,ReviewScore,ReviewDiagnostic,ReviewUsage,ReviewResult, and paired schemas.modelCall: 'single' | 'agent' | 'auto'.Follow-ups
buildReviewRequestoutput at the external integration boundary.Known Tradeoffs
buildReviewRequestmaps existing validatedPromptFilevalues conservatively and does not re-parse its own output; callers should validate at the external wiring boundary.Verification
How to test / verify
Checks run
npm run verify- passed, 52 files / 346 tests.npm run test:run -- tests/review- passed, 7 files / 46 tests.src/reviewandtests/review- no matches.origin/mainforsrc/cli,src/agent,src/providers,src/output,src/scoring, andsrc/evaluators- empty.Summary by CodeRabbit
New Features
Documentation
Tests