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
| Concern | Owner today | Target |
|---|---|---|
Effective ResolvedAIConfig for one invocation | resolve_effective_ai_config() (#2299), the one caller of AIConfig.resolve_from_mapping | Same resolver; #1923 extends it |
| Invocation transport / timeout / cost-cap overrides | AICliOverrides on that resolver, for lint and review alike | Same resolver pipeline (#1970 / #1923 / #2024) |
| Monotonic cost-cap clamp | MCP adapter (resolve_budget_policy) | Shared domain prep; adapters keep policy |
| Diff review preparation | prepare_review / execute_review (#2300) | Done (Phase 3) |
| Review execution facade | run_review / run_review_async | Done (Phase 4); internals split across modules |
Provider client aclose() API | BaseAIProvider.aclose() and provider overrides (#1885) | Provider-side only in #1885 |
| Provider close call-site wiring | ReviewSession 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:
- 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 asAICliOverrides, which may change values and stampflagprovenance, 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. - Each adapter gates on
ai.reviewbeing enabled (adapter-specific error shape) and builds aReviewRunRequestfrom its own argument surface. prepare_reviewthen does the deterministic, provider-free work once: apply the run’s timeout and the transport profile, collect the review context (honouringai.exclude_pathson both surfaces), classify changed files, select and format the checklist, optionally build the lint digest, resolve sensitivity, and resolve custom agents from the request’sCustomAgentMode(both adapters forwardreview.custom_agents).- Each adapter constructs its own provider — so each labels its own construction
failure — and calls
execute_review, which is the singlerun_reviewcall site. Lifetime is not the adapter’s: since #2302 the run enters aReviewSessionthat closes that provider, and any provider a custom agent’smodeloverride builds, exactly once on every exit path. An adapter must not reuse the provider afterexecute_reviewreturns. - 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:
| Step | Module | What it owns |
|---|---|---|
| Plan | lintro/ai/review/run_planning.py | Sensitivity policy, diff budget (incl. the CLI ceiling), chunks, resume plan, agent selection |
| Run | lintro/ai/review/run_execution.py | The provider-call sequence and the two finalizers (completed / gracefully stopped) |
lintro/ai/review/chunk_runner.py | Bounded-concurrency fan-out, graceful stops, incremental coverage checkpoints | |
lintro/ai/review/chunk_pass.py | One chunk’s depth-1/2/3 passes | |
| Report | lintro/ai/review/result_assembly.py | Totals, 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 split | Sibling it gained | Seam |
|---|---|---|
orchestrator.py | run_execution.py | The facade and the three steps vs. the provider-call sequence |
custom_agents.py | custom_agent_types.py, custom_agent_parsing.py | Schema types vs. front-matter parsing vs. discovery/selection |
agent_prompts.py | agent_prompt_text.py | Wrapping/fencing/titles vs. the prompt bodies and panels |
finding_matcher.py | finding_pairing.py | Identity and the round walk vs. per-fingerprint pairing |
coverage.py | coverage_rounds.py | One round’s classification vs. round-to-round carry-over |
severity_gate.py | cross_chunk_gate.py | The P1 evidence gate vs. the cross-chunk contradiction guard |
preparation.py | preparation_resolvers.py | Request/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:
| Function | Was | Request object |
|---|---|---|
classify_files | 9 | ClassifyFilesRequest |
run_custom_agent_passes | 10 | CustomAgentPassRequest |
run_synthesis_pass | 12 | SynthesisPassRequest |
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_reviewdeclares 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, exit0/1/2.tests/unit/ai/review/golden/— prompt bytes, chunk plan, merge output, mergedReviewResultandReviewMetadata, as plain-file goldens (ADR-0008, #2298).tests/unit/ai/review/test_cli_mcp_parity.py— CLI/MCP parity: for equalReviewRunRequestvalues over one workspace the two surfaces produce an equalPreparedReview(custom agents included — the fixture ships an agent file), and the only divergence left is the namedReviewExecutionPolicyallowlist. It also pins theexecute_reviewpacking: everyReviewSessionOptionsfield an adapter fills is asserted against its named source (PreparedRevieworReviewExecutionPolicy), and the only fields left at a default aretimeoutandstop. MCP’s post-prepwith_max_cost_usdclamp 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 sharedReviewSessionOptionsfields 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.