Skip to content

feat: add --diff <ref>, a baseline taken from git instead of a file - #1

Open
hamodywe wants to merge 1 commit into
mainfrom
feat/diff-ref
Open

feat: add --diff <ref>, a baseline taken from git instead of a file#1
hamodywe wants to merge 1 commit into
mainfrom
feat/diff-ref

Conversation

@hamodywe

Copy link
Copy Markdown
Owner

Closes the first item under Next in ROADMAP.md.

What

overpull check --diff main --fail-on hazard

Report only what the working copy adds relative to a git ref, with no baseline file to keep up to date. The tree at the merge base of <ref> and HEAD is checked out into a temporary worktree, analysed with the same command and the same thresholds, and whatever it reports becomes the baseline for the run over the working copy. The worktree is removed before the report prints.

cycles  clean
  4 modules scanned, no new import cycles.

  1 finding hidden by --diff main (merge base c96d7dd) (1 cycle, 0 barrels)

Why the merge base, not the tip

A fix that landed on main after the branch was cut is not something the branch removed, and a finding main gained since is not something the branch added. Comparing against the point where the histories diverged judges a branch on its own changes and nothing else. diff_compares_against_the_merge_base_not_the_tip builds exactly that history — crash on main, branch cut, main fixes it, branch still carries it — and asserts the branch is not blamed for it.

Design

  • The baseline machinery is reused, not duplicated. The comparison run produces the same JSON documents --json would print, and Baseline::from_parts reads them. So a finding is "already there" under exactly the rules --baseline uses: same cycle key (member set, order-independent), same "worse than known is still reported" rule, same amplification slack. There is one place that decides what "known" means.
  • --diff and --baseline combine. Baseline::merge keeps the worse reading where both record the same finding, because "not worse than what was known" has to mean the most that was known. The note names both: hidden by the baseline and --diff main (merge base …).
  • Errors are usage errors, and they come first. An unknown ref, no repository, or no git on PATH exits 2 before any graph is built. The ref is verified with rev-parse --verify <ref>^{commit} so --diff typo says so rather than failing inside merge-base.
  • --root inside a subdirectory works. rev-parse --show-prefix maps the root into the checkout; if the directory did not exist at the merge base, everything is new and the run says nothing was hidden.
  • --entry and --tsconfig are carried across only where they exist. An entry file that did not exist at the merge base is simply not an entry there — not an error, and not silently re-pointed at another file.
  • Cleanup is a Drop. git worktree remove --force, falling back to remove_dir_all + worktree prune so a failed removal never leaves a dangling registration. Every --diff test asserts git worktree list is back to one entry.
  • One behaviour change, deliberate: --baseline (and --diff) are now rejected on cost and why. Neither has findings a baseline can hide, so the flag was accepted and did nothing — the rule the other flags already follow is that a flag that silently does nothing is worse than one that is not accepted.

Tests

Five new process-level tests in tests/cli.rs build a real repository in a temp dir (identity and commit.gpgsign=false set per call, so nothing depends on the machine):

test asserts
diff_reports_only_what_the_working_copy_adds uncommitted crash → exit 1; committed → exit 0, hidden by --diff HEAD (merge base …), no new import cycles, worktree gone
diff_compares_against_the_merge_base_not_the_tip the scenario above
diff_treats_a_finding_that_got_worse_as_new a committed benign cycle over the same two files is hidden at --fail-on any; the crashing version of it is reported
diff_names_an_unknown_ref_and_a_missing_repository exit 2 with the ref / "git repository" named
diff_and_a_baseline_file_both_hide_findings the combined note

Plus unit tests for Baseline::merge (both orders keep the worse reading), from_parts(&[]), the CLI parse/validate paths, and Comparison::checkout outside a repository. scripts/ci.sh gained a behaviour check that --diff HEAD on a committed fixture is quiet and leaves no worktree behind.

sh scripts/ci.sh — fmt, clippy (pedantic, -D warnings), 34 + 30 + 25 tests, MSRV 1.95 build, node fixture verification, behaviour checks — passes on this machine.

Not in this PR

  • JSON output is unchanged. Suppressed counts are not in the JSON today for --baseline either; adding them is a separate, additive change to the documented contract.
  • Submodules are not initialised in the comparison worktree. node_modules is never there either, and resolution already degrades to counting externals at the boundary, so this matches how a fresh checkout is analysed.

A baseline is a file someone has to keep up to date. Most of the time the
question is simpler: what does this branch add that main did not already
have? `--diff <ref>` checks out the merge base of <ref> and HEAD into a
temporary worktree, runs the same command with the same thresholds over
it, and uses what it reports as the baseline for the working copy. The
worktree is removed before the report prints.

The merge base rather than the tip of <ref>, because a fix that landed on
main after the branch was cut is not something the branch removed, and a
finding main gained since is not something the branch added. A test builds
exactly that history and asserts the branch is not blamed for it.

The baseline machinery is reused rather than duplicated: the comparison
run produces the JSON documents `--json` would print and `Baseline` reads
them, so "already known" means the same thing whichever way the baseline
arrived. `--diff` and `--baseline` combine through `Baseline::merge`,
which keeps the worse reading of a finding both record. The note at the
end of the report now names what hid the findings and the merge base.

Also rejects `--baseline` and `--diff` on `cost` and `why`, which have no
findings to hide: the flag was accepted and changed nothing, and a flag
that silently does nothing is worse than one that is not accepted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: hamodywe <iosapk.org@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant