TimZander / TimZander/claude

deep-review: pre-flight deterministic scan to feed mechanical findings into Step 2

Open
#181 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
Python
Stars
0
Forks
1
Avg merge
1d 3h
Merged PRs (30d)
7

Description

Goal

Add a pre-flight script to deep-review that runs the target repo's already-configured deterministic tooling (linters, analyzers, secret scanners, diff coverage), scopes the output to the reviewed diff, and passes the results into the Step 2 agents as known findings — so opus tokens go to what tools cannot see.

Background

deep-review invokes zero deterministic tooling today. The command markdown never mentions lint, analyzers, compilers, or coverage; every finding is derived by an LLM from diff text. That means:

  • Opus re-derives dead code, unused imports, and style drift that a linter reports exactly and for free.
  • file:line on mechanical findings is LLM-estimated rather than tool-reported.
  • Step 6 ("Leave It Better") asks agents to eyeball things analyzers are strictly better at.

Where tools beat tokens

  • Dead code — unused imports/variables/unreachable branches (IDE0005, no-unused-vars, ruff F401).
  • Hardcoded secrets — gitleaks / trufflehog. Note standards/CLAUDE.md gained a Secret Handling rule in 5100b33; an LLM scanning a diff for secrets is worse than a scanner built for it.
  • Diff coverage — coverlet + diff-cover, istanbul. This answers which changed lines are unexecuted — the real question. The Test Coverage agent currently greps for tests that merely look related, which is a proxy for this. See #182.
  • Compiler / type errors — tsc --noEmit, dotnet build, mypy. Ground truth rather than inference.
  • SAST — semgrep for injection patterns. A supplement to the Correctness agent, not a replacement — noisier and less complete.

Constraints that shape the design

  • Self-contained / portable. deep-review is a distributed plugin that runs in other people's repos, and plugin files must be self-contained at runtime. It cannot assume gitleaks, semgrep, or any toolchain exists. The script must detect what the target repo already configures (.editorconfig, .eslintrc, ruff.toml, package.json scripts, .semgrep.yml) and silently skip everything else.
  • Diff-scoped output. Running a linter repo-wide surfaces pre-existing findings the author did not cause. Output must be filtered to lines touched by the reviewed diff.
  • Read-only modes only. Never dotnet format, prettier --write, or ruff --fix — mutation is exactly what #178 exists to prevent. Check modes only.
  • CI overlap is the biggest risk to the whole idea. If the target repo already gates lint/format in CI, re-reporting those findings is pure noise the author already knows about. The value concentrates in repos where these are not gated. Consider detecting existing CI gating, or making the scan opt-in.

Proposed change

  • Add plugins/deep-review/scripts/preflight-scan.sh following the established script pattern (see plugins/typ-glyph/scripts/dependency-check.sh), per the "prefer scripts over inline markdown" rule in CLAUDE.md.
  • Invoke it from Step 1; return a compact structured list (file:line|rule|message).
  • Pass the list into the Step 2 agent prompts as context: these mechanical findings are already known — do not re-derive them; focus on what tools cannot see.
  • Ship a smoke test alongside the script (usage error + happy path), per the repo's script-testing rule.
  • Degrade silently: no configured tooling detected → no output, review proceeds exactly as today.

Acceptance criteria

  • In a repo with no configured tooling, /deep-review behaves identically to today.
  • In a repo with a linter configured, mechanical findings appear with tool-reported file:line and are not duplicated by the LLM agents.
  • No tool is ever invoked in a mutating mode.
  • Findings from pre-existing violations outside the diff do not appear.

Why the cheap findings still matter

An earlier framing of this issue argued the payoff was small because linters only remove the cheap findings while correctness and design taste — the expensive reasoning — remain untouched. That framing is wrong about where the cost actually sits.

The marginal cost of an opus subagent is dominated by fixed overhead: system prompt, tool schemas, and ingesting the full diff. Step 2 passes the full diff to all four agents, so a large diff is ingested 4x before a single finding is produced. An opus agent that finds nothing is still expensive. With a 1M context window the ceiling on that overhead is high, which makes it easier to stop noticing.

So the payoff is not measured in findings removed. It is measured in remit removed:

  • Shrinking an agent's remit shrinks its prompt and the reasoning it must do over the same ingested diff.
  • If a deterministic scan covers enough of an agent's concern, the agent's tier can drop (#182) — or the agent may not need to be spawned at all, which saves the entire fixed overhead rather than a slice of output tokens.
  • Precision is still a genuine win on its own: tool-reported file:line beats an LLM's estimate.

The real question for implementation is therefore not "how many findings does this replace?" but "does this let us drop or downgrade an agent?" Evaluate it on that basis.

Related

  • #85 — deep-review token-usage optimization. Overlaps in motivation but not method: #85 tiers models, this issue removes work from the models entirely. Its Phase 4 should rank "can we not spawn this agent at all?" above "can this agent be cheaper?" — this issue is how that becomes possible.
  • #86 — local LLM triage for the review pipeline. Same cost problem, third solution. Supplies the empirical overhead numbers (~75k tokens/agent, ~444k per review burst).
  • #180 — .editorconfig for team standards; produces diagnostics this scan would consume rather than re-derive.
  • #178 — read-only reviewer agents; shares the "never invoke a mutating mode" constraint.
  • #93 — documents objectively-real defects deep-review missed on opus (e.g. dynamic cast without InternalsVisibleTo → runtime RuntimeBinderException). Deterministic tools have perfect recall within their categories, which is the gap this issue targets. Note #93's headline hit-rate is measured against one reviewer across two PRs — directionally useful, not settled.
  • #182 — closed as duplicate of #85.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with plugins/typ-glyph/scripts/dependency-check.sh and the plugin runtime rules in CLAUDE.md. Trace how deep-review invokes Step 1 and builds Step 2 prompts, then inspect the repository's script-testing pattern. Done means a self-contained smoke-tested preflight script reports only configured, read-only, diff-scoped findings and stays silent when no tooling is available.

Written by the indexing model from the issue text.

Assessment

Tech stack
shell
Domain
testing, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.