# The 2.0 rewrite, phase by phase
drep 2.0 replaced a Python package with a Rust binary over nine phases. This is
the development narrative for each one: what landed, what it replaced, and the
defect that produced each rule. The plan those phases followed, the module map
and the invariants are in [rust-migration.md](rust-migration.md); the released
summary is the 2.0.0 entry in [../CHANGELOG.md](../CHANGELOG.md).
It lives here rather than in the changelog for a mechanical reason. cargo-dist
parses the released version's changelog section and embeds it *twice* in the
plan manifest, which the release workflow passes to the Homebrew publish job as
one environment variable. Linux caps a single environment variable at 128 KB, so
an 87 KB changelog section made `execve` fail with E2BIG and the formula was
never pushed. See the note in CLAUDE.md.
### Rust rewrite - 2026-08-19 - Phase 8: delete Python
The Python package is gone. `drep/`, the pytest suite, `pyproject.toml`,
`uv.lock` and `scripts/install.sh` are deleted, along with everything that only
existed to ship or serve them: the Dockerfile and compose file, the CI and
docker-publish workflows, the sphinx `docs/api/` tree, and the 1.x docs
(`llm-setup`, `roadmap`, `multi-language-analysis`,
`rust-optimization-analysis`, `SECURITY`). Nothing was yanked from PyPI. 1.3.0
stays up and nothing new is published there.
`README.md` went from ~900 lines describing webhooks, platform adapters,
docstring generation and a metrics dashboard to 228 describing a commit gate.
`docs/technical-design.md` was rewritten as the architecture of the Rust
binary, and now says what the shape is while `CLAUDE.md` says why it cannot
change. `CLAUDE.md` itself went from 1071 lines to 438: the invariants block
survives intact, everything around it was 1.x.
The gate needed rewiring, not just editing. Every hook in
`.pre-commit-config.yaml` ran out of `./venv`, and `.git/hooks/pre-commit`
named `./venv/bin/python3.14` as the interpreter that drives pre-commit itself,
so deleting the venv would have silently disarmed the gate rather than breaking
it loudly. `pre-commit` now lives on PATH (`uv tool install pre-commit`), the
hooks were reinstalled to point at it, and the config keeps cargo fmt, cargo
clippy, cargo mutants and drep - all `language: system`, no ruff, no venv.
`tests/no_python_remains.rs` (5 tests) pins the outcome, because every one of
these fails somewhere other than here: on a fresh clone, in a contributor's
first commit, on a user's machine. No `.py` anywhere in the tree, no venv or
pytest in the commit gate, no workflow installing Python, and a README that
installs the binary the release workflow ships rather than the package that
stopped being published.
The `/simplify` pass rewrote the walk in that file. It was a hand-rolled stack
with its own list of directories not to descend into, which had already drifted
from `files::is_ignored_dir` - the production answer to the same question -
by missing `node_modules`, `build` and `dist`. It now prunes through that
predicate, adding only `repos/` and `external/`, which are clones of other
projects fetched for manual testing and hold 129 `.py` files between them.
Gitignore stays off: a stray `.py` that something later gitignores is still a
Python file in the tree.
`tests/ground_truth_walk.rs` had to be updated rather than deleted, and it is
the one place the deletion showed up as a test failure. It walks *this*
repository to prove the walk survives contact with a real tree, and its ground
truth was the Python one: it asserted `drep/cli.py` is found and that the walk
never descends into a 222MB `venv/`. Both statements are now about a tree that
does not exist. It asserts `tests/cli.rs` and a 15GB `target/` instead, and the
gitignore assertion lost `.pytest_cache/` and kept `.claude/` - a directory
that no longer exists cannot fail an assertion about being skipped.
### Rust rewrite - 2026-08-19 - Phase 7: cargo-dist
`cargo-dist` 0.32.0 is still current: published 2026-05-21, and both the latest
GitHub release and the latest version on crates.io. The version the migration
doc pinned holds. Its config *location* does not. `dist init` writes a
`dist-workspace.toml` with a `[dist]` table, and the `[workspace.metadata.dist]`
block in `Cargo.toml` that the doc's snippet used is the older variant, the one
`dist migrate` exists to convert. The snippet has been corrected in place.
A release builds four targets, each on a runner of its own architecture:
`aarch64-apple-darwin` on macos-14, `x86_64-apple-darwin` on macos-15-intel,
`x86_64-unknown-linux-gnu` on ubuntu-22.04 and `aarch64-unknown-linux-gnu` on
ubuntu-22.04-arm. Nothing cross-compiles, so no `cross` container and no zig in
CI. Locally that is the one sharp edge: a bare `dist build --artifacts=local`
tries to produce every target from this machine and asks for `cargo-zigbuild`,
where CI passes `--target=<triple>` per runner.
The Homebrew formula is named `drep` while the crate stays `drep-ai`, which
carries the suffix because `drep` is taken on crates.io exactly as it is on
PyPI. A tap is namespaced by its owner and has no such conflict, so the formula
matches the binary: `brew install slb350/tap/drep`.
`[profile.dist]` adds nothing to `[profile.release]`. `dist init` writes it as
`inherits = "release"` plus `lto = "thin"`, which is its own build-time default
rather than a judgement about this crate, and it would have reverted the fat
LTO, the single codegen unit and the `strip` that `[profile.release]` sets for
the only binaries anyone installs. `dist init` leaves an existing
`[profile.dist]` alone, verified by running it twice, so the pruned profile
survives regeneration.
`tests/release_config.rs` pins five facts across the two generated files, on
the same reasoning as `tests/published_hooks.rs`: nothing else in the suite
reads them, and the first sign of a mistake is a release that already happened.
Four are the config (the target list, the installer pair, the tap and the
publish job, the formula name); the fifth compares `cargo-dist-version` against
the `dist` release URL baked into `release.yml`, which catches the config being
edited without `dist init` being re-run - a release planned by a version of
`dist` that never saw the change.
The `/simplify` pass moved the comment-stripping reader out of both test
files. `published_hooks.rs` had it first and `release_config.rs` transcribed
it, which is the duplication `src/test_support.rs` exists to prevent - except
integration tests are separate crates and cannot see a `pub(crate)` module, so
their sharing point is `tests/common/mod.rs`. It also split the assertion
style: the two generated files stay textual, and the `[profile.dist]` check now
parses `Cargo.toml` and compares the key set, because that file is hand-edited
here and reformatting the line is not the mistake being guarded against.
Verified without cutting a release. `dist plan` emits the four archives plus
`drep-ai-installer.sh`, `drep.rb`, `source.tar.gz` and `sha256.sum`, with
`README.md`, `CHANGELOG.md` and `LICENSE` in each archive.
`dist build --artifacts=local --target=aarch64-apple-darwin` produced
`drep-ai-aarch64-apple-darwin.tar.xz` in 45s; the extracted binary is a 3.5 MB
arm64 Mach-O that reports `drep 2.0.0-alpha.0`. `dist build --artifacts=global`
renders the formula with a URL for all four platforms.
`.github/workflows/release.yml` is a new file and `rust.yml` is untouched, so
the mutation sweep stays the local full sweep it has to be.
**The tap and its token are still missing**, and the version number is what
hides it. `slb350/homebrew-tap` does not exist and `slb350/drep` has no secrets,
so `publish-homebrew-formula` would fail at checkout - but dist gates that job
on `!announcement_is_prerelease`, and `2.0.0-alpha.0` is a prerelease, so it
skips. An alpha tag would release cleanly today and push no formula. The first
stable tag is where both prerequisites become load-bearing.
### Rust rewrite - 2026-08-19 - Phase 7: what drep installs now runs `lint-docs`
Three wiring gaps between the command Phase 6 shipped and the hooks drep hands
to other people. All of them had the same shape: `lint-docs` worked, and
nothing that installs itself ran it correctly.
`drep init` wrote a pre-commit hook that was `exec drep check --staged` and
nothing else, so a repository gated by drep's own installer had no markdown
gating at all. Invisible here, because this repository's `.pre-commit-config.yaml`
ran `lint-docs` through the Python venv - and `src/cli/lint_docs/mod.rs`
claimed in its module doc that the command runs on every commit, which was true
of exactly one repository. The hook now runs `drep lint-docs --staged --fail-on
error` before `drep check --staged`: rule-based first at ~10 ms, so an
unclosed fence does not cost an LLM round trip, and its status propagates.
`--staged` is new, and it needed the diff layer to stop deciding the file class
for its callers. `diff::staged_files` hardcoded `is_scan_target` -
registered-language sources - so the only way to lint the staged markdown was
to lint the whole repository and hope the noise was tolerable. The predicate is
a parameter now; `check` passes `is_scan_target`, `lint-docs` passes
`is_markdown`, and the two classes stay disjoint the way Phase 6 drew them.
Staged mode deliberately does not route through `files::expand_named`: that
expander resolves an *empty* path list to `root`, which is what makes bare
`drep lint-docs` mean "this tree" and would have turned "this commit touches no
markdown" into "lint every document in the repository", on every commit.
`--fail-on <severity>` joins `--strict`, matching `check`'s vocabulary, and
`--strict` becomes the shorthand for `--fail-on info` rather than a second
mechanism. The published `drep-lint-docs` hook shipped `--strict`, written
against 1.x where the doc checks carried no severity: under the Phase 6 scale
that blocks on any finding, which over this repository is 24 findings across
the top-level docs, every one of them `info`. A consumer adopting that hook has
commits blocked by line length. It now ships `--fail-on error`, which is the
one check that changes how the rest of the document renders.
Two smaller things fell out of using it. The renderer grew a third footer:
`--fail-on error` over this repository prints two dozen findings and exits 0,
and "24 issue(s) found." is the same line a blocking run prints - a hook log
that reports problems and passes has to say why, so it now reads "(none at or
above error)". And the published hooks moved from `language: python` to
`language: rust`, since the PyPI package they install disappears in Phase 8;
`tests/published_hooks.rs` pins both facts, because that file's consumer is
somebody else's commit gate and nothing else in the suite reads it.
The `/simplify` pass found the renderer re-deriving the gate. Its footer asked
"did any finding reach the threshold" a second time, from the findings and the
threshold, which is the mistake `check` documents on `CheckOutcome::exit` and
fixed - and it was already wrong in one case, because `gate` gives failures
precedence and the footer did not, so a run with both an unreadable file and
blocking findings printed the blocked phrasing under exit 2. `LintOutcome` now
carries a `Gating` the gate computes once, and the renderer reports it rather
than recomputing it. The comparison itself became
`findings::any_at_or_above`, shared with `check`, which had it written out
separately. The same pass finished the diff-layer generalization: the *hunks*
queries still hardcoded `is_scan_target` while the *names* query took it from
the caller, so `staged_hunks`, `hunks_since` and `hunks_between` now take the
predicate too.
This repository's own hook now runs the 2.0 binary at `--fail-on error` too,
rather than the Python one report-only. Report-only was the old answer to
`long_line` contradicting `MD013: false`, and it meant the one finding that
breaks a document did not block either.
### Rust rewrite - 2026-08-18 - the mutation sweep moves off the laptop
`scripts/mutants-remote.sh` syncs the tree to another machine over SSH, runs
`scripts/mutants-run.sh` there and propagates its exit code, mirroring
`target/mutants/` back so a surviving mutant is read where the fix gets written.
The pre-commit hook goes through it; CI still calls `mutants-run.sh` directly,
because a GitHub runner cannot reach the LAN. An unreachable host falls back to
a local run with a warning on stderr - a commit gate that silently skips itself
because the network blipped is worse than a slow one.
Measured on the 12 mutants of `src/docs/fence.rs`: 1m54 locally at `-j 4` with
the machine pinned for the duration, 39s end to end on a 32-thread box,
including the sync in both directions. `-j` is now `MUTANTS_JOBS` on
`mutants-run.sh` so both callers can tune it, but raising it is a trap: the same
scope took 38s at `-j 4`, 54s at `-j 8` and 72s at `-j 16`, never exceeding
2200% CPU of a possible 3200. Every job copies the tree *including* `target/`,
which is what keeps its builds warm, so more jobs multiply a multi-gigabyte copy
before the first mutant runs. The run is I/O-bound, not CPU-bound.
The `/simplify` pass moved two things down a layer. `target/mutants` was the
same string literal in six places across three scripts, where a stale copy does
not error - it just stops finding `missed.txt`; `scripts/mutants-common.sh` is
now the one definition. And the transport script no longer scans its arguments
for `--in-diff` to discover which file to ship: `mutants-staged.sh` names it in
`MUTANTS_EXTRA_FILES`, because "move these bytes" belongs at that layer and
cargo-mutants' argument grammar does not. The efficiency pass found ~64MB of
caches (`.mypy_cache`, `.pytest_cache`, `.ruff_cache`, stale `mutants.out.old`)
syncing on every commit against a Rust payload of about 1MB, and two `ssh
mkdir -p` round trips that `rsync --mkpath` does as part of the transfer.
The ETXTBSY fix in the commit before this one is what made any of it usable: the
suite failed one Linux run in three, and inside a mutation sweep that does not
read as a flake - it records a mutant as caught that nothing actually caught.
### Rust rewrite - 2026-08-18 - Phase 6: `lint-docs`, and markdown gets a home
Ported `drep/documentation/analyzer.py` to `src/docs/`, and closed the `.md`
hole Phase 5 left open.
Ten checks, by the `type=` strings 1.x emitted: `bare_url`, `empty_heading`,
`link_syntax_invalid`, `long_line`, `missing_space_after_heading`,
`multiple_blank_lines`, `tab_character`, `trailing_blank_lines`,
`trailing_whitespace`, `unclosed_code_fence`. The names are pinned against an
independently written list, so a rename fails a test rather than silently
breaking anyone who scripted against them.
`src/docs/fence.rs` derives fence state once per file and every check that needs
it consults that one answer. The tests state each check's fence position in a
table whose completeness is asserted, so a new check cannot be added without
deciding what it does inside a code block. Sources split across `fence.rs`,
`lines.rs`, `links.rs` and `blocks.rs` by what a check needs to see.
No regex dependency. The five patterns 1.x compiled are hand-written scanners
over `&[char]`, which keeps every reported column a character offset rather than
a byte offset, and keeps startup at ~10 ms with nothing to compile.
Three divergences from 1.x, each because the Python's advice was wrong:
- A link reference definition's URL is not a bare URL. `[1.1.3]: https://...`
declares a target that is supposed to be bare, and "wrap it as `[text](url)`"
breaks the definition. This repository's own CHANGELOG footer is nine of them.
- `tab_character` now respects code fences, because "replace tabs with spaces"
stops a ```` ```make ```` sample being a Makefile. A check that cannot give
correct advice about a line should not fire on it. `trailing_whitespace` stays
fence-blind for the mirror reason.
- `suggestion` is advice rather than a literal replacement. 1.x's `replacement`
field held a rewritten line half the time and a sentence the rest, because a
draft-PR autofix consumed it. 2.0 has no autofix.
Severity follows one question: does it change how the document renders? An
unclosed fence turns every line below it into code, so it alone is `error`. A
heading or link that renders wrong is `warning`. Whitespace and line length
render identically, so they are `info`. That is what keeps `--strict`
calibratable; on a scale where a trailing space blocks a commit, the gate gets
switched off.
#### The `.md` hole
`files::is_scan_target` accepted `.md`, but no language claimed it, so `drep
check README.md` read the file, found no deterministic tool and no LLM language,
and printed "No issues found." A file drep declined to analyze, reported as
clean, on a path the user named explicitly.
Two fixes were available. Routing markdown through `check` to the doc checks was
rejected: it needs a third gating category, since the doc checks are
deterministic but are drep's opinion rather than the project's configured tool,
so "tool findings always block" does not extend to them. It also has no good
answer in the diff modes, where a file arrives as hunks and a whole-file check
like `unclosed_code_fence` would see an odd delimiter count on every partial
view.
So `check` and `lint-docs` own disjoint file classes. `is_scan_target` is
registered-language sources, `is_markdown` is markdown, and nothing satisfies
both. A path the user *names* outside the running command's class becomes a new
`FailureReason::Unsupported` (JSON `kind: "unsupported"`) carrying the extension
and, where another command handles the type, what to run instead. A directory
walk that finds nothing analyzable stays legitimately empty, which is the same
distinction `resolve_paths` already drew for a non-existent argument. The rule
generalised for free: `drep check notes.txt` had the identical bug.
`lint-docs` exits 2 for a file it could not read whether or not `--strict` was
passed, since that is the absence of analysis rather than a finding. Findings
exit 1 only under `--strict`, matching the report-only default this repository's
pre-commit hook relies on. Its startup path touches `docs` and `files` and
nothing else.
#### Verification
Against this repository, `drep lint-docs` reports 75 findings across the tracked
docs in ~10 ms, with no false positives on inspection. An independent oracle
confirms the fence invariant: `rg '^#{1,6}[^#[:space:]]'` finds seven lines that
look like malformed headings, `#!/bin/bash` in `technical-design.md` and six
`#[pyfunction]` attributes in `rust-optimization-analysis.md`. All seven sit
inside code fences and drep reports none of them.
The mutation gate found ten survivors on the first pass, all in `links.rs`. An
exhaustive differential over generated inputs separated them: four were missing
tests (blanking a code span with text before it, two consecutive nested brackets
in link text, and both arms of the reference-definition scan), and five were
equivalent mutants where a `+ 1` was unobservable because the position had
already been blanked or could not hold the character being searched for. The
four gaps got tests; the five unobservable expressions were restructured away,
which also put the offending bracket counts into the `link_syntax_invalid`
message where a reader can use them.
The `/simplify` pass then found that the fix had been applied twice rather than
once. Both commands re-walked their argument list with their own `exists()` /
`is_file()` probes to rediscover what `expand_paths` had already decided and
thrown away, and both reconstructions were lossy the same way: a named fifo
satisfies neither `is_file` nor `is_dir`, so it was dropped by the expander,
missed by the reconstruction, and reported as a clean run. That is the same
banned move the phase was written to close, still open one file type over.
`files::expand_named` now returns `Expansion { targets, rejected }` and owns the
no-arguments default as well, `files::owning_command` replaces each command's
hardcoded pointer at the other, and `cli::render` and `text::excerpt` absorb the
transcribed copies of the failure block and of the untrusted-text bound. The
`bare_url` copy of `excerpt` truncated without stripping control characters,
which is the one thing that function exists for, so a URL carrying an escape
sequence reached the terminal intact.
The efficiency pass measured rather than guessed. Holding a `Vec<char>` per line
was 43% of the analysis pass, so `Line` no longer carries one and `analyze`
fills two reused buffers instead; a first-character guard in `find_url` removed
another 19%, since the URL scan ran over every character of every prose line to
find 65 URLs. Three suggestions were checked and disconfirmed: fusing the four
bracket counters into one loop is a 2.5x pessimization because the separate
counts autovectorize, the two per-file allocations in `analyze` are both read,
and the double sort had already been narrowed.
Two pre-existing test defects surfaced while updating the suite.
`tests/ground_truth_walk.rs` wrapped its explicit-path assertion in
`if ignored.exists()` against one review file under the gitignored `.claude/`,
so on any fresh clone - CI included - the guard was false and the test asserted
nothing. `files::tests::expand_paths` already covers that property hermetically,
so the vacuous copy is gone rather than rewritten. The `unanalyzed` JSON tag
test's exhaustive match did its job: adding `Unsupported` failed to compile
there, in the file holding the sample list.
Testing: 556 passing, up from 445. Clippy-clean, rustfmt-clean, mutation-clean.
### Rust rewrite - 2026-08-18 - open-agent-sdk 0.8.0: the server says why it stopped
Upgraded to open-agent-sdk 0.8.0, which surfaces `finish_reason` for the first
time. `query()` now yields `StreamEvent`, ending in exactly one
`Finish(FinishReason)`.
That replaces a heuristic with a fact. The previous commit retried *any*
response with no JSON, twice, because the body could not say why it was
JSON-free. The server can:
- **`Length`** - the model hit its output token cap before emitting JSON. drep
sends no `max_tokens`, so the cap is the server's and the same request hits it
every time. This is the genuinely deterministic case the original Phase 3 rule
was reaching for; it just identified it by "no JSON in the body", which is the
wrong proxy. Not retried, and reported as something a user can act on: the
file is too large for this model in one pass.
- **`ContentFilter`** - the provider refused the payload. Also not retried, with
its own message, because "too big" and "refused" want different actions.
- **`Stop`, `Unspecified`, anything else** - nothing rules out a different
answer, so the bounded retry stays. `Unspecified` is deliberately distinct
from `Stop`: several OpenAI-compatible servers never report a reason, and "no
information" is not "finished normally".
New `LlmError::ModelStopped { finish, message }` and
`FailureReason::ModelStopped` (JSON `kind: "model_stopped"`), shaped like
`Transport { status, message }` - the server's own word kept as a machine tag
beside the human sentence.
**It neither fails over nor demotes**, and that is the same rule as a 400: a
token cap is a property of the request, not the endpoint, so a second provider
cannot fix it and remembering it would stop the chain for every later file.
Two smaller things fell out of the upgrade. `FinishReason::as_str()` already
exists, so a hand-rolled wire-name mapping was deleted before it could drift.
And the mutation gate flagged two match arms that were behaviourally identical
to the wildcard beneath them - an enumerated "everything else is retryable" arm
and a standalone `StreamEvent::Reasoning(_) => {}`. Both were collapsed:
`FinishReason` and `StreamEvent` are `#[non_exhaustive]`, so a wildcard is
mandatory, and an arm indistinguishable from it is dead code rather than
documentation.
**Testing: 445 passing**, clippy-clean, rustfmt-clean, mutation-clean.
**Not done, deliberately.** `finish_reason` also makes `Truncated` checkable:
today drep infers truncation from brace-balancing alone, so a response that
needed closing braces *and* stopped normally is reported as truncated when it is
really malformed. Wiring that in cleanly needs either a reshaped `Extracted` or
a `capped: bool` passed beside it - and the latter is the exact shape
`code_quality` documents as a past bug, where `(Extracted::Truncated(v), false)`
compiled and reported a truncated file as clean. Both outcomes are already exit
2, so this is diagnostic precision, not a correctness gap.
### Rust rewrite - 2026-08-18 - a response with no JSON is not the deterministic case
Phase 3 split LLM outcomes two ways: retry transport failures, never retry a
parse failure. It had one case too few, and the missing one cost three of the
four attempts at pushing Phase 5c.
The rule "a non-empty body that yields no JSON must never retry" was justified
as *the same prompt truncates the same way*. But truncation is
`Extracted::Truncated` - a different branch, where brace-balancing recovers a
prefix and returns it. A body with **no JSON at all** did not truncate an
answer, it never produced one, and that does not repeat: each push died on a
different file, and every failing file analyzed cleanly when asked again on its
own. The rule borrowed truncation's justification for a case truncation does
not cover - the same shape of error as the empty-response bug fixed in
`b20ef9f`, which sat one line away.
Three outcomes now, not two: an **empty** body is `Transport` (retried by the
SDK, may fail over); **no JSON at all** is `Unparseable`, retried up to
`NO_JSON_ATTEMPTS` but never reclassified; a body that parsed only after
brace-balancing is `Truncated` and never retried.
**The no-JSON retry deliberately lives in `complete_json`, not in the SDK's
retry layer.** Returning `Err` and letting the SDK retry would work and then
surface as `Transport` once attempts ran out - which fails over to the next
provider *and* demotes this one for the rest of the run. A model that answered
in prose has told us nothing about the endpoint. The SDK's own retry still runs
inside each pass, so transport failures stay with the layer that classifies
them.
**`LlmError::Unparseable` carries the body now.** It was the constant string
"response contained no parseable JSON", so every occurrence looked identical
and nothing could tell a refusal from a prose preamble from reasoning that
leaked into the content channel. The excerpt is bounded to 200 characters and
control characters are replaced - it is model output and it lands in a terminal.
Two things `open-agent-sdk` still hides, found while diagnosing this and worth
recording: `StreamAccumulator` consumes `finish_reason` internally and never
surfaces it, so drep cannot tell `"stop"` from `"length"`; and `OpenAIDelta`
deserializes only `role`/`content`/`tool_calls`, so the `reasoning` and
`reasoning_content` fields DeepSeek and OpenRouter stream are dropped as unknown
fields. Both are fixable in the SDK. Until then the excerpt is what turns the
next occurrence into a diagnosis rather than a guess.
**Testing: 439 passing**, clippy-clean, rustfmt-clean, mutation-clean including
a full sweep of `src/llm/client/mod.rs`.
- Two existing tests asserted the old rule. `a_non_empty_unparseable_body_is_not_retried`
became `..._stays_unparseable_rather_than_transport`: the request count no
longer discriminates, since both cases retry, so the *classification* is what
is pinned - and it is load-bearing, because `Transport` would fail over and
demote. `unparseable_is_never_retried` asserted the inverse of the new
contract and its coverage is now held by the bounded-retry test; it was
removed rather than inverted in place.
- The mutation sweep surfaced a separate gap: `LlmClient::temperature` could be
replaced by a constant with every test still passing. It feeds
`Provider::cache_key` while the request reads the field directly, so two
providers differing only in temperature shared a cache entry - the same shape
as the missing endpoint, request going one place and the key naming another.
### Rust rewrite - 2026-08-18 - Phase 5c: multi-provider LLM failover
`[[llm]]` becomes a real failover chain. `src/llm/chain.rs` tries each enabled
provider in turn; the analyzer calls it instead of `LlmClient` directly.
A deliberate reversal. The rewrite dropped the circuit breaker and the rate
limiter as server-shaped complexity, and failover is the same category. It was
taken for one failure that happens in practice: a local endpoint that is off
blocks every commit, because exit 2 is a hard stop by design.
**The policy.** Only transport failures advance the chain - status-less
failures (timeout, refused connection, empty body) plus 408, 429 and 5xx. A 401
or 403 stops it: that is misconfiguration, and falling back masks it. A
non-empty body carrying no JSON stops it too, because it is deterministic. An
*empty* body does fail over: it reaches the chain only after the SDK has
already retried it, it produced zero output tokens, and a provider that keeps
answering with nothing is as unusable as one that is down.
**`enabled` flipped to default `true`.** It is an opt-out - the way to park one
entry of an ordered list. Defaulting it to `false` made declaring a provider do
nothing until you also enabled it, so a fallback added by copying the first
block minus its `enabled` line was silently inert. `Config::providers()` is now
the chain (enabled entries, file order), a disabled head falls through to the
entry below it, and a config where every entry is disabled is rejected at load
with its own error rather than at the LLM boundary.
**Demotion is sticky for the run, and checked twice.** "Does the chain advance"
and "is this failure remembered" are separate questions, and the sticky set is
the wider one: every endpoint-level failure is remembered, a 401 included,
because a stale key answers the same way for every file and re-handshaking once
per file is pure wall-clock on a gate that will exit 2 regardless. The
remembered reason is then replayed through the failover rule, so a demoted 401
still *stops* the chain rather than being silently routed around. The first file
that fails marks that provider down; later files skip it, so a dead endpoint
does not cost forty-nine files the full backoff schedule each. Files are
analyzed concurrently, so the check *before* the limiter only stops files that
had not started - everything already queued passed it before the first failure
landed. A second check after the slot is granted bounds the waste by
`max_concurrent` instead of by the file count. Each provider also carries its
own limiter now, because `max_concurrent` is a per-entry field and the slot
represents work against one endpoint.
**The cache key moves with the provider.** It is computed inside the loop, once
per provider tried, and `Served::key` names whoever answered. Keying the head
and letting the fallback serve would file that answer under a key it did not
come from, and a later run with the head restored would get a hit that never
came from the head. This is the bug that would have shipped green under every
"did the loop advance" test.
**Reporting.** Text output is silent about providers on the happy path and
prints every provider that served - model, endpoint, file count - once the
chain falls through, because a silent switch from a local endpoint to a paid
API is a cost surprise. JSON always carries a `providers` array.
`FailureReason::ChainFailed` carries every provider's reason including the
skipped ones, under "no LLM provider analyzed this file" - phrased by what
happened rather than by a count, since a 401 at the head stops the chain and
the providers below it are deliberately never asked. It appears **only for a
chain of two or more**; a single-provider config reports exactly what it did
before, JSON `kind` included. The trigger is the chain's length rather than the
number of failures, because those differ exactly where the structure is worth
most. `doctor` marks
disabled entries `(disabled - skipped)` and states which of three situations
the config is in: nothing enabled, one provider and no fallback, or N tried in
order with the 401/403 exception named.
**Found by drep's own pre-push gate, reviewing this commit.** The cache key was
built from the model and temperature but *not* the endpoint, so two providers
running the same model at different endpoints shared an entry — one open model
served locally and from a cloud provider, which is the canonical failover pair.
The fallback's answer was filed where the head would look for its own, and a
later run with the head restored was served a response it never produced: the
exact defect the per-provider key exists to prevent. Every cache-key test passed
because they all used distinct model names, so the keys differed for the wrong
reason. Composition moved onto `Provider::cache_key`, so a test cannot spell the
key a different way than production does — spelling it out by hand is what made
the tests agree with the bug.
The same review pass also fixed: `max_concurrent = 0` building a permit-less
semaphore that hangs every request forever (now rejected at load, naming the
block); `${VAR}` expansion and field validation running over *disabled*
providers, so parking the cloud entry refused to load the file when its key was
not exported; `doctor` reporting the pre-2.0 single-table `[llm]` shape as
"declares no `[[llm]]` provider" and pointing the user at `drep init`, which
would refuse to overwrite the file; a stray leading blank line before the
"could not be analyzed" block on runs with no findings; a present-but-non-string
`category` or `suggestion` in an LLM response being silently accepted (and
cached) rather than marking the file unanalyzed, unlike `severity` and
`message`; `doctor` warning that a *parked* provider's unset `${VAR}` would
break analysis, which after the expansion fix it no longer does — the same
doctor-disagrees-with-check failure its old narrower scanner produced, in the
opposite direction; and an `unsafe` `set_var` in an `init` test whose safety
note relied on the wrong contract; and `LlmConfig` deriving `Debug` with the
API key in the clear - `Config` derives `Debug` and holds these, so any `{:?}`
on a loaded config leaked the credential `LlmClient` already hand-writes `Debug`
to redact; `is_sticky` treating *every* transport failure as endpoint-level,
so a single oversized payload drawing a 400 demoted the provider and — since a
400 does not fail over — stopped the chain for every later file without ever
reaching the configured fallback; and `Cache::evict_if_needed` walking any
directory under the cache root while its own comment claimed it descended only
into two-hex-char shards, so the module's one destructive path could delete
files a user had placed there — the round trip now uses a preset that needs no key, and
the `${VAR}` half is asserted on the rendered text.
**Testing: 435 passing** (up from 366), clippy-clean, rustfmt-clean, and
mutation-clean on the staged diff and on `src/llm/chain.rs` swept whole.
- 21 tests in `src/llm/chain/tests/` across construction, failover policy,
cache-key movement and demotion.
- 8 in `src/analysis/tests/code_quality_failover.rs` for the analyzer join:
provider accounting, the `ChainFailed` mapping, the collapse rule, and the
actual `cache.put` landing under the serving provider's key.
- 7 in `src/cli/check/tests/failover_report.rs` for both output formats.
- 4 new `doctor` tests, 5 new config tests.
The suite was not trusted until three deliberate sabotages were run against it:
keying on the head instead of the serving provider (3 of 4 `cache_key` tests
failed), failing over on every status (2 `failover` tests failed), and deleting
demotion (all 4 `demotion` tests failed).
`unanalyzed_json.rs`'s "every variant renders a distinct kind" test was a
hand-written list that `ChainFailed` walked straight past. The expected tag is
now restated as an exhaustive `match`, so a new `FailureReason` variant cannot
be added without this file failing to compile.
### Rust rewrite - 2026-08-18 - an empty LLM response is a transport failure
The second thing drep's own gated push found. With clippy fixed, the push was
blocked again - this time by 7 of 49 files reporting:
```
LLM response was unparseable: response contained no parseable JSON
```
Re-running one of those files immediately afterwards **succeeded**, with
findings and exit 0. So the failure was never deterministic, and the retry
taxonomy had a gap that Phase 3 stated confidently in the wrong direction:
> An empty response body becomes `LlmError::Unparseable`, not an empty success
> - "the model returned nothing" is a deterministic outcome for the same
> prompt + max_tokens pair.
It is not. `run_one_query` returned `Ok(None)` for two unrelated situations -
a body we could not parse, and *no body at all* - and `Ok(None)` is precisely
the path the retry layer must not retry. So provider flakiness was being
classified as a deterministic parse failure and failed immediately, which is
the same `finish_reason='error'` that blocked three consecutive pushes under
1.x. Phase 3 split retry by failure class specifically to fix that, and then
put the empty-response case on the wrong side of its own split.
**An empty (or whitespace-only) body is now `open_agent::Error::stream`**,
which the SDK classifies retryable. That is both accurate - the stream
completed carrying no content - and nearly free, because a response with no
output tokens cost nothing to produce. A **non-empty** body that yields no JSON
still returns `Ok(None)` and still never retries: re-sending it buys the same
prose for the price of a full reasoning call.
The request count is what the tests assert. Classification alone is not enough
- an implementation that labelled the empty body correctly and still refused to
retry would pass without it.
### Rust rewrite - 2026-08-18 - `cargo clippy` never actually ran
Found by pushing. The 2.0 gate blocked its own first real push with exit 2 and
49 files reported `ToolUnavailable`:
```
src/config.rs: clippy could not run: clippy exited 1 without producing
diagnostics: error: unexpected argument 'src/analysis/code_quality.rs' found
```
`cargo clippy` checks a **crate**. It does not take file paths, and rejects one
outright. `run_tool` appends the file list to every tool's argv, so every
clippy invocation since Phase 1 has failed - meaning **the deterministic half
for Rust has never run**, on any repository, for the entire rewrite.
The contract worked exactly as designed: an unavailable tool is not a pass, so
this surfaced as exit 2 rather than as 49 files quietly reported clean. That is
the whole reason `ToolStatus::Unavailable` exists, and it is why the bug was
loud when it finally ran instead of silent forever.
Nothing in 353 tests caught it. The parser tests feed captured clippy output to
`parse_output` directly, and the `run_tool` tests use stub executables that
accept anything - both correct as far as they go, and both blind to whether the
real binary accepts the argv drep builds. Only running drep against its own
source found it.
**Fix:** `ToolSpec::accepts_files`, false for clippy alone. A tool that does not
take files is invoked bare, and its findings are then **narrowed to the files
being checked** - a whole-crate run otherwise reports pre-existing issues in
untouched code, which a commit gate cannot act on and the author cannot fix.
Verified against the real binary, not fixtures: a `clippy::len_zero` injected
into `src/files/mod.rs` is reported at the right file, line and column when
that file is checked, and checking `src/lib.rs` instead reports zero tool
findings - no leakage from the crate-wide run.
Also from that run, two findings the LLM raised on drep's own diff and which
were correct: `PAYLOAD_MAX_BYTES`' doc claimed it was "enforced **here**" in
`payload.rs` when `code_quality.rs` enforces it, and `AnalysisResult::merge`
had the first-writer-wins union written out longhand next to the
`union_failures` helper that states it.
### Rust rewrite - 2026-08-17 - Phase 5b: `drep doctor` and `drep init`
Both commands land, and this repository's own pre-push gate now runs the 2.0
binary against `drep.toml` instead of 1.x against `config.yaml`.
**`drep doctor`** answers "what will drep actually do here": languages present
with file counts, every configured tool's status straight from
`runner::tool_status` (so it cannot claim "ready" for a tool `check` will skip),
and the LLM configuration. It never fails - diagnosis is not a gate - and a
closed pipe (`drep doctor | head`) is the reader's choice rather than an error.
Two decisions worth recording:
- **The provider display reads the raw parsed file, not `config::load`.** A
fresh clone with the API key not yet exported is exactly when the report is
most useful, and `load` fails outright on an unset `${VAR}`. `load` is still
consulted, but only to surface problems the raw pass cannot see.
- **A repo with no recognised source files still gets the LLM section.** The
first spec returned early there, which answered "is my model configured?"
with silence in the repository most likely to be asking.
**`drep init`** writes `drep.toml` from a named provider preset and installs
**native git hooks** - not a `.pre-commit-config.yaml` entry. 2.0 is a single
binary with no Python runtime, so requiring the `pre-commit` framework to
install a hook is a dependency the rewrite exists to shed. The two hard-won
parts of the 1.x installer carry over unchanged: the hooks directory comes from
`git rev-parse --git-common-dir` (in a linked worktree `.git` is a *file*, so
`$REPO/.git/hooks` does not exist and the hook silently never runs), and a set
`core.hooksPath` makes git ignore `.git/hooks` entirely, so a chainer in that
directory is what keeps a repo-local hook alive.
Presets carry **no `max_tokens`**. The Python sets 100,000 for reasoning models
to stop them truncating; 2.0 sends no cap at all, so a preset setting one would
reintroduce exactly the coupling Phase 3a removed.
**`check --diff` gained `--tip`**, and it is not cosmetic. `--diff <base>`
resolves to `git diff <base>...HEAD`, and git can push a ref that is not the
checked-out one - `git push origin feature:feature` from another branch, or
`git push --all`. The hook read the pushed oid from stdin and used it only for
the deletion check, so the content actually reviewed was always `HEAD`: the
pushed branch went through unseen while a different branch was reviewed in its
place. The hook now names the pushed oid as the tip.
Three more defects in that hook, all found by review rather than by tests:
- The fallback for a branch that is new upstream was the **root commit**, so a
first push in a mature repo sent the entire history to a reasoning model.
The search is now bounded, and stops at `<remote>/HEAD`, `<remote>/main`,
`<remote>/master`, or 50 commits back.
- `origin` was hardcoded, ignoring the remote git passes as `$1`.
- Multi-ref pushes took the *last* exit code, so a ref that merely had findings
(1) downgraded an earlier ref that went unanalyzed (2). Highest wins now.
**`doctor`'s `${VAR}` scanner disagreed with `config`'s substituter.** Doctor
matched `[A-Z_][A-Z0-9_]*` while `expand_string` substitutes anything between
`${` and `}` - so `api_key = "${openrouter_key}"` produced no warning, and
because doctor suppresses `EnvVarUnset` believing it already reported it, the
user was told a config was fine that `drep check` refuses to load. One scanner
now serves both (`config::env_var_refs_in`), over the *parsed* tree, so a
`${VAR}` inside a comment no longer raises a false alarm either.
**`config_file::render` could emit unparseable TOML.** The escaper handled `\`
and `"` only; TOML also forbids literal control characters in a basic string,
so an endpoint pasted from a CRLF file wrote a `drep.toml` nothing could read -
and `write` then refused to replace it without `--force`.
Smaller, all from the same review pass: `--force` over a hand-written hook now
keeps a `.drep-backup` copy rather than destroying it (the error message from
`config_file::write` steers users straight into that flag); hooks are written
via a temp file and renamed, because a truncated-but-executable hook exits 0
and waves every push through; `set_executable` ORs `0o111` rather than `0o755`,
which was widening the mode of files in a shared directory; a `git config`
failure is no longer indistinguishable from "unset", which would have skipped
the chainer while `core.hooksPath` was in fact set; `--hooks none` does no
filesystem or git work at all; a tool shared by two languages (eslint) is named
once in the missing list rather than twice; and the dead `EMPTY_TREE` fallback
in `since_diff` is gone - a three-dot spec needs two commits, so it could only
ever turn "no commits yet" into git's opaque "Invalid symmetric difference
expression".
63 tests added (349 total), zero clippy, zero missed mutants over 89 mutants.
**On the verification.** The delegated implementation passed its own 50 tests,
`cargo clippy` and `cargo fmt` before review. Four review agents then found 30+
issues, of which the load-bearing ones were invisible to the test suite by
construction: a test asserting `!ready || line.contains("configured")` (which
fails on any machine that has `tsc`, and is vacuous on one that does not), a
unit test that re-declared the implementation's regex and therefore tested the
`regex` crate, and three `write_llm_section` branches with no fixture at all.
Breaking the implementation by hand found more: `--git-common-dir` →
`--git-dir` passed every existing test, because the two differ only in a linked
worktree - the exact invariant the port carried forward. The suite also
inherited the developer's global `core.hooksPath`, so `install` was reaching
into `~/.git-hooks` during test runs.
### Rust rewrite - 2026-08-17 - Phase 5b groundwork: config shape, size ceiling, failure detail
The three cross-cutting changes `doctor` and `init` need to exist on top of.
`drep check` behaviour is unchanged except where noted.
**`[[llm]]` is an array of tables.** `Config.llm` is a `Vec<LlmConfig>` and
`load` rejects a file that declares no provider (`ConfigError::NoProviders`) -
the LLM layer is mandatory in 2.x, so a provider-less file can never produce a
passing run, and the earliest place to say so is the place that read the file.
Only `Config::primary()` (the head of the list) is consulted today; the shape
lands now because `drep init` writes this file and Phase 5c must not change the
format underneath a file drep itself wrote. The temperature error carries the
offending provider's index, which with one provider is trivially 0 and with
three is the only way to know which one is wrong.
The discriminating test is that the old single-table `[llm]` is now a **parse
error**. Without it, a suite full of `[[llm]]` fixtures passes just as well
against a `Config` that still holds one `LlmConfig`, because TOML would feed it
the first table.
**The size ceiling moved to where the payload is built, and split in two.**
`WHOLE_FILE_MAX_BYTES` was consulted only during paths-mode input resolution,
so `--staged` and `--diff` - the two modes a commit gate actually runs in -
never saw it, and a newly-added 5 MB file reached the model whole. There are now
two constants because there were always two questions:
- `analysis::payload::PAYLOAD_MAX_BYTES` (256 KiB) is checked against the
**rendered payload**, in `code_quality::analyze_file`, which every input mode
passes through. This is the authority on "too large to analyze".
- `cli::check::input::READ_MAX_BYTES` (8 MiB) guards `read_to_string` against a
pathological file in paths mode. Nothing more.
They were one constant, and that is what made the reported number wrong: `bytes`
held the file's size on one path and the rendered payload's on the other, so
`file is too large (330102 bytes)` could name a file `ls` reports as 261900.
`FailureReason::TooLarge` is likewise split into `FileTooLarge` and
`PayloadTooLarge`, each naming what it measured. A `const` assertion, not a
test, pins that the read guard never sits below the payload ceiling - it is a
property of two literals, so it fails the build.
The comment justifying the retained pre-read filter asserted the wrong
direction: that the filter can never *accept* something the payload check
rejects. It can, and routinely does - a payload adds a header plus ten bytes of
gutter per line. The property that matters is the converse, that it never
*rejects* a file whose payload would have fit, and that is what the comment now
says.
**An oversize file is still linted.** Dropping it from `Work::by_file` also
removed it from the deterministic layer, so `drep check big.py` reported no ruff
findings for it while `drep check --staged` on the same file reported them. A
file too large for the model still has a path, and ruff reads the file itself,
so `Work::lint_only` carries it to the tool runner. It remains a failure - the
LLM never saw it - but its linters run.
**`--format json`'s `unanalyzed` entries carry structure.** Each is now
`{file, kind, status?, reason}`. `kind` is a stable tag and `status` is the HTTP
code as a number, present only when there was one, so its presence is itself the
signal. The reason it is worth the field: Phase 5c fails over on a 429 and must
*not* on a 401, and that decision cannot be made by matching on English. The
tag table lives in `cli::check::render`, which already owns the wire shape and
declines to `Serialize` `Finding` for the same reason; `FailureReason::status()`
stays on the type because it returns data rather than presentation.
**Also:** `languages::group_by_language` extracted so `check` and `doctor`
cannot disagree about what a repository contains; it borrows rather than clones,
and indexes `ALL_LANGUAGES` rather than recovering the language by pointer
comparison. `source_extensions()`/`vendored_dirs()` are `LazyLock` statics -
`files::is_scan_target` calls the first for **every entry the tree walk
touches**, and it was rebuilding a `BTreeSet` each time. `detect` no longer
allocates two `String`s per call. `AnalysisResult::failed(path, reason)`
replaces four hand-assembled copies of the one shape where forgetting the insert
reports an unanalyzed file as clean. `config.rs` split its tests into
`src/config/tests/` by topic (it had passed the 600-line limit).
10 new tests (298 total), zero clippy, zero missed mutants. Four review agents
found 30 issues across reuse/simplification/efficiency/altitude; the size-ceiling
disagreement was found independently by three of them.
### Note - 2026-08-17 — Phase 5a pushed with `--no-verify`
Four commits (`824b17f`, `8b8c959`, `76ec4e4`, `9562871`) were pushed with the
local pre-push gate bypassed. Recording why, because silently skipping the gate
is the failure drep exists to prevent and a bypass should never be invisible.
The gate blocked three consecutive attempts with exit 2. Each time the
unanalyzed set was **different**, and each time it included files the change
never touched:
| 1 | `resolution.rs` (error); four length stops: `output.rs`, `complete_json.rs`, `diff/mod.rs`, `main.rs` |
| 2 | `diff/mod.rs` (error) |
| 3 | `diff/mod.rs` (error), `resolve.rs` (length) |
`main.rs` is 70 lines. `diff/mod.rs` has not changed since Phase 4a and failed
all three, the last two with `finish_reason='error'` — not truncation at all.
The blocking condition was LLM-side flakiness, not a defect in the change.
This is the case for Phase 4 demonstrated on the project itself. 1.x sets
`max_retries: 1` with a comment that a length failure repeats deterministically
— correct for truncation, and it silently disables retry for *transport* errors
too, which is exactly what `finish_reason='error'` is. 2.0 splits them:
transport retries with backoff, truncation becomes a typed
`Extracted::Truncated` that yields its partial findings **and** marks the file
unanalyzed. Once `drep check` runs on the 2.0 binary this class of block should
stop recurring.
Everything else the gate asks for was done first: three rounds of its findings
were triaged and applied (the round-two `run_tool` exit-status bug and the
relative-root double-resolution were real and serious), and the change carries
285 tests, zero clippy warnings, and zero missed mutants.
### Landed - 2026-08-17 — `ToolOutcome::passed` removed
The gate flagged `passed()` as letting a commit through despite findings. The
premise was wrong — it had **zero production callers**, and Phase 5a's gate
reads `tool_findings` and `failures` directly — but the name and doc were
genuinely misleading: "safe to treat as nothing wrong here" reads as "no
findings" when it meant "the tool got to look". A test-shaped method with a
misleading name is a trap for the phases that build on it, so it is gone, and
with it a test that only asserted field equality on values it had just set. The
invariant it claimed lives in `unavailable_tool_marks_every_file_in_the_batch_as_failed`.
### Landed - 2026-08-17 — Phase 5a review-gate, round two
The gate blocked again with six `error`-severity findings. Four were real, and
the two most serious were in `languages/runner`, untouched since Phase 1:
- **`run_tool` ignored the exit status entirely.** A comment asserted the code
was "irrelevant" because ruff and clippy exit non-zero *when they find
issues* — true, but it misses the other case. A tool that exits non-zero
having produced no diagnostics at all did not run: bad config, crash, bad
invocation. That was reported as `Ok` with zero findings, which is exactly
the "unavailable is not a pass" failure the module exists to prevent. The
rule is now the conjunction: non-zero **and** an empty diagnostics stream.
- **A relative `root` made the child resolve its executable twice.**
`resolve_tool` returns `root.join(relative)` and the child is spawned with
`current_dir(root)`, so a root of `repo` produced
`repo/repo/node_modules/.bin/eslint`. It worked only because the CLI passes
`"."` and every test passed an absolute temp dir. The path is absolutised
before spawning.
- **The `unsafe` env mutation in the test suite was unsound.** `PathGuard`'s
SAFETY comment claimed `PATH_LOCK` prevented concurrent access; it does not —
a mutex excludes tests that *take* it, not every reader in the process, and
these tests run beside ones that spawn `git`, which reads `PATH`. Rather than
document the race, the shared mutable state is gone: `resolve_tool_in` and
`which_first_in` take the `PATH` value as a parameter, so the tests pass a
synthetic value and never touch the environment. `PathGuard`, `PATH_LOCK` and
`lock_path` are deleted, and with them the last `unsafe` block in the test
support.
Declined: `ArgGroup` lacking `required(true)` — bare `drep check` meaning "the
whole tree" is deliberate, and the reasoning is now a comment on the group so
it stops being re-flagged. `matches!` moving its scrutinee — a `_` pattern
binds nothing, so nothing moves; the code compiles. Wiremock's server not being
polled after `block_on` returns — the subprocess tests assert on
LLM-derived findings and pass consistently, so the server is serving.
Worth recording: the first draft of the exit-status test passed for the wrong
reason. With a `json` spec, empty stdout is not valid JSON, so it reached
`Unavailable` through the *parse-failure* path without exercising the new rule
at all. It uses a `lines` spec now, where empty input is legitimately zero
findings — and disabling the guard makes it fail, which is the check that
caught it.
Testing:
- 286 passing, 2 added
- `cargo mutants` 0 missed
### Landed - 2026-08-17 — Phase 5a review-gate fixes
The pre-push gate blocked Phase 5a with exit 2 and 53 advisories. Exit 2 was
five files coming back `finish_reason='length'` or `'error'` — the truncation
class again, which 1.x has no type for.
Real defects, three of them in code this session had already touched:
- **A truncated response cut off before `issues` reported as
`MalformedFinding`.** The early return for a missing `issues` array did not
consult the truncation flag, so the real cause was replaced by a symptom.
- **The text renderer detached suggestions from their findings.** It printed
every finding line and then every suggestion line, so with two findings the
first suggestion appeared under the second finding. Only a single-finding
fixture could miss it, which is what the acceptance test used.
- **`resolve_tool` indexed `spec.command[0]`**, panicking on an empty command
in a function documented never to panic.
- **A file whose name begins with `-` was passed to tools as an option.** A
repository can contain `--fix`. Guarded with a `./` prefix rather than a
`--` separator, because `--` is not accepted uniformly across
ruff/eslint/tsc/gofmt/go vet/clippy while `./` is unambiguous to any
argument parser.
Three non-discriminating tests:
- `merge_unions_same_path_into_one_entry_keeping_the_first_reason` asserted
only things that were already true *before* `merge` ran, so a `merge` that
did nothing would have passed.
- `non_executable_repo_local_path_falls_through_to_path` asserted `None` with
nothing on PATH, which a `resolve_tool` that gave up at the non-executable
local hit would also satisfy — and it depended on the host not having a
`mytool` installed.
- `request_count` mapped an unavailable mock-server log to `0`, making a broken
server indistinguishable from one that received nothing. The tests asserting
"no request was made" are exactly the ones that would have passed wrongly.
Test-infrastructure fixes: `PathGuard` now **prepends** to `PATH` instead of
replacing it — replacing made the fake binary resolvable and `git`
unresolvable, and `PATH_LOCK` does not help because it excludes other
PATH-rewriting tests, not the git-shelling ones running concurrently. Two
`diff` tests failed that way while this was being written. `PATH_LOCK` is also
taken through a helper that tolerates poisoning, so one panicking test no
longer cascades into every later one. `make_executable` had five copies across
four modules and now has one. The gating tests that still called `check::run`
directly — bypassing the cache seam and writing to the developer's real
`~/Library/Caches` — go through `run_with`.
Declined, with reasons: `ArgGroup` lacking `required(true)` (bare `drep check`
meaning "the whole tree" is deliberate and now tested); the unconfigured-tool
test depending on ambient `PATH` (`tool_status` returns `Skipped` before any
PATH lookup); Windows `.cmd`/`.exe` resolution and non-UTF-8 paths (both
already recorded as out of scope); and a TOCTOU window between `metadata` and
`read_to_string` (the read's result is what is used).
Testing:
- 284 passing, 2 added
- Break-tests re-derived; `cargo mutants` 0 missed
### Landed - 2026-08-17 — Phase 5a: `drep check` end to end
`src/cli/check/{mod,input,deterministic,render}.rs` plus 27 acceptance tests.
`failed_files` became `BTreeMap<PathBuf, FailureReason>` and
`LlmError::Transport` now carries the HTTP status as a number, so a 429 or 500
reaches the user instead of being discarded at `Err(_)`.
Three correctness bugs, all found by review rather than by the suite:
- **Bare `drep check` could not succeed.** With no paths the root path was
returned *unexpanded* — a directory. `metadata` succeeded, the size gate
passed, and `read_to_string` then failed with "Is a directory", so the
plainest invocation of the primary command reported the repo root as
unreadable and exited 2 having analyzed nothing. Both branches go through
`expand_paths` now. Nothing covered it; `src/cli/mod.rs`'s parse tests had
even dropped `["drep", "check"]` from their list.
- **`render` computed its own exit code** and ignored `--fail-on`, so a run
with an LLM finding and no `--fail-on` exited 0 while the JSON reported
`"exit": 1`. The gate is the only source of the verdict, and `exit` is now a
field on `CheckOutcome` so the two cannot be paired wrongly. Found by
`cargo mutants`, not by the hand-written break-tests: criterion 23 only
exercised a failure run, where every way of computing the exit agrees on 2.
- **An explicitly named path that does not exist read as clean.**
`expand_paths` silently skips missing paths — a deliberate contract
inherited from 1.x `scan`. Behind a gate whose thesis is that unanalyzed is
never clean, `drep check typo.rs` printing "No issues found." is the same
category as a file too large to send.
From the `/simplify` pass, beyond the above:
- The deterministic and LLM layers now run under `tokio::join!`. They share no
data, and the tool leg is otherwise pure added latency — a warm
`cargo clippy` on this repo is ~3.5 s while the LLM leg is multi-second
regardless.
- Two discarded `git` queries per run deleted: `staged_files`/`changed_since`
were called only as an error probe, and the hunk calls that follow go through
the same helpers with the same `has_head` probe and the same dash-guard.
~37 ms of fixed startup latency on every hook run.
- `plan_tasks` keyed a map on the language *name* and then re-found the
language by scanning `all_languages()` — the CLI re-deriving identity from a
string that `languages/` owns, with an unreachable `else { continue }` that
would have dropped a whole language's batch. Its map values were cloned
`Vec<Vec<Hunk>>` — a deep copy of every line of every file — read only for
the file path that was already in hand.
- `union_failures` states the failure-union rule once; it had been written
longhand at four sites, each re-explaining it.
- `severity_name` in `render` was a second wire-name table beside
`Severity::as_str`, which exists precisely to stop that.
- One `PATH_LOCK` for the crate. There were two, in different modules — and two
mutexes do not exclude each other, so the PATH-rewriting suites raced rather
than taking turns.
- `run_with`, the cache seam added so tests could avoid the developer's real
`~/Library/Caches`, had zero callers; the in-process tests now use it.
Testing:
- 282 passing, 27 added
- 12 break-tests on the contract rules; `cargo mutants` 75 mutants, 0 missed
- Two findings surfaced by the delegated test-writing itself: the LLM cache is
process-global, so a stale entry from one test satisfied another and made an
unreachable-endpoint test exit 0; and a `MockServer` returned from a
`block_on` block is dropped at the closing brace, freeing the port before a
subprocess connects.
### Landed - 2026-08-17 — Phase 4b review-gate fixes
The pre-push gate (drep 1.x reviewing drep 2.0) blocked the Phase 4b push with
exit 2 and 22 advisory findings. Exit 2 was correct: `analysis/tests/result.rs`
came back `finish_reason='length'`, so 1.x got nothing for it — which is exactly
the truncation case 2.0 was built to handle and 1.x has no type for.
Real defects found, two of them regressions introduced hours earlier:
- **`UnknownSeverity` reported the wrong vocabulary.** Its `Display` hardcoded
`Severity::ALL`, so once `LlmSeverity` began sharing the error type a rejected
`"blocker"` said "expected one of: info, warning, error" — a list the parser
never accepts and the model was never asked for. It now carries the expected
names, derived from `ALL` in a const so the two cannot drift.
- **A previously-fixed test regressed.** `unset_max_tokens_is_absent_from_the_request`
was back to `is_none_or(is_null)`, accepting an explicit `null` where the
contract says absent — the exact defect `docs/phase-3-followups.md` records as
fixed in `8a20522`. Now `is_none()`.
- **A schema-invalid response was cached.** Valid JSON with no `issues` array
arrives as `Complete`, so the cache stored it and replayed the same
file-level failure for the whole TTL with no request to notice the endpoint
had recovered. Only responses that parse without a file-level failure are
cached now, the same rule truncation already had.
Four tests did not test what they claimed: the merge test had every file
succeed so the union was never exercised; the cache-hit test never observed the
limiter it is named for; the clean-response test would have passed had no
request been made; and the missing-field test did not pin
`dropped_out_of_range == 0`, leaving the two failure classes indistinguishable
there.
**`parse_issue` now checks record shape before line membership.** The old order
left the combined case — an out-of-range line *and* an unknown severity — as a
silent drop, reporting a demonstrably schema-violating response as fully
understood. Neither existing criterion constrained that case, so the comment
claiming criterion 13 required the old order was a coincidence retrofitted with
a justification. Shape asks "did the model answer in our schema", membership
asks "did it talk about code we sent"; shape is the more conservative first
question, and a test now pins it.
Declined, with reasons: the `transient_500` retry test (flagged `error`, but it
asserts `calls >= 2`, which cannot pass unless a failure was served first);
`format!` panicking on braces (braces in argument *values* are not re-parsed);
`summary` not validated (ignored by design); cache stampede (one file is one
key, and files are distinct).
Testing:
- 250 passing, break-tests re-derived at 16/16
- `cargo mutants`: 10 mutants, 5 caught, 5 unviable, 0 missed
### Landed - 2026-08-17 — Phase 4b: analysis and the failure contract
`src/analysis/prompt.rs` (system prompt per language), `src/analysis/result.rs`
(`AnalysisResult`), and `src/analysis/code_quality.rs` (payload → cache → LLM →
findings). Phase 4 is complete; `drep check` can be wired up in Phase 5.
The contract, which is the whole point of the phase:
- **`Extracted::Truncated` returns its partial findings *and* marks the file
unanalyzed**, unconditionally. This layer never consults `--fail-on`; Phase 5
decides what a failed file maps to. A truncated response is also **never
cached** — caching it would make one truncation permanent for the TTL.
- **A finding whose line is not in `Payload::valid_lines` is dropped and
counted, never clamped.** Clamping attaches a real-looking finding to
arbitrary code. The file still counts as analyzed: the model misreported, but
the response was understood.
- **A malformed record makes the file unanalyzed** — unknown severity, missing
`line`/`severity`/`message`, a non-integer or out-of-`u32` line. Deliberately
a different class from an out-of-range line, and the tests are written so an
implementation cannot conflate the two.
`LlmSeverity` joins `Severity` in `findings.rs`: the model reasons in
critical/high/medium/low/info and collapsing that to three levels is drep's
decision. The prompt renders its alternation from `LlmSeverity::ALL` and the
parser accepts the same list, so the two cannot drift — a level named in the
prompt but missing from the parser would mark every file carrying it
unanalyzed, which is an exit-code consequence, not a cosmetic one.
Testing:
- 31 tests added, 247 passing
- 14 break-tests, one per contract rule, each confirmed to fail its naming test
- `cargo mutants`: 24 mutants, 13 caught, 11 unviable, 0 missed
- **Criterion 22 was decoration and took three attempts to fix.** Deleting the
limiter acquisition outright left it green. An in-flight counter in
`respond_with` cannot work — wiremock runs that closure under its own state
lock, so requests serialise inside it regardless. Wall-clock with `set_delay`
cannot work either: measured, four requests take 652 ms at
`max_concurrent = 1` and 595 ms at `max_concurrent = 8`, because wiremock
never overlaps requests at all. The test now watches the limiter's own permit
count, and both dead ends are recorded on it.
- One coverage gap found by break-testing: nothing covered a line beyond
`u32`, which the old `unwrap_or(u32::MAX)` clamp let fall into `Dropped` by
accident. Now `Malformed`, with a test.
### Landed - 2026-08-17 — Phase 4b cleanup (`/simplify`)
Four agents over the staged diff. The structural findings:
**`model`/`temperature` were duplicated onto the analyzer** on the stated
grounds that `LlmClient`'s fields are "not part of the public API" — they are
`pub(crate)` and readable from the same crate, so the justification did not
exist. The doc then said "take the model from the validated client instead"
directly above code reaching back into `cfg`, with an unreachable
`unwrap_or_else(|| "unknown")` of exactly the kind `findings.rs` refuses
elsewhere. Both fields deleted; the cache key reads the client.
**`parse_response` took `extracted: Extracted` *and* a `truncated: bool`**, then
discarded the discriminant and trusted the bool. `(Extracted::Truncated(v),
false)` compiled and reported a truncated file as clean — the one outcome the
module exists to prevent, resting on four call sites agreeing by convention.
The flag is now read off the discriminant.
**The limiter is a constructor parameter**, like the cache. Built per analyzer
it would stop capping the moment a second analyzer existed: two of them put
`2 * max_concurrent` requests in flight against one endpoint.
**`src/analysis/tests/support.rs` was a byte-identical copy of
`src/llm/client/tests/support.rs`** — and the copy had dropped the doc paragraph
explaining why `fast_retry_client` must not override `max_attempts`, which is
the paragraph recording a real past bug. Both now come from
`src/test_support.rs`. The analyzer fixtures also built the struct by literal,
restating the cache-key derivation by hand; they go through
`CodeQualityAnalyzer::new` now, and `analyzer_with_fast_retry_serial` is gone
(its only caller already set `max_concurrent = 1`).
Also: `parse_response`/`parse_issue` are free functions (they read no analyzer
state, so the parsing core is testable without a `MockServer`, a `Cache` and a
`TempDir`); the gutter format is described only by `payload.rs`, which owns it —
`prompt.rs` had drifted into describing removed lines even for a whole-file
payload that has none; the two "makes no request" tests no longer mount a mock,
so a wrongly-issued call 404s and is caught twice over.
Declined: making `dropped_out_of_range` a map keyed by path. The objection was
that it sums where `failed_files` unions — but the two axes differ for a
reason. `failed_files` identifies *files*, where the same file failing twice is
one failure; `dropped_out_of_range` counts *findings*, and two analyzers drop
distinct findings. Summing is correct; the doc now says why.
Testing:
- 247 passing, unchanged by the refactor
- Break-tests re-derived for the new shapes: 14/14 caught
- `cargo mutants`: 24 mutants, 13 caught, 11 unviable, 0 missed
### Landed - 2026-08-17 — Phase 4a cleanup (`/simplify`)
A four-agent `/simplify` pass over 8ec1ed1. The findings that mattered were
structural, and the first one is the reason the pass was worth running:
**The line-numbering rule existed twice.** `Hunk::numbered_new_lines` had zero
production callers — `payload::build_payload` walked `hunk.lines` and
maintained its own counter with its own copy of "a removed line does not
advance the number". Hardening one did nothing for the other, and the
ground-truth check that validated 1,870 real line numbers went through the copy
production did not use. There is now one `Hunk::numbered_lines` yielding
`(Option<u32>, &HunkLine)`; `numbered_new_lines` is a projection of it and the
renderer consumes it directly. `HunkLine::marker()`/`content()` moved to the
type so the gutter is one `writeln!`.
**`render` bypassed the language registry.** It took `language_name: &str`,
which let a caller pass the registry key where the prompt wants the
`display_name` — the tests were passing `"rust"`, so the payload header said
`rust` rather than `Rust`. It now takes `&LanguageSupport`, restoring the
invariant that `languages/` is the only place a language is named. It also no
longer takes a `file_path`: every `Hunk` carries one, and a second copy was
something the caller had to keep in sync with nothing to check it.
**Scan-target policy left the parser.** `parse_unified_diff` called
`files::is_scan_target`; which files drep reviews is a product decision and now
sits in `mod.rs` beside `filter_scan_targets`, at the same layer. The parser is
policy-free and reusable for a differently-scoped caller.
**The ACMR + empty-tree rules were stated four times.** `staged_diff` and
`since_diff` now build the argv once, so `staged_files`/`staged_hunks` and
`changed_since`/`hunks_since` cannot drift into disagreeing about scope — which
would mean analyzing a file the gate never listed.
Also: `PendingHunk` deleted (a field-for-field clone of `Hunk` with no added
invariant, ~50 lines); `skip_until_next_hunk` deleted (provably implied by
`pending.is_none()`); `RangeTail` and its hand-rolled scanner replaced by
`split_whitespace` + `split_once` (45 lines → 20, two fewer allocations per
header); duplicated inline test modules removed, keeping only the private-item
tests the sibling module cannot reach.
Declined, with reasons: dropping the `has_head` probe on the `--cached` paths
(verified redundant on git 2.50.1, but it trades a compatibility guard for 18 ms
against a multi-second LLM call, and the git version floor is unpinned);
replacing `Hunk::whole_file` with a `Scope` enum (the data-driven inference is
sound — git never emits a hunk with no changed line — and the assumption is now
documented where it belongs). Concurrency of the git calls and memoizing the
HEAD probe are Phase 5 wiring; the API is already shaped to allow both.
Testing:
- 216 passing, unchanged
- Break-tests re-derived for the new shape: 12/12 caught, including three new
ones the refactor made possible (a single mutation to `numbered_lines` now
fails both consumers, which is the point)
- `cargo mutants`: 60 mutants, 14 caught, 46 unviable, 0 missed
### Landed - 2026-08-17 — Phase 4a: diff hunks and the LLM payload
The first half of Phase 4, and the first part of the rewrite with no Python to
port: 1.x sent whole files (`analyze_file`, capped at 32k chars), so there is no
reference implementation for sending changed hunks with enclosing context.
`src/diff/hunks.rs` parses `git diff --unified=N` into `Hunk`/`HunkLine`.
`src/diff/mod.rs` gains `staged_hunks` and `hunks_since`, mirroring the
selection rules of `staged_files`/`changed_since` (`--diff-filter=ACMR`,
empty-tree fallback, three-dot for the branch case) but returning the diff
rather than the names. `CONTEXT_LINES = 20`: git merges hunks whose context
windows overlap, so a generous value cannot double-cover the same lines, and it
substitutes for the parser drep deliberately does not have.
`drep/pr_review/diff_parser.py` was the nearest reference and carries two
defects the Rust does not reproduce:
- It reads the file path off `diff --git a/… b/…` with `re.search(r"b/(.+)$")`.
A repository path containing `b/` — `src/b/mod.rs` — captures the wrong span.
The Rust reads it off the unambiguous `+++ b/<path>` line instead.
- It skips any hunk-body line starting with `---` or `+++` as a file header.
Removing a source line beginning with `--` arrives as `---…` and was silently
dropped. Those headers only appear before the first `@@`, so inside a body the
first byte alone decides the kind.
`src/analysis/payload.rs` renders hunks into the text the model sees. Each line
carries its **true new-file line number** in a gutter — `{marker}{n:>6} | ` —
because a model handed a bare diff must infer line numbers from the `@@` header,
and every finding then points at plausible-looking wrong code. Removed lines get
a blank number field. `render` returns the text plus `valid_lines`, the set of
numbers actually shown; Phase 4b drops any finding outside it as a finding about
code the model never saw. Context lines are in the set deliberately — they were
shown with real numbers, so a finding on one is an observation, not a
hallucination. The scope sentence is chosen from the data (any Added/Removed
line means diff mode, otherwise whole-file mode), so no caller can set it wrong.
Testing:
- 33 tests added, 216 passing
- 9 targeted break-tests: each load-bearing behaviour inverted and confirmed to
fail its naming test — path source, `---` body lines, removed-line numbering,
payload-relative numbering, `valid_lines` membership, omission-gap
arithmetic, `--unified` reaching git, scope-sentence selection, three-dot
- `cargo mutants` on the diff: 54 mutants, 11 caught, 43 unviable, 0 missed
- Ground truth: parsed a real commit (`f3ad627..8a20522`) and verified all
1,870 numbered lines against the actual file content at that revision
- One non-discriminating test found and fixed:
`valid_lines_contains_context_and_added_numbers_only` used a mid-hunk removed
line, whose number collides with the following numbered line, so an
implementation that wrongly numbered removals was invisible to it. The fixture
now ends with a trailing removal.
### Landed - 2026-08-17 — Phase 2: file targeting and diff
`src/files/` ports `drep/core/file_targets.py` onto the `ignore` crate. The
walker prunes vendored directories during traversal rather than collecting
then filtering (the reason `rglob` is wrong on a real repo: it `stat`s every
entry under `node_modules/` before discarding it), and a single hardcoded
ignored-dir set plus `languages::vendored_dirs()` keeps the registry as the
only place to add a build directory.
`is_scan_target` derives from `languages::source_extensions()` so adding a
language widens discovery automatically. `is_python_source` stays separate
on purpose: the docstring-style pass runs `ast.parse`-equivalent logic and
must never see a Go file even after a future registry addition.
`is_markdown` is its own predicate for the same reason — the documentation
analyzer is not a code language.
`expand_paths` deduplicates via `BTreeSet`, so `drep check a.rs .` pays once
for `a.rs`. The gitignore asymmetry with `walk_targets` is deliberate and
pinned by `explicit_filenames_are_honoured_even_when_gitignored`: an explicit
file path is a stronger signal than a repo-wide `.gitignore`.
`src/diff/` shells out to `git` rather than adding libgit2, matching the
migration plan's principle that the only operations drep needs are the ones
git's own CLI was built for. Three filters:
- `staged_files` — `git diff --cached --name-only --diff-filter=ACMR`,
with the empty-tree fallback for repos that have no `HEAD` yet.
- `changed_since` — three-dot diff (`<ref>...HEAD`), so a pre-push gate
reports only what the branch changed since the merge base, never what
landed on the base afterwards. Pinned by
`three_dot_excludes_base_modifications_that_two_dot_would_report`.
The first attempt at that test did *not* discriminate. It had the base branch
**add** a file, which a two-dot diff reports as a deletion — and
`--diff-filter=ACMR` drops deletions, so it passed under `..` as well. The
base has to **modify** a shared file, which two-dot reports as `M` and the
filter keeps. Both tests are retained: the first documents intent, the second
is verified to fail under `..` and pass under `...`.
- `current_commit_sha` — 5s timeout, `"unknown"` on every failure. The one
counter-example to "git errors propagate" here: this feeds a cache key
only, and a cache-key component must never take analysis down.
`drep::files::is_scan_target` and `drep::languages::source_extensions()` are
the new shared module pattern for "do not re-implement these in a CLI
command"; `docs/rust-migration.md` already points to them and the next
phase uses them directly.
30 new tests cover every criterion in `PHASE2_SPEC.md` (deleted), one per
acceptance point, plus a discriminating three-dot test and
`tests/ground_truth_walk.rs`, which walks *this* repository and asserts the
walk never descends into the 222MB `venv/`, `target/`, `node_modules/` or
`.git/`, keeps gitignored `.claude/` and `.pytest_cache/` out despite
`hidden(false)`, finds Python, Rust and Markdown sources, and honours an
explicitly named gitignored file. Test count went 63 → 95. Both source files stay well
under the 600-line soft limit; the directories `src/files/tests/` and
`src/diff/tests/` follow the same `mod.rs` declare-a-sub-module pattern
that the runner introduced in Phase 1 after the orphaned-files incident.
Every file is reachable through a `mod` declaration, verified by appending
invalid Rust to each and confirming the build *fails* — all 8 confirmed live.
`ignore = "0.4"` arrives in `Cargo.toml` for the gitignore-aware walk; no
other dependency changes. `cargo test --all-targets`,
`cargo clippy --all-targets --all-features`, and `cargo fmt --all --check`
all pass.
### Landed - 2026-08-17 — Phase 3: the LLM layer
**3a** — `src/config.rs` (TOML, `${VAR}` expansion over the whole parsed tree so
future fields inherit it), `src/llm/json_parsing.rs` (fence → direct → comma
repair → truncation recovery, returning `Complete` or **`Truncated`** as a type
because under `--fail-on` a truncated response could omit the one blocking
finding), and `src/llm/client/` over open-agent-sdk.
**3b** — `src/llm/cache.rs` and `src/llm/concurrency.rs`, both deliberately
smaller than the Python:
- The cache key is **content-addressed and not git-aware**. 1.x mixed the commit
SHA in, which forced a miss on every unchanged file at every new commit — the
exact case a commit gate's cache exists to serve. Fields are length-prefixed so
`("ab","c")` cannot collide with `("a","bc")`. Reads are infallible: a corrupt
entry is a miss, never an error, because a cache must not be able to take the
gate down.
- The limiter is **just a concurrency cap**. The per-repo semaphores,
requests-per-minute window, token budget with two-phase reservation, and
circuit breaker all existed because 1.x was a server scanning whole repos. One
repo per invocation, 429 now retried by the SDK, no token budget means nothing
to reconcile, and one developer committing is not a stampede.
**Requires open-agent-sdk 0.7.0**, which fixes three bugs found here: streamed
text was silently discarded when a response ended without `finish_reason`; 429
was not classified retryable; and `max_tokens` could not be left unset.
**Bugs the gate found in its own new code.** drep 1.x reviewing drep 2.0 returned
23 advisory findings; five were real:
- `FENCE_RE` had no `(?s)`, so `.` never crossed a newline and **no multi-line
fenced response parsed** — and real model output is pretty-printed. It failed
safe (unanalyzed, never a false clean) but the primary path was unusable. The
three existing fenced tests used single-line bodies and did not discriminate.
- The trailing-comma repair was a regex over raw text with no string awareness:
`{"a":",}","b":1,}` was rewritten to `{"a":"}",...}` and returned `Complete`.
Same defect class as the single-quote repair this module deliberately omits.
Now a string-aware walk.
- Repairs did not compose, so truncated-plus-trailing-comma was unrecoverable.
Balancing now happens before stripping, because a dangling comma at the end of
a truncated response has no delimiter after it to be seen by.
- `LlmClient` derived `Debug` while holding a plaintext `api_key`.
- `run_git` had no timeout except at one call site.
Test count 117 → 183. `cargo mutants`: 170 mutants, 69 caught, 101 unviable,
zero missed.
### Decided - 2026-08-17
**drep 2.0 will be a Rust binary with a much smaller scope.** Plan in
[docs/rust-migration.md](docs/rust-migration.md); work happens on the `rust-rewrite`
branch. Python 1.x is frozen — no new features.
The product becomes exactly two things: run the linters and formatters the repository has
configured, and send the changed code to an LLM for review. Triggers are pre-commit and
pre-push only.
Dropped (~5,400 LOC): the Gitea/GitHub/GitLab adapters, the webhook server, PR comment
posting, the SQLite layer, docstring *generation*, the config wizard, the Bedrock
provider, and the metrics history. Platform integration is a crowded market and was never
the differentiator; running locally against your own model before the code leaves the
machine is.
Rust rather than Go because [open-agent-sdk-rust](https://github.com/slb350/open-agent-sdk-rust)
(v0.6.9, feature-parity with the Python SDK) already exists and is where the LLM transport
work is happening. Dropping docstring generation removes the last `ast.parse` caller, so
the rewrite needs no Python interpreter and no tree-sitter.
Distribution via `cargo-dist`: multi-arch binaries, a shell installer, and a Homebrew tap.
### Landed - 2026-08-17 — Phase 1: deterministic tool runner
`src/languages/runner.rs` ports `drep/languages/runner.py` faithfully. The
five output parsers (`lines`, `json` for both ruff and eslint, `position` for
`go vet`, `tsc`, `cargo` for clippy NDJSON) are in one module; `Finding` lands
in `src/analysis/findings.rs` so the tool parsers can produce it.
31 acceptance tests cover every criterion in `PHASE1_SPEC.md` (deleted):
3 for `resolve_tool`, 4 for `is_configured`/`tool_status`/`passed`, 3 for
`lines`, 6 for `json` (including the empty-string and invalid-JSON split
that makes the gate refuse to "green" on a broken plugin), 3 for `position`
(package-header skip + `vet:` prefix), 3 for `tsc`, 4 for `cargo`
(non-JSON is an error, primary span wins), 1 for unknown format, and 4
real-`/bin/sh`-script end-to-end runs of `run_tool` covering skipped,
unavailable, stderr-streaming, and unparseable.
`tests/real_tool_output.rs` adds 5 more against output captured from ruff
0.16, gofmt and go vet 1.21 run on deliberately broken files. Hand-written
fixtures prove the parser matches its spec; only real output proves the spec
matches the tools. It covers both `go vet` shapes — the bare
`main.go:6:14:` form and the `# package` header with a `./` prefix — and
pins that a clean run (`[]`, or empty output) is zero findings rather than a
parse error.
Total test count went 27 → 63. `cargo test --all-targets`,
`cargo clippy --all-targets --all-features`, and `cargo fmt --all --check`
all pass; no new dependencies beyond the ones Phase 1 declared.
The runner is a directory module — `runner/mod.rs` decides whether a tool runs,
`runner/parsers.rs` reads what it said, and `runner/tests/` holds the unit
tests. Largest file is 371 lines, against this repo's 600-line soft limit.
**Caught during review: four test files that were never compiled.** An earlier
draft left `runner/tests/{parsers,resolve,run_tool,support}.rs` on disk with no
`mod` declaration reaching them. Rust only compiles files a `mod` points at, so
those 31 tests silently did not exist — appending invalid Rust to one did not
fail the build. They also duplicated the tests running inline, which is what
made the gap invisible: the count looked right. Every file under
`runner/tests/` is now verified by appending garbage and confirming the build
breaks. If you add a file there, declare it in that directory's `mod.rs`.
### Planned
- Doc-comment generation for JavaScript/TypeScript, Go and Rust (needs a parser per
language; the LLM review path does not) — **dropped in 2.0**, see above
- Removal of deprecated `CodeQualityAnalyzer.analyze_files` (1.4.0)