AI Effective Config and Review Execution

This page is the architecture index for Lintro's AI review execution seams. The normative decision record is [ADR-0006](../adr/0006-ai-effective-config-and-review-execution.md) (epic

AI Effective Config and Review Execution

This page is the architecture index for Lintro’s AI review execution seams. The normative decision record is ADR-0006 (epic #1972).

Ownership boundaries

ConcernOwner todayTarget
Effective ResolvedAIConfig for one invocationresolve_effective_ai_config() (#2299), the one caller of AIConfig.resolve_from_mappingSame resolver; #1923 extends it
Invocation transport / timeout / cost-cap overridesAICliOverrides on that resolver, for lint and review alikeSame resolver pipeline (#1970 / #1923 / #2024)
Monotonic cost-cap clampMCP adapter (resolve_budget_policy)Shared domain prep; adapters keep policy
Diff review preparationprepare_review / execute_review (#2300)Done (Phase 3)
Review execution facaderun_review / run_review_asyncDone (Phase 4); internals split across modules
Provider client aclose() APIBaseAIProvider.aclose() and provider overrides (#1885)Provider-side only in #1885
Provider close call-site wiringReviewSession in session.py, entered by run_review_async (#2302)Done (Phase 5)

Shared preparation (lintro/ai/review/preparation.py)

Since #2300 both the CLI (lintro review) and MCP (lintro_review) run one shared path — ReviewRunRequest → prepare_review → PreparedReview → execute_review:

  1. Each adapter resolves AI config through resolve_effective_ai_config — one resolver pipeline for both (#2299). What each passes into it differs by design: CLI review passes its flags as AICliOverrides, which may change values and stamp flag provenance, while MCP passes none because its one per-call knob (max_cost_usd) is a downstream monotonic clamp (resolve_budget_policy) rather than an overlay. Given identical resolver inputs the two produce identical values and provenance; the parity suite asserts exactly that.
  2. Each adapter gates on ai.review being enabled (adapter-specific error shape) and builds a ReviewRunRequest from its own argument surface.
  3. prepare_review then does the deterministic, provider-free work once: apply the run’s timeout and the transport profile, collect the review context (honouring ai.exclude_paths on both surfaces), classify changed files, select and format the checklist, optionally build the lint digest, resolve sensitivity, and resolve custom agents from the request’s CustomAgentMode (both adapters forward review.custom_agents).
  4. Each adapter constructs its own provider — so each labels its own construction failure — and calls execute_review, which is the single run_review call site. Lifetime is not the adapter’s: since #2302 the run enters a ReviewSession that closes that provider, and any provider a custom agent’s model override builds, exactly once on every exit path. An adapter must not reuse the provider after execute_review returns.
  5. Each adapter translates failures into its own contract.

ReviewExecutionPolicy carries what is genuinely adapter-only into execute_review: terminal progress, --context-window, resume state, --full, and the CLI’s cost-cap gate. MCP runs on DEFAULT_EXECUTION_POLICY, whose values are the ReviewSessionOptions defaults. It is a frozen value object that may carry an optional progress callback — not a hook or plugin architecture.

Adapter-only policy that must stay out of the shared layer:

  • CLI: Click errors, progress UI, JSON/terminal rendering, GitHub posting, exit 0/1/2.
  • MCP: workspace locking, budget clamp, structured tool envelopes, no posting.
  • Advisory-only CLI mode: master-switch semantics without the diff-review sub-toggle.

Orchestrator phase plan

run_review remains the stable facade. Phase 4 decomposed the internals into session, planning, chunk fan-out, per-chunk passes, prompts, merge and result-assembly modules without changing prompts, findings, severity, or exit semantics. Every provider call continues through call_ai; prompt redaction remains mandatory.

lintro/ai/review/orchestrator.py is now the sequence and nothing else. It sits well below the 800-line [tool.lintro.module_size] threshold, so it no longer needs a baseline entry, and it carries no C901 / PLR0912 / PLR0913 / PLR0915 suppression. run_review_async reads as three steps:

StepModuleWhat it owns
Planlintro/ai/review/run_planning.pySensitivity policy, diff budget (incl. the CLI ceiling), chunks, resume plan, agent selection
Runlintro/ai/review/run_execution.pyThe provider-call sequence and the two finalizers (completed / gracefully stopped)
lintro/ai/review/chunk_runner.pyBounded-concurrency fan-out, graceful stops, incremental coverage checkpoints
lintro/ai/review/chunk_pass.pyOne chunk’s depth-1/2/3 passes
Reportlintro/ai/review/result_assembly.pyTotals, file selection, coverage records, flag reconciliation, ReviewMetadata

The Run step is a sequence, not a single call: run_execution.execute_run runs the chunk fan-out, then the custom-agent passes (custom_agent_runner.py), merges and filters (merge.py), and may run the cross-chunk synthesis pass (synthesis.py). The two halves of the flow travel as frozen objects: ReviewRunPlan (what the run resolved to do) and ReviewRunOutcome (what it produced, completed or stopped).

Session options (lintro/ai/review/session.py, #2301)

Phase 4 ended the keyword wall. Every setting a review run takes is a field on the frozen ReviewSessionOptions, and that object is the whole argument surface: run_review(context, *, options) forwards it to run_review_async(context, *, options), which reads it and nothing else. The defaults live on the dataclass only — the facade declares none of its own — so what a caller who omits a setting gets is decided in exactly one place. New settings are added as a field rather than as another keyword threaded through each layer. Two neighbours share the module: ChunkRunPlan, the run-scope bundle every chunk layer reads, and the graceful-stop predicates (is_cost_cap_stop, cost_cap_reason, is_timeout_stop, timeout_reason) with the aborted_before_completion wrapper — deciding whether a run stopped gracefully is session-level, not chunk-level.

Prompt construction (lintro/ai/review/prompts.py, #2301)

The second slice moves the two chunk prompt builders — build_review_prompt for the API transport and build_git_native_review_prompt for CLI-backed providers — and the non-diff token estimate estimate_prompt_overhead out of the orchestrator. The shared render inputs (chunk, context, checklist text and count, interaction paths, lint digest, the run’s per-PR questions, strictness section) travel as one frozen PromptInputs; only the git-native diff-delivery flags stay as separate keywords, since they are the one thing the two builders do not share. redact_prompt_text and make_boundary_marker now fire inside this module, which makes it the redaction choke point for prompt bytes: the git-native builder still embeds the redacted diff unless the caller explicitly opts out. That slice left the emitted bytes unchanged and the #2298 prompt goldens passed without regeneration; the milestone 0.5 findings-only prompts have since changed those bytes, so the prompt goldens are regenerated here.

Cross-chunk merge (lintro/ai/review/merge.py, #2301)

The third slice moves the fold-back stage out of the orchestrator: the partial a chunk returns (ChunkReviewPartial), the six merge_* functions that combine partials into one ReviewResult, the finalize_partials wrapper that merges and then applies the sensitivity policy, and parse_review_response. The merge rules are behaviour, not implementation detail, and the #2298 merge goldens pin them: findings deduplicate by (file, line, title) in first-seen order, a yes checklist answer from any chunk beats a no from any other regardless of evidence, chunk summaries join in chunk order, walkthrough bullets deduplicate by text and cap at MAX_WALKTHROUGH_BULLETS, and the first chunk to speak wins verdict prose and per-file assessments.

Per-chunk passes (response_pipeline.py, the two pass modules, #2301)

The fourth slice moves everything that happens inside one chunk out of the orchestrator, leaving it the chunk scheduler it is meant to be.

lintro/ai/review/response_pipeline.py owns the main round-trip: invoke_chunk_review builds the prompt through PromptInputs and calls the provider once. When CLI transport hits its output-token ceiling mid-JSON, lintro/ai/review/chunk_split_retry.py splits the chunk by file into two halves and reviews each once (a single-file chunk is retried once unchanged); the split is recorded as a CoverageDegradation entry, so a re-reviewed chunk can never present as an untouched one (#2003). There is no per-call findings cap (lintro-ops milestone 0, decision A). Its sixteen inputs travel as one frozen ChunkReviewRequest rather than a keyword wall. parse_review_payload_with_recovery runs the parse ladder — parse, then at most one schema-reminder retry when the timeout budget allows one, then recovery of the prose as unstructured findings — so a paid-for answer is never discarded (#1853); it folds the retry’s usage in through merge_response_usage. payload_to_partial turns the parsed payload (findings and flagged_files only, lintro-ops milestone 0) into the ChunkReviewPartial the merge layer consumes.

lintro/ai/review/question_pass.py owns the per-PR questions (#2720): generate_run_questions asks the model once per run, over the redacted whole-PR diff fitted to review_synthesis_diff_tokens plus the PR title and body, and every chunk shares the answer as “consider” items beside the rubric. A failed pass degrades the run to the rubric alone and is recorded once as GENERATED_QUESTIONS_FAILED. lintro/ai/review/adversarial_pass.py owns the depth-3 sweep: run_adversarial_pass returns findings and usage only, and degrades to usage alone when the answer is malformed.

The provider-call seam (lintro/ai/review/provider_call.py, #2301)

All three built-in passes issue their provider call as provider_call.call_ai(...) — they import the module, never the function, so the name is resolved on lintro.ai.review.provider_call at call time. That makes it one documented hook: patching lintro.ai.review.provider_call.call_ai intercepts every call the built-in review makes at any depth, and nothing below it reaches a real provider. The custom-agent runner and the cross-chunk synthesis pass are separate, independently stubbed passes and keep their own seams.

Chunk fan-out and incremental coverage (#2301)

The final slice moved the chunk scheduler out of the orchestrator. lintro/ai/review/chunk_runner.py owns the bounded-concurrency fan-out: a semaphore capped at ChunkRunPlan.max_parallel_calls (forced to 1 under a cost cap so the resume queue cannot invert), the sibling harvest that keeps completed chunks when a cost cap, timeout or SIGTERM stops the run, and the per-chunk queued/in-flight timing split. lintro/ai/review/chunk_pass.py owns one chunk’s passes end to end. lintro/ai/review/incremental_coverage.py owns the SIGTERM insurance: checkpoint_writer returns the callback the fan-out invokes after every completed chunk, writing a numbered coverage part when LINTRO_REVIEW_STATE_DIR is set and logging (never raising) when a part cannot be written.

With every mover landed, the orchestrator’s re-exports are gone: external callers import run_review, run_review_async and guard_changed_paths from lintro.ai.review.orchestrator, and everything else from the module that defines it (resolve_review_chunks from run_planning, parse_review_response from merge, and so on). Behaviour is unchanged throughout and the #2298 goldens pass without regeneration.

Module-size ratchet (#2301, closing slice)

The last slice split the seven remaining modules in lintro/ai/review/ that were over 500 lines. Each split follows the same rule the earlier slices did — move the functions verbatim, keep the public names importable from the module that had them, change nothing a golden can see:

Module splitSibling it gainedSeam
orchestrator.pyrun_execution.pyThe facade and the three steps vs. the provider-call sequence
custom_agents.pycustom_agent_types.py, custom_agent_parsing.pySchema types vs. front-matter parsing vs. discovery/selection
agent_prompts.pyagent_prompt_text.pyWrapping/fencing/titles vs. the prompt bodies and panels
finding_matcher.pyfinding_pairing.pyIdentity and the round walk vs. per-fingerprint pairing
coverage.pycoverage_rounds.pyOne round’s classification vs. round-to-round carry-over
severity_gate.pycross_chunk_gate.pyThe P1 evidence gate vs. the cross-chunk contradiction guard
preparation.pypreparation_resolvers.pyRequest/result types and entry points vs. field resolution

The same slice bundled the last three signatures in the package that took more than 8 arguments into frozen kw_only request objects, the shape PromptInputs and ChunkReviewRequest already use:

FunctionWasRequest object
classify_files9ClassifyFilesRequest
run_custom_agent_passes10CustomAgentPassRequest
run_synthesis_pass12SynthesisPassRequest

Their PLR0913 entries left the #2291 structural baseline with them, which also took synthesis.py back over 500 lines — so its response reading (parse_synthesis_findings, deduplicate_synthesis_findings) moved to lintro/ai/review/synthesis_response.py.

tests/unit/ai/review/test_review_module_sizes.py is the ratchet that keeps both true: no module directly in lintro/ai/review/ may exceed 500 lines, and no function in one may take more than 8 parameters. The GitHub-surface modules are named exceptions owned by #1974, and the chunker/ and context/ subpackages are part of the wider burn-down owned by #1995. The repo-wide [tool.lintro.module_size] gate warns at 800 lines, so it cannot hold the first line on its own, and the second is otherwise only held by the absence of a baseline entry someone could add.

Exit and error contracts

  • Exit 0 — successful review, no P1 findings.
  • Exit 1 — successful review with P1 findings.
  • Exit 2 — no review produced (REVIEW_ERROR_EXIT_CODE).

CLI JSON failures and MCP review failures both build diagnosis fields through build_error_contract.

Characterization tests

Phase 1 locks the gaps listed in ADR-0006:

  • tests/unit/test_core_ai_import_boundary.py — AC10 / #724 import edge.
  • tests/unit/ai/review/test_review_session_options.py — the single options surface: run_review declares no keyword (and no default) of its own, forwards the caller’s object unchanged, and the whole defaulted field set is pinned as a snapshot.
  • tests/unit/ai/review/test_architecture_characterization.py — CLI/MCP preparation, effective-config parity, metadata keys, error mapping, exit 0/1/2.
  • tests/unit/ai/review/golden/ — prompt bytes, chunk plan, merge output, merged ReviewResult and ReviewMetadata, as plain-file goldens (ADR-0008, #2298).
  • tests/unit/ai/review/test_cli_mcp_parity.py — CLI/MCP parity: for equal ReviewRunRequest values over one workspace the two surfaces produce an equal PreparedReview (custom agents included — the fixture ships an agent file), and the only divergence left is the named ReviewExecutionPolicy allowlist. It also pins the execute_review packing: every ReviewSessionOptions field an adapter fills is asserted against its named source (PreparedReview or ReviewExecutionPolicy), and the only fields left at a default are timeout and stop. MCP’s post-prep with_max_cost_usd clamp is the one thing it applies to the prepared review afterwards.
  • tests/unit/ai/review/test_architecture_characterization_1972.py — gap coverage: config-resolution idempotence, the shared ReviewSessionOptions fields both surfaces set, error-contract body parity, MCP error mapping.
  • tests/unit/ai/test_effective_config_parity.py — one resolver: for identical resolver inputs, check, fix, review CLI, MCP and doctor resolve identical values and sources, and the two cap rules stay split (CLI/env may raise or lift; MCP’s per-call argument only clamps).

File-level resume (#2154 / ADR-0007)

Coverage is keyed (path, normalized patch hash) and stored in workflow artifacts (CI) or .lintro-cache/ai/review-state/ (local). A quiet re-review of an already-covered HEAD makes zero provider calls. Partial coverage forces ReviewVerdict.INCOMPLETE; lintro review still exits 0/1 from findings, and classify_review_outcome.py reddens the check.

See ADR-0007 and ai-review-report.md.