From 7df07665a874fa9c3650941440e5d4007e39a21a Mon Sep 17 00:00:00 2001 From: Joe Esquibel Date: Wed, 23 Sep 2026 14:06:45 -0400 Subject: [PATCH 1/2] tools: content-keyed ruff/mypy baselines so line shifts don't churn them (#3384) Baseline keys were `{file}:{line}: {code}`, so any unrelated line shift in a touched file changed the baseline and conflicted on rebase (the recurring hand-fixes of moved audit_recorder.py entries). Keys are now `{file}: {code} @{sha256[:12] of the whitespace-stripped flagged line}#{occurrence}` via a shared tests/lint_baseline.py, used by both ruff_audit.py and mypy_audit.py (same design, same problem). - --ci strictness unchanged: new key -> fail, baseline key no longer flagged -> FYI stale list; a new duplicate of a baselined line gets the next occurrence index and fails; editing a flagged line fails. - Output still shows each finding's current line number. - New --write-baseline flag writes the canonical sorted form (byte-identical on regenerate); replaces the broken `from tests.ruff_audit import` hint. - Baselines migrated (5 ruff, 1 mypy); old-key translation equals the regenerated file. - audit_check.py: shift detection now pairs edited baselined lines one-for-one with stale entries (multiset), never a new duplicate. - Tests incl. a temp-git-repo merge of two line-shifting branches. Part of #3384 Co-Authored-By: Claude Opus 5.5 (1M context) --- ANTIGRAVITY.md | 10 +- CLAUDE.md | 20 +- ..._to_extend_class_start_named_extraction.md | 2 +- tests/extraction/how_to_harden_extraction.md | 2 +- tests/lint_baseline.py | 123 ++++++++ tests/mypy_audit.py | 146 ++++++---- tests/mypy_audit_baseline.json | 4 +- tests/ruff_audit.py | 155 ++++++---- tests/ruff_audit_baseline.json | 10 +- tests/tools/audit_check.py | 75 +++-- tests/tools/rebase_rebless.py | 14 +- tests/tools/test_content_keyed_baseline.py | 270 ++++++++++++++++++ 12 files changed, 662 insertions(+), 169 deletions(-) create mode 100644 tests/lint_baseline.py create mode 100644 tests/tools/test_content_keyed_baseline.py diff --git a/ANTIGRAVITY.md b/ANTIGRAVITY.md index 8adf9f4fbf..e5d7b12200 100644 --- a/ANTIGRAVITY.md +++ b/ANTIGRAVITY.md @@ -179,7 +179,9 @@ Repeated ad hoc file drops like this (`scratch_func.py`, `pr_groovy_body.txt`, ` ## CI Ruff Audit The `ruff-audit.yml` CI job enforces a STRICT EXACT MATCH against `tests/ruff_audit_baseline.json`. -This baseline uses the line number as part of the JSON keys (e.g. `"gitgalaxy/core/detector.py:1002: PERF401"`). -**Important**: Any code edits that add or remove lines will SHIFT the line numbers of subsequent lint violations, causing the ruff audit to fail even if you didn't introduce new violations. -To fix this, ALWAYS regenerate the baseline before committing if you've added/removed lines in files with pre-existing lint violations: -`python -c "from tests.ruff_audit import run_ruff_check; import json; json.dump(run_ruff_check(), open('tests/ruff_audit_baseline.json', 'w'), indent=2, sort_keys=True)"` +Since #3384 its keys are content-based, not line numbers (e.g. `"gitgalaxy/recorders/gpu_recorder.py: C414 @2c826ccd023d#0"`: +file, rule code, a hash of the whitespace-stripped flagged line, and an occurrence index -- see `tests/lint_baseline.py`). +Adding or removing lines elsewhere in a file no longer changes the baseline, so there is nothing to regenerate for a pure line shift. +Editing a flagged line itself, or adding another identical violating line, IS a new key and fails the audit. If that's pre-existing +debt you deliberately carry (or you fixed findings), regenerate with `python tests/ruff_audit.py --write-baseline` +(`tests/mypy_audit.py` uses the same scheme and flag). diff --git a/CLAUDE.md b/CLAUDE.md index 979428aa86..57c5a2bd03 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -215,15 +215,19 @@ language or a follow-on bug sweep rather than re-deriving the process from scrat These are regression gates, not zero-tolerance floors: each has a committed baseline file (`tests/ruff_audit_baseline.json` etc.) and `--ci` mode fails only on *new* findings beyond it. This lets CI enforce "no new problems" without demanding the pre-existing backlog be fixed -first. If you fix backlog findings, regenerate the baseline by running the script without -`--ci`. `ruff format --check` is the one zero-tolerance exception (whole repo was reformatted -once at adoption, so there's no backlog to carry). +first. If you fix backlog findings, regenerate the baseline with `--write-baseline` +(`python tests/ruff_audit.py --write-baseline`, same for `mypy_audit.py`). `ruff format +--check` is the one zero-tolerance exception (whole repo was reformatted once at adoption, so there's no backlog to carry). Use `python tests/tools/audit_check.py` (add `--regenerate`) instead of running the three `--ci` scripts separately and manually diffing each baseline by eye — it bundles all three plus -the format check, and auto-detects "pure line-shift" findings (same file/code/message, just moved -because an earlier edit in the same file shifted everything below it) from genuine new findings -that need real review, only regenerating the former. +the format check. Since #3384 the ruff and mypy baseline keys are content-based -- +`{file}: {code} @{hash of the whitespace-stripped flagged line}#{occurrence}` (`tests/lint_baseline.py`) +-- not line numbers, so an unrelated line shift elsewhere in a file changes nothing and can't +conflict on rebase. `--regenerate` only auto-accepts an *edited* baselined line (a new key that +pairs one-for-one with a now-stale entry of the same file/code/message); a new finding, or a new +duplicate of a baselined line (next `#occurrence`), still fails. Human-readable output still +prints each finding's current line number. ## Testing conventions @@ -266,7 +270,9 @@ re-bless with `crucible_check.py --update`, then **verify** the resulting golden against main is limited to this branch's own new keys (auto-detected from the pre-rebase branch-vs-merge-base diff, or passed via `--expect-keys`) — failing loudly on any other ("foreign") drift, the same scoped-review discipline "Scoping a bless" below describes. It then -refreshes baseline entries for moved lines in touched files only and runs ruff check/format plus +runs `audit_check.py --regenerate` scoped to touched files (with #3384's content-keyed ruff/mypy +baselines, moved lines no longer need refreshing; this only re-accepts edited baselined lines) and +runs ruff check/format plus a fast, touched-file-scoped test selection. The default is a dry run (a read-only `git merge-tree` preview, no branch mutation); `--execute` performs it for real, and `--push` (only valid with `--execute`) does `git push --force-with-lease` — never plain force. diff --git a/tests/extraction/how_to_extend_class_start_named_extraction.md b/tests/extraction/how_to_extend_class_start_named_extraction.md index c2a0b0b0fd..158a75b6ab 100644 --- a/tests/extraction/how_to_extend_class_start_named_extraction.md +++ b/tests/extraction/how_to_extend_class_start_named_extraction.md @@ -151,7 +151,7 @@ to give them), not worth the risk of touching the shared `_resolve_class_start_m python tests/tools/tree_sitter_accuracy_audit.py --lang --ci python tests/tools/tree_sitter_accuracy_audit.py --lang --regenerate # if clean/improved -- also refreshes the summary table python tests/tools/crucible_check.py # both modes - python tests/tools/audit_check.py --regenerate # if pure line-shifts + python tests/tools/audit_check.py --regenerate # if only edited baselined lines python -m pytest tests/core_engine/ tests/extraction/ ``` If `--regenerate` refuses because of a real `extra_classes`/`found_classes` regression that diff --git a/tests/extraction/how_to_harden_extraction.md b/tests/extraction/how_to_harden_extraction.md index 9244d11015..33d27864cc 100644 --- a/tests/extraction/how_to_harden_extraction.md +++ b/tests/extraction/how_to_harden_extraction.md @@ -190,7 +190,7 @@ reasonable *floor*. Push further — aim for 10-15 per major language covering, - ReDoS scaling check if the fix touches a quantifier — `verify_candidates.py`'s `check_redos_scaling()` for quick iteration; a real assertion (`assert_redos_immune`, already available from `_extraction_harness.py`) once the pattern is finalized. - - `python tests/tools/audit_check.py` (add `--regenerate` if it reports pure line-shifts — see its + - `python tests/tools/audit_check.py` (add `--regenerate` if it reports only edited baselined lines — see its own docstring) instead of running `ruff format`/`ruff_audit.py --ci`/`mypy_audit.py --ci`/ `dead_key_audit.py --ci` as four separate commands and manually eyeballing each diff. - **Crucial Linter Step**: Run `ruff check .` to catch issues before the CI pipeline rejects them. GitGalaxy uses `flake8-bandit` which aggressively flags `assert` usage. Any `assert` you use in your test files MUST be appended with `# noqa: S101` (e.g., `assert match is not None # noqa: S101`). diff --git a/tests/lint_baseline.py b/tests/lint_baseline.py new file mode 100644 index 0000000000..d661777c80 --- /dev/null +++ b/tests/lint_baseline.py @@ -0,0 +1,123 @@ +#!/usr/bin/env python3 +""" +Content-keyed baseline keys shared by tests/ruff_audit.py and tests/mypy_audit.py (#3384). + +WHY +Both audits used to key baseline entries as `{file}:{line}: {code}`. Line numbers +are not stable: an unrelated insertion earlier in a file shifts every finding below +it, so the baseline changed (and conflicted on rebase) even though nothing +lint-relevant changed. Rebases repeatedly had to hand-fix moved entries. + +KEY SCHEME + "{file}: {code} @{hash}#{occurrence}" + + - `file` repo-relative POSIX path (always first, so `key.split(":", 1)[0]` + is still the file -- rebase_rebless.py relies on that). + - `code` the rule code (ruff) / error code (mypy). + - `hash` first 12 hex chars of sha256 over the flagged source line with ALL + whitespace removed (see normalize_line), so reindenting or + re-spacing the line doesn't change it either. + - occurrence 0-based index among findings sharing the same (file, code, hash), + in source order (line, then column). Two identical violating lines + in one file get #0 and #1, so a NEW duplicate of a baselined line + produces an unbaselined #1 and is still caught; it also means two + findings of the same code on the same line no longer collapse into + one entry the way the old line-number key did. + +The line number is deliberately NOT part of the key; it's carried alongside each +current finding (Finding.line) purely so human-readable output can still point at +where the finding is now. +""" + +import hashlib +import json +from collections.abc import Iterable +from pathlib import Path +from typing import Callable, NamedTuple + +HASH_LEN = 12 + + +class Finding(NamedTuple): + file: str # repo-relative POSIX path + line: int # 1-based; 0 if the tool gave no location + column: int # 1-based; 0 if unknown + code: str + message: str + + +def normalize_line(text: str) -> str: + """Strips ALL whitespace, so indentation/spacing-only edits don't change a key.""" + return "".join(text.split()) + + +def line_hash(text: str) -> str: + return hashlib.sha256(normalize_line(text).encode("utf-8")).hexdigest()[:HASH_LEN] + + +def format_key(file: str, code: str, digest: str, occurrence: int) -> str: + return f"{file}: {code} @{digest}#{occurrence}" + + +def parse_key(key: str) -> tuple[str, str, str, int]: + """Inverse of format_key: returns (file, code, hash, occurrence).""" + file_part, _, rest = key.partition(": ") + code_part, _, tail = rest.rpartition(" @") + digest, _, occurrence = tail.partition("#") + return file_part, code_part, digest, int(occurrence or 0) + + +def source_reader(repo_root: Path) -> Callable[[str, int], str]: + """Returns read(file, line) -> that source line's text ("" if unavailable), caching each file.""" + cache: dict[str, list[str]] = {} + + def read(rel_path: str, line: int) -> str: + if rel_path not in cache: + try: + cache[rel_path] = (repo_root / rel_path).read_text(encoding="utf-8", errors="replace").splitlines() + except OSError: + cache[rel_path] = [] + lines = cache[rel_path] + return lines[line - 1] if 1 <= line <= len(lines) else "" + + return read + + +def key_findings(findings: Iterable[Finding], read_line: Callable[[str, int], str]) -> dict[str, Finding]: + """Assigns each finding its content key. Deterministic: occurrence indexes follow source order.""" + ordered = sorted(findings, key=lambda f: (f.file, f.line, f.column, f.code, f.message)) + seen: dict[tuple[str, str, str], int] = {} + keyed: dict[str, Finding] = {} + for finding in ordered: + digest = line_hash(read_line(finding.file, finding.line)) + triple = (finding.file, finding.code, digest) + occurrence = seen.get(triple, 0) + seen[triple] = occurrence + 1 + keyed[format_key(finding.file, finding.code, digest, occurrence)] = finding + return keyed + + +def to_baseline(keyed: dict[str, Finding]) -> dict[str, str]: + """The committed baseline shape: {key: message} -- no line numbers, so line shifts don't touch it.""" + return {key: finding.message for key, finding in keyed.items()} + + +def dump_baseline(baseline: dict[str, str]) -> str: + """Canonical serialization (sorted, 2-space indent, trailing newline) -- byte-identical on regenerate.""" + return json.dumps(baseline, indent=2, sort_keys=True) + "\n" + + +def write_baseline(path: Path, baseline: dict[str, str]) -> None: + path.write_text(dump_baseline(baseline), encoding="utf-8") + + +def load_baseline(path: Path) -> dict[str, str]: + if not path.exists(): + return {} + with open(path, encoding="utf-8") as f: + return json.load(f) + + +def describe(key: str, finding: Finding) -> str: + """Human-readable line for a CURRENT finding: current location first, stable key after.""" + return f"{finding.file}:{finding.line}: {finding.code} -- {finding.message} [{key}]" diff --git a/tests/mypy_audit.py b/tests/mypy_audit.py index cc77afa1eb..73a4e65262 100644 --- a/tests/mypy_audit.py +++ b/tests/mypy_audit.py @@ -11,8 +11,11 @@ USAGE python tests/mypy_audit.py # full report, exits 1 if mypy finds - # anything -- use this to regenerate - # the baseline after a cleanup PR. + # anything. + python tests/mypy_audit.py --write-baseline + # regenerate mypy_audit_baseline.json + # from the current errors (e.g. after + # a cleanup PR). python tests/mypy_audit.py --ci # baseline-gated regression check # (see BASELINE below) -- this is # what CI runs. @@ -32,38 +35,39 @@ unrelated PR, so `--ci` mode is a REGRESSION gate: it fails only on errors not already in the baseline. Fixing a baselined error doesn't fail the build either -- shrinking the baseline is a deliberate, reviewable edit you -make yourself (same "deliberate, reviewable updates instead of silent +make yourself (`--write-baseline`) (same "deliberate, reviewable updates instead of silent overwrite" philosophy as #330's golden_master.json and dead_key_audit.py's own baseline), not something this script does automatically. `--ci` prints anything it notices has already been fixed as an FYI, so the baseline doesn't silently go stale, but does not fail the build over it. -SCOPE & LIMITATIONS (read before treating the baseline as static) -Baseline keys are `{file}:{line}: {code}`. Line numbers are NOT stable -- -an unrelated edit earlier in a file shifts every error below it, which -will look like "N new errors" even though nothing type-relevant changed. -This is a standard, accepted tradeoff for line-based mypy baselining (most -real-world baseline tools, including mypy's own `--baseline-file` used -elsewhere, have the same property), not a bug in this script. If a PR -that didn't touch type-relevant code trips `--ci`, regenerate the baseline -(`python tests/mypy_audit.py` and copy its output into -mypy_audit_baseline.json) rather than treating it as a real regression. - -Also, the key doesn't disambiguate multiple distinct errors of the same -code on the same line (confirmed: 5 lines in the initial baseline each -carry 2-4 separate "operator" or "index" errors) -- those collapse into -one baseline entry each, so fixing some-but-not-all of them on such a -line won't show up as partial progress. Real, but rare (5/235 initially) -and not worth a more fragile per-occurrence key scheme to fully solve. +BASELINE KEYS (#3384) +Keys are content-based, NOT line-based, using the same scheme as +ruff_audit.py (see tests/lint_baseline.py): +`{file}: {code} @{hash of the whitespace-stripped source line}#{occurrence}`. +An unrelated edit that only shifts lines leaves the baseline unchanged; +editing the flagged line itself, or adding another identical erroring line +in the same file, is a new key and still fails `--ci`. The per-(file, code, +line-hash) occurrence index also means several errors of the same code on +one line are each their own entry now, instead of collapsing into one the +way the old `{file}:{line}: {code}` key did (5 lines in the initial 235-error +baseline carried 2-4 each), so partial progress on such a line is visible. +Human-readable output still prints each current error's line number. """ import argparse -import json import re import subprocess import sys from pathlib import Path -from typing import Dict + +# tests/ has no __init__.py -- make the sibling helper importable no matter how +# this file is loaded (script, audit_check.py, or a test's sys.path insert). +if str(Path(__file__).resolve().parent) not in sys.path: + sys.path.insert(0, str(Path(__file__).resolve().parent)) + +import lint_baseline +from lint_baseline import Finding REPO_ROOT = Path(__file__).resolve().parent.parent SCAN_ROOT = REPO_ROOT / "gitgalaxy" @@ -74,82 +78,105 @@ _ERROR_LINE = re.compile(r"^(?P[^:]+):(?P\d+): error: (?P.+?)\s*\[(?P[a-z][a-z0-9-]*)\]$") -def run_mypy() -> Dict[str, str]: - """Returns {"{file}:{line}: {code}": message} for every error mypy reports.""" +def parse_mypy_output(stdout: str, repo_root: Path) -> dict[str, Finding]: + """Turns mypy's text output into {content key: Finding}.""" + findings = [] + for line in stdout.splitlines(): + match = _ERROR_LINE.match(line) + if not match: + continue + findings.append( + Finding( + file=Path(match["file"]).as_posix(), + line=int(match["line"]), + column=0, + code=match["code"], + message=match["message"], + ) + ) + return lint_baseline.key_findings(findings, lint_baseline.source_reader(repo_root)) + + +def run_mypy_findings() -> dict[str, Finding]: + """Returns {content key: Finding} for every error mypy reports.""" result = subprocess.run( ["mypy", str(SCAN_ROOT), "--ignore-missing-imports"], capture_output=True, text=True, cwd=REPO_ROOT, ) + return parse_mypy_output(result.stdout, REPO_ROOT) - errors: Dict[str, str] = {} - for line in result.stdout.splitlines(): - match = _ERROR_LINE.match(line) - if not match: - continue - key = f"{match['file']}:{match['line']}: {match['code']}" - errors[key] = match["message"] - return errors + +def run_mypy() -> dict[str, str]: + """Returns {content key: message} -- the committed baseline's shape.""" + return lint_baseline.to_baseline(run_mypy_findings()) -def load_baseline() -> Dict[str, str]: - if not BASELINE_PATH.exists(): - return {} - with open(BASELINE_PATH, encoding="utf-8") as f: - return json.load(f) +def load_baseline() -> dict[str, str]: + return lint_baseline.load_baseline(BASELINE_PATH) -def _print_errors(errors: Dict[str, str]) -> None: - for key in sorted(errors): - print(f" {key} -- {errors[key]}") +def _print_findings(findings: dict[str, Finding]) -> None: + for key, finding in sorted(findings.items(), key=lambda kv: (kv[1].file, kv[1].line, kv[0])): + print(f" {lint_baseline.describe(key, finding)}") def run_full_report() -> int: - errors = run_mypy() - if not errors: + findings = run_mypy_findings() + if not findings: print("Mypy Audit: no errors found.") return 0 - print(f"Mypy Audit: {len(errors)} error(s) found:\n") - _print_errors(errors) + print(f"Mypy Audit: {len(findings)} error(s) found:\n") + _print_findings(findings) print( "\nTo accept this as the new baseline (e.g. after a cleanup PR), regenerate it with:\n" - ' python -c "from tests.mypy_audit import run_mypy; import json; ' - "json.dump(run_mypy(), open('tests/mypy_audit_baseline.json', 'w'), indent=2, sort_keys=True)\"" + " python tests/mypy_audit.py --write-baseline" ) return 1 +def write_current_baseline() -> int: + baseline = run_mypy() + lint_baseline.write_baseline(BASELINE_PATH, baseline) + print(f"Mypy Audit: wrote {len(baseline)}-error baseline to {BASELINE_PATH.relative_to(REPO_ROOT)}.") + return 0 + + def run_ci_check() -> int: """ Baseline-gated regression check (#430): fails only on errors NOT already in mypy_audit_baseline.json. See the module docstring's BASELINE section for why this isn't a zero-tolerance check. """ - errors = run_mypy() + findings = run_mypy_findings() baseline = load_baseline() - new_errors = {key: msg for key, msg in errors.items() if key not in baseline} - resolved_keys = sorted(set(baseline) - set(errors)) + new_findings = {key: f for key, f in findings.items() if key not in baseline} + resolved_keys = sorted(set(baseline) - set(findings)) if resolved_keys: - print("Mypy Audit: FYI -- these baselined errors are no longer flagged (fixed, or line-shifted).") - print("Consider removing them from mypy_audit_baseline.json in this PR:\n") + print( + "Mypy Audit: FYI -- these baselined errors are no longer flagged (fixed, or the flagged line was edited)." + ) + print("Consider removing them from mypy_audit_baseline.json in this PR (`--write-baseline`):\n") for key in resolved_keys: print(f" {key} -- {baseline[key]}") print() - if not new_errors: + if not new_findings: print(f"Mypy Audit: no NEW type errors beyond the {len(baseline)}-error baseline.") return 0 - print(f"Mypy Audit: {len(new_errors)} NEW type error(s) beyond the {len(baseline)}-error baseline:\n") - _print_errors(new_errors) + print(f"Mypy Audit: {len(new_findings)} NEW type error(s) beyond the {len(baseline)}-error baseline:\n") + _print_findings(new_findings) print( - "\nEach hit above is either a real new type error to fix, or a baseline that needs " - "regenerating because unrelated edits shifted line numbers (see the module docstring's " - "SCOPE & LIMITATIONS section) -- confirm which before assuming it's a regression." + "\nBaseline keys are content-based (see the module docstring's BASELINE KEYS section), so a " + "pure line shift can't cause these: each is a new error, a new duplicate of a baselined " + "line, or an edit to a baselined line. Fix it, or -- only if it's pre-existing debt you " + "deliberately carry -- re-bless with `python tests/mypy_audit.py --write-baseline` (in " + "mypy-audit.yml's environment, see IMPORTANT above)." ) return 1 @@ -157,8 +184,13 @@ def run_ci_check() -> int: def main() -> int: parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) parser.add_argument("--ci", action="store_true", help="Baseline-gated regression check (what CI runs).") + parser.add_argument( + "--write-baseline", action="store_true", help="Regenerate mypy_audit_baseline.json from current errors." + ) args = parser.parse_args() + if args.write_baseline: + return write_current_baseline() return run_ci_check() if args.ci else run_full_report() diff --git a/tests/mypy_audit_baseline.json b/tests/mypy_audit_baseline.json index 9a21a8cf28..fa17267158 100644 --- a/tests/mypy_audit_baseline.json +++ b/tests/mypy_audit_baseline.json @@ -1,3 +1,3 @@ { - "gitgalaxy/galaxyscope.py:391: assignment": "Incompatible types in assignment (expression has type \"TextIOWrapper[_WrappedBuffer]\", variable has type \"BufferedReader[_BufferedReaderStream]\")" -} \ No newline at end of file + "gitgalaxy/galaxyscope.py: assignment @e42f3bf6e636#0": "Incompatible types in assignment (expression has type \"TextIOWrapper[_WrappedBuffer]\", variable has type \"BufferedReader[_BufferedReaderStream]\")" +} diff --git a/tests/ruff_audit.py b/tests/ruff_audit.py index de078cf490..135bee08c5 100644 --- a/tests/ruff_audit.py +++ b/tests/ruff_audit.py @@ -17,9 +17,11 @@ USAGE python tests/ruff_audit.py # full report, exits 1 if ruff - # finds anything -- use this to - # regenerate the baseline after - # a cleanup PR. + # finds anything. + python tests/ruff_audit.py --write-baseline + # regenerate ruff_audit_baseline.json + # from the current findings (e.g. + # after a cleanup PR). python tests/ruff_audit.py --ci # baseline-gated regression check # (see BASELINE below) plus the # zero-tolerance format check -- @@ -39,28 +41,29 @@ it would have drowned out every other rule family's signal. BASELINE -This repo had 241 pre-existing lint findings (240 unique baseline keys --- two share a `{file}:{line}: {code}` key, same rare-but-accepted -collision mypy_audit.py's docstring already documents) across 27 rule -families the day this check was wired into CI, measured AFTER a one-time +This repo had 241 pre-existing lint findings across 27 rule families the +day this check was wired into CI, measured AFTER a one-time `ruff check --fix` (+ `--unsafe-fixes` for the PEP 585 typing modernization specifically) and `ruff format` pass across the whole repo (see ruff_audit_baseline.json, and #469 for the adoption PR). `--ci` mode is a REGRESSION gate: it fails only on findings not already in the baseline. Fixing a baselined finding doesn't fail the build either -- shrinking the baseline is a deliberate, reviewable edit you -make yourself, not something this script does automatically. `--ci` -prints anything it notices has already been fixed as an FYI, so the -baseline doesn't silently go stale, but does not fail the build over it. - -SCOPE & LIMITATIONS (read before treating the baseline as static) -Baseline keys are `{file}:{line}: {code}`. Line numbers are NOT stable --- an unrelated edit earlier in a file shifts every finding below it, -which will look like "N new findings" even though nothing lint-relevant -changed. Same accepted tradeoff as mypy_audit.py's baseline. If a PR -that didn't touch lint-relevant code trips `--ci`, regenerate the -baseline (`python tests/ruff_audit.py` and copy its output into -ruff_audit_baseline.json) rather than treating it as a real regression. +make yourself (`--write-baseline`), not something `--ci` does +automatically. `--ci` prints anything it notices has already been fixed +as an FYI, so the baseline doesn't silently go stale, but does not fail +the build over it. + +BASELINE KEYS (#3384) +Keys are content-based, NOT line-based: +`{file}: {code} @{hash of the whitespace-stripped source line}#{occurrence}` +-- see tests/lint_baseline.py for the full scheme. An unrelated edit that +only shifts lines leaves every key (and so the baseline file) unchanged, +so the baseline no longer conflicts on rebase just because a sibling PR +moved code around. Editing the flagged line itself, or adding another +identical violating line in the same file (a new `#N` occurrence), IS a +new key and still fails `--ci`. Human-readable output still prints each +current finding's line number. """ import argparse @@ -68,28 +71,52 @@ import subprocess import sys from pathlib import Path -from typing import Dict + +# tests/ has no __init__.py -- make the sibling helper importable no matter how +# this file is loaded (script, audit_check.py, or a test's sys.path insert). +if str(Path(__file__).resolve().parent) not in sys.path: + sys.path.insert(0, str(Path(__file__).resolve().parent)) + +import lint_baseline +from lint_baseline import Finding REPO_ROOT = Path(__file__).resolve().parent.parent SCAN_ROOT = REPO_ROOT / "gitgalaxy" BASELINE_PATH = Path(__file__).resolve().parent / "ruff_audit_baseline.json" -def run_ruff_check() -> Dict[str, str]: - """Returns {"{file}:{line}: {code}": message} for every lint finding ruff reports.""" +def parse_ruff_json(stdout: str, repo_root: Path) -> dict[str, Finding]: + """Turns `ruff check --output-format=json` output into {content key: Finding}.""" + findings = [] + for item in json.loads(stdout or "[]"): + rel_path = Path(item["filename"]).resolve().relative_to(repo_root).as_posix() + location = item.get("location") or {} + findings.append( + Finding( + file=rel_path, + line=int(location.get("row") or 0), + column=int(location.get("column") or 0), + code=str(item.get("code")), + message=item["message"], + ) + ) + return lint_baseline.key_findings(findings, lint_baseline.source_reader(repo_root)) + + +def run_ruff_findings(scan_root: Path = SCAN_ROOT, repo_root: Path = REPO_ROOT) -> dict[str, Finding]: + """Returns {content key: Finding} for every lint finding ruff reports.""" result = subprocess.run( - ["ruff", "check", str(SCAN_ROOT), "--output-format=json"], + ["ruff", "check", str(scan_root), "--output-format=json"], capture_output=True, text=True, - cwd=REPO_ROOT, + cwd=repo_root, ) + return parse_ruff_json(result.stdout, repo_root) - errors: Dict[str, str] = {} - for item in json.loads(result.stdout or "[]"): - rel_path = Path(item["filename"]).resolve().relative_to(REPO_ROOT).as_posix() - key = f"{rel_path}:{item['location']['row']}: {item['code']}" - errors[key] = item["message"] - return errors + +def run_ruff_check() -> dict[str, str]: + """Returns {content key: message} -- the committed baseline's shape.""" + return lint_baseline.to_baseline(run_ruff_findings()) def run_ruff_format_check() -> bool: @@ -106,34 +133,44 @@ def run_ruff_format_check() -> bool: return result.returncode == 0 -def load_baseline() -> Dict[str, str]: - if not BASELINE_PATH.exists(): - return {} - with open(BASELINE_PATH, encoding="utf-8") as f: - return json.load(f) +def load_baseline() -> dict[str, str]: + return lint_baseline.load_baseline(BASELINE_PATH) -def _print_errors(errors: Dict[str, str]) -> None: - for key in sorted(errors): - print(f" {key} -- {errors[key]}") +def _print_findings(findings: dict[str, Finding]) -> None: + for key, finding in sorted(findings.items(), key=lambda kv: (kv[1].file, kv[1].line, kv[1].column, kv[0])): + print(f" {lint_baseline.describe(key, finding)}") def run_full_report() -> int: - errors = run_ruff_check() - if not errors: + findings = run_ruff_findings() + if not findings: print("Ruff Audit: no lint findings.") return 0 - print(f"Ruff Audit: {len(errors)} finding(s):\n") - _print_errors(errors) + print(f"Ruff Audit: {len(findings)} finding(s):\n") + _print_findings(findings) print( "\nTo accept this as the new baseline (e.g. after a cleanup PR), regenerate it with:\n" - ' python -c "from tests.ruff_audit import run_ruff_check; import json; ' - "json.dump(run_ruff_check(), open('tests/ruff_audit_baseline.json', 'w'), indent=2, sort_keys=True)\"" + " python tests/ruff_audit.py --write-baseline" ) return 1 +def write_current_baseline() -> int: + baseline = run_ruff_check() + lint_baseline.write_baseline(BASELINE_PATH, baseline) + print(f"Ruff Audit: wrote {len(baseline)}-finding baseline to {BASELINE_PATH.relative_to(REPO_ROOT)}.") + return 0 + + +def compare(findings: dict[str, Finding], baseline: dict[str, str]): + """Returns (new findings not in the baseline, sorted stale baseline keys no longer flagged).""" + new_findings = {key: f for key, f in findings.items() if key not in baseline} + resolved_keys = sorted(set(baseline) - set(findings)) + return new_findings, resolved_keys + + def run_ci_check() -> int: """ Baseline-gated regression check (#469) for lint, plus a zero-tolerance @@ -144,29 +181,30 @@ def run_ci_check() -> int: if not format_ok: print("Ruff Audit: files are not `ruff format`-compliant (zero-tolerance -- run `ruff format` locally).") - errors = run_ruff_check() + findings = run_ruff_findings() baseline = load_baseline() - - new_errors = {key: msg for key, msg in errors.items() if key not in baseline} - resolved_keys = sorted(set(baseline) - set(errors)) + new_findings, resolved_keys = compare(findings, baseline) if resolved_keys: - print("Ruff Audit: FYI -- these baselined findings are no longer flagged (fixed, or line-shifted).") - print("Consider removing them from ruff_audit_baseline.json in this PR:\n") + print( + "Ruff Audit: FYI -- these baselined findings are no longer flagged (fixed, or the flagged line was edited)." + ) + print("Consider removing them from ruff_audit_baseline.json in this PR (`--write-baseline`):\n") for key in resolved_keys: print(f" {key} -- {baseline[key]}") print() - if not new_errors: + if not new_findings: print(f"Ruff Audit: no NEW lint findings beyond the {len(baseline)}-finding baseline.") return 0 if format_ok else 1 - print(f"Ruff Audit: {len(new_errors)} NEW lint finding(s) beyond the {len(baseline)}-finding baseline:\n") - _print_errors(new_errors) + print(f"Ruff Audit: {len(new_findings)} NEW lint finding(s) beyond the {len(baseline)}-finding baseline:\n") + _print_findings(new_findings) print( - "\nEach hit above is either a real new lint finding to fix, or a baseline that needs " - "regenerating because unrelated edits shifted line numbers (see the module docstring's " - "SCOPE & LIMITATIONS section) -- confirm which before assuming it's a regression." + "\nBaseline keys are content-based (see the module docstring's BASELINE KEYS section), so a " + "pure line shift can't cause these: each is a new finding, a new duplicate of a baselined " + "line, or an edit to a baselined line. Fix it, or -- only if it's pre-existing debt you " + "deliberately carry -- re-bless with `python tests/ruff_audit.py --write-baseline`." ) return 1 @@ -174,8 +212,13 @@ def run_ci_check() -> int: def main() -> int: parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) parser.add_argument("--ci", action="store_true", help="Baseline-gated regression check (what CI runs).") + parser.add_argument( + "--write-baseline", action="store_true", help="Regenerate ruff_audit_baseline.json from current findings." + ) args = parser.parse_args() + if args.write_baseline: + return write_current_baseline() return run_ci_check() if args.ci else run_full_report() diff --git a/tests/ruff_audit_baseline.json b/tests/ruff_audit_baseline.json index 749ffde9f1..4ce0511f1a 100644 --- a/tests/ruff_audit_baseline.json +++ b/tests/ruff_audit_baseline.json @@ -1,7 +1,7 @@ { - "gitgalaxy/core/guidestar_lens.py:145: C401": "Unnecessary generator (rewrite as a set comprehension)", - "gitgalaxy/recorders/audit_recorder.py:482: C416": "Unnecessary dict comprehension (rewrite using `dict()`)", - "gitgalaxy/recorders/audit_recorder.py:499: C414": "Unnecessary `list()` call within `sorted()`", - "gitgalaxy/recorders/gpu_recorder.py:293: C414": "Unnecessary `list()` call within `sorted()`", - "gitgalaxy/security/security_auditor.py:366: RUF046": "Value being cast to `int` is already an integer" + "gitgalaxy/core/guidestar_lens.py: C401 @c173e1643439#0": "Unnecessary generator (rewrite as a set comprehension)", + "gitgalaxy/recorders/audit_recorder.py: C414 @e2de6a02ae7b#0": "Unnecessary `list()` call within `sorted()`", + "gitgalaxy/recorders/audit_recorder.py: C416 @a81d30627b75#0": "Unnecessary dict comprehension (rewrite using `dict()`)", + "gitgalaxy/recorders/gpu_recorder.py: C414 @2c826ccd023d#0": "Unnecessary `list()` call within `sorted()`", + "gitgalaxy/security/security_auditor.py: RUF046 @c35c68835c3a#0": "Value being cast to `int` is already an integer" } diff --git a/tests/tools/audit_check.py b/tests/tools/audit_check.py index 756d88979f..b60225c064 100644 --- a/tests/tools/audit_check.py +++ b/tests/tools/audit_check.py @@ -4,21 +4,26 @@ dead-key, ast-accuracy) plus the zero-tolerance ruff format check. Why this exists: after any regex/detector.py change, the established workflow -is to run all three `--ci` checks separately, then manually eyeball each -failure to decide "is this a real new finding, or did an earlier edit in the -same file just shift everything below it by N lines" (see each script's own -SCOPE & LIMITATIONS docstring section -- baseline keys embed line numbers, -which aren't stable). That eyeballing is mechanical (same file/code/message, -different line number) and was done by hand, from scratch, on every one of -this session's language passes -- exactly the kind of repeated judgment call -worth automating so it's not re-derived 42+ more times. +was to run all three `--ci` checks separately, then eyeball each failure to +decide whether it was a real new finding or an unrelated line shift. + +Since #3384, ruff/mypy baseline keys are content-based +(`{file}: {code} @{line-hash}#{occurrence}`, see tests/lint_baseline.py), so a +pure line shift no longer changes any key at all -- there is nothing to +regenerate for it. What's left for `--regenerate` to recognise mechanically is +an EDITED baselined line: a new key whose (file, code, message) exactly matches +a now-stale baseline entry (e.g. the flagged line was reformatted or touched, +but the same pre-existing finding is still there). Matching is one-for-one, so +a genuinely new duplicate on top of the old finding is still "genuine". USAGE python tests/tools/audit_check.py # run all checks, report python tests/tools/audit_check.py --regenerate # additionally: for any # audit whose new - # findings are ALL pure - # line-shifts, regenerate + # findings ALL pair + # one-for-one with a + # stale entry (same file/ + # code/message), regenerate # its baseline automatically. # Audits with a genuine # new finding are left @@ -31,9 +36,9 @@ always, without it), so it's safe to use as a pre-PR gate. """ -import json import subprocess import sys +from collections import Counter from pathlib import Path REPO_ROOT = Path(__file__).resolve().parent.parent.parent @@ -46,48 +51,56 @@ if str(TESTS_DIR) not in sys.path: sys.path.insert(0, str(TESTS_DIR)) +import lint_baseline # noqa: E402 import mypy_audit # noqa: E402 import ruff_audit # noqa: E402 def _identity(key: str, message: str) -> tuple: """ - Strips the line number out of a "{file}:{line}: {code}" baseline key, - leaving (file, code, message) -- the part of a finding's identity that's - stable across an unrelated line-shift elsewhere in the same file. + (file, code, message) of a content key (tests/lint_baseline.py) -- the part + of a finding's identity that survives an edit to the flagged line itself. """ - file_part, _, rest = key.partition(":") - code_part = rest.rsplit(": ", 1)[-1] if ": " in rest else rest + file_part, code_part, _digest, _occurrence = lint_baseline.parse_key(key) return (file_part, code_part, message) def _classify(new: dict, resolved: dict) -> tuple: - """Splits `new` findings into (pure_shifts, genuine) against `resolved`.""" - resolved_identities = {_identity(k, v) for k, v in resolved.items()} - shifts, genuine = {}, {} - for k, v in new.items(): - (shifts if _identity(k, v) in resolved_identities else genuine)[k] = v - return shifts, genuine + """Splits `new` findings into (edited_lines, genuine), pairing each new + finding with at most ONE stale entry of the same identity (multiset match), + so N+1 identical findings against N stale entries still leaves one genuine.""" + available = Counter(_identity(k, v) for k, v in resolved.items()) + edited, genuine = {}, {} + for k, v in sorted(new.items()): + ident = _identity(k, v) + if available[ident] > 0: + available[ident] -= 1 + edited[k] = v + else: + genuine[k] = v + return edited, genuine def _report(label: str, new: dict, resolved: dict, baseline_path: Path, regenerate: bool, current: dict) -> bool: if not new: return True - shifts, genuine = _classify(new, resolved) - if shifts: - print(f"[{label}] {len(shifts)} finding(s) look like pure line-shifts (same file/code/message, moved):") - for k, v in sorted(shifts.items()): + edited, genuine = _classify(new, resolved) + if edited: + print( + f"[{label}] {len(edited)} finding(s) look like an edited baselined line " + "(same file/code/message as a now-stale entry):" + ) + for k, v in sorted(edited.items()): print(f" {k} -- {v}") if genuine: - print(f"[{label}] {len(genuine)} finding(s) need real review (no matching baseline entry by message):") + print(f"[{label}] {len(genuine)} finding(s) need real review (no matching stale baseline entry):") for k, v in sorted(genuine.items()): print(f" {k} -- {v}") if regenerate and not genuine: - with open(baseline_path, "w") as baseline_file: - json.dump(current, baseline_file, indent=2, sort_keys=True) - print(f"[{label}] Regenerated baseline ({len(current)} findings, all were pure line-shifts).") + lint_baseline.write_baseline(baseline_path, current) + print(f"[{label}] Regenerated baseline ({len(current)} findings, all new keys were edited baselined lines).") return True return False @@ -180,7 +193,7 @@ def main() -> int: failing = [name for name, ok in results.items() if not ok] print(f"audit_check: {', '.join(failing)} need attention (see above).") if not regenerate: - print("Re-run with --regenerate to auto-fix any that are pure line-shifts.") + print("Re-run with --regenerate to auto-accept edited baselined lines (never genuine new findings).") return 1 diff --git a/tests/tools/rebase_rebless.py b/tests/tools/rebase_rebless.py index 73f125f54c..bd318e4a5a 100644 --- a/tests/tools/rebase_rebless.py +++ b/tests/tools/rebase_rebless.py @@ -10,7 +10,9 @@ 2. regenerate (bless) those artifact files against the rebased tree; 3. verify the resulting golden-master diff against main is limited to this branch's OWN new keys -- fail loudly on any other ("foreign") drift; - 4. refresh baseline entries for moved lines in files this branch touched; + 4. refresh lint baselines (audit_check.py --regenerate) scoped to files this + branch touched -- since #3384 ruff/mypy keys are content-based, so moved + lines need no refresh; this only re-accepts edited baselined lines; 5. run ruff check/format and a fast, touched-file-scoped test selection; 6. optionally `git push --force-with-lease` (never plain force). @@ -291,9 +293,11 @@ def regenerate_golden_masters(repo: Path) -> None: def refresh_lint_baselines(repo: Path) -> bool: """audit_check.py --regenerate only rewrites a baseline when EVERY new - finding is a pure line-shift (same file/code/message, just moved) -- a - genuine new finding anywhere is left untouched and still fails. Its exit - code also doubles as our ruff check/format gate.""" + finding pairs one-for-one with a now-stale entry of the same file/code/ + message (an edited baselined line; since #3384's content-keyed baselines a + pure line shift produces no new keys at all) -- a genuine new finding + anywhere is left untouched and still fails. Its exit code also doubles as + our ruff check/format gate.""" result = subprocess.run([PY, "tests/tools/audit_check.py", "--regenerate"], cwd=repo) return result.returncode == 0 @@ -437,7 +441,7 @@ def main(argv: list[str] | None = None) -> int: path = repo / rel_path before_baselines[rel_path] = json.loads(path.read_text(encoding="utf-8")) if path.is_file() else {} - print("[baselines] refreshing ruff/mypy baseline entries for moved lines, running ruff check/format ...") + print("[baselines] refreshing ruff/mypy baselines (edited lines only), running ruff check/format ...") lint_ok = refresh_lint_baselines(repo) scope_violations = verify_baseline_refresh_scope(repo, before_baselines, touched) diff --git a/tests/tools/test_content_keyed_baseline.py b/tests/tools/test_content_keyed_baseline.py new file mode 100644 index 0000000000..a330392804 --- /dev/null +++ b/tests/tools/test_content_keyed_baseline.py @@ -0,0 +1,270 @@ +""" +Content-keyed ruff/mypy baselines (#3384): tests/lint_baseline.py's key scheme, +as consumed by ruff_audit.py, mypy_audit.py and audit_check.py. + +Most tests feed ruff_audit.parse_ruff_json() synthetic `ruff --output-format=json` +output produced by a tiny fake linter over real temp files, so they exercise the +real parsing/keying/comparison code without depending on which rules the installed +ruff happens to ship. One test runs the real `ruff` binary when it's on PATH. +""" + +import json +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +TESTS_DIR = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(TESTS_DIR)) +import lint_baseline as lb +import mypy_audit +import ruff_audit + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +import audit_check + +REL = "pkg/mod.py" +PATTERN = "sorted(list(" + +BASE_SOURCE = """\ +import os + + +def a(xs): + return sorted(list(xs)) + + +def b(ys): + return sorted(list(ys)) +""" + + +def _fake_ruff_json(repo: Path) -> str: + """Emulates `ruff check --output-format=json`: one C414 per PATTERN occurrence.""" + items = [] + for path in sorted(repo.rglob("*.py")): + for row, text in enumerate(path.read_text(encoding="utf-8").splitlines(), start=1): + col = text.find(PATTERN) + while col != -1: + items.append( + { + "filename": str(path), + "location": {"row": row, "column": col + 1}, + "code": "C414", + "message": "Unnecessary `list()` call within `sorted()`", + } + ) + col = text.find(PATTERN, col + 1) + return json.dumps(items) + + +def _write(repo: Path, source: str) -> None: + path = repo / REL + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(source, encoding="utf-8") + + +def _findings(repo: Path) -> dict: + return ruff_audit.parse_ruff_json(_fake_ruff_json(repo), repo) + + +def _bless(repo: Path) -> str: + """What `ruff_audit.py --write-baseline` would write, as text.""" + return lb.dump_baseline(lb.to_baseline(_findings(repo))) + + +def test_key_shape_and_roundtrip(tmp_path): + _write(tmp_path, BASE_SOURCE) + keys = sorted(_findings(tmp_path)) + assert len(keys) == 2 + for key in keys: + # rebase_rebless.verify_baseline_refresh_scope relies on the file coming first. + assert key.split(":", 1)[0] == REL + file, code, digest, occurrence = lb.parse_key(key) + assert (file, code) == (REL, "C414") + assert len(digest) == lb.HASH_LEN + assert lb.format_key(file, code, digest, occurrence) == key + # different content -> different hashes, each #0 + assert {lb.parse_key(k)[3] for k in keys} == {0} + + +def test_unrelated_line_insertion_keeps_baseline_valid_and_byte_identical(tmp_path): + _write(tmp_path, BASE_SOURCE) + baseline_text = _bless(tmp_path) + before = _findings(tmp_path) + + shifted = BASE_SOURCE.replace("import os\n", "import os\nimport sys\n\nCONSTANT = 1\n\n\n") + _write(tmp_path, shifted) + after = _findings(tmp_path) + + # same keys, but the human-readable line numbers moved + assert set(after) == set(before) + assert [f.line for f in after.values()] != [f.line for f in before.values()] + assert "mod.py:10: C414" in lb.describe(*next(iter(after.items()))) + + new, stale = ruff_audit.compare(after, json.loads(baseline_text)) + assert new == {} and stale == [] + assert _bless(tmp_path) == baseline_text # regenerate is a byte-identical no-op + + +def test_reindent_and_respacing_is_stable(tmp_path): + _write(tmp_path, BASE_SOURCE) + baseline = json.loads(_bless(tmp_path)) + _write(tmp_path, BASE_SOURCE.replace(" return sorted(list(xs))", " return sorted(list(xs))")) + assert ruff_audit.compare(_findings(tmp_path), baseline) == ({}, []) + + +def test_new_duplicate_of_baselined_line_is_caught(tmp_path): + _write(tmp_path, BASE_SOURCE) + baseline = json.loads(_bless(tmp_path)) + + # an identical copy of an already-baselined violating line, ABOVE the original + dup = BASE_SOURCE.replace("def a(xs):\n", "def a0(xs):\n return sorted(list(xs))\n\n\ndef a(xs):\n") + _write(tmp_path, dup) + new, stale = ruff_audit.compare(_findings(tmp_path), baseline) + + assert stale == [] + assert len(new) == 1 + (key,) = new + assert lb.parse_key(key)[3] == 1 # occurrence #1 of the same (file, code, hash) + + +def test_changed_line_is_caught(tmp_path): + _write(tmp_path, BASE_SOURCE) + baseline = json.loads(_bless(tmp_path)) + + _write(tmp_path, BASE_SOURCE.replace("sorted(list(xs))", "sorted(list(xs), reverse=True)")) + new, stale = ruff_audit.compare(_findings(tmp_path), baseline) + assert len(new) == 1 and len(stale) == 1 + assert next(iter(new.values())).line == 5 + + +def test_removed_violation_is_reported_stale(tmp_path): + _write(tmp_path, BASE_SOURCE) + baseline = json.loads(_bless(tmp_path)) + + _write(tmp_path, BASE_SOURCE.replace("sorted(list(ys))", "sorted(ys)")) + new, stale = ruff_audit.compare(_findings(tmp_path), baseline) + assert new == {} + assert len(stale) == 1 and baseline[stale[0]].startswith("Unnecessary") + + +def test_two_findings_on_one_line_do_not_collapse(tmp_path): + _write(tmp_path, "x = sorted(list(a)) + sorted(list(b))\n") + keys = sorted(_findings(tmp_path)) + assert [lb.parse_key(k)[3] for k in keys] == [0, 1] + + +def test_ci_check_end_to_end(tmp_path, monkeypatch, capsys): + """run_ci_check(): pass after a shift, fail on a new duplicate, FYI for stale; prints current line numbers.""" + _write(tmp_path, BASE_SOURCE) + baseline_path = tmp_path / "baseline.json" + lb.write_baseline(baseline_path, lb.to_baseline(_findings(tmp_path))) + monkeypatch.setattr(ruff_audit, "BASELINE_PATH", baseline_path) + monkeypatch.setattr(ruff_audit, "run_ruff_format_check", lambda: True) + monkeypatch.setattr(ruff_audit, "run_ruff_findings", lambda: _findings(tmp_path)) + + _write(tmp_path, "\n\n" + BASE_SOURCE) + assert ruff_audit.run_ci_check() == 0 + + _write(tmp_path, "\n\n" + BASE_SOURCE + "\n\ndef c(zs):\n return sorted(list(ys))\n") + assert ruff_audit.run_ci_check() == 1 + out = capsys.readouterr().out + assert "1 NEW lint finding(s) beyond the 2-finding baseline" in out + assert "pkg/mod.py:15: C414" in out + + _write(tmp_path, "\n\n" + BASE_SOURCE.replace("sorted(list(xs))", "sorted(xs)")) + assert ruff_audit.run_ci_check() == 0 + assert "no longer flagged" in capsys.readouterr().out + + +def test_mypy_output_parsing_uses_same_scheme(tmp_path): + _write(tmp_path, BASE_SOURCE) + stdout = ( + f"{REL}:5: error: Bad thing [operator]\n" + f"{REL}:5: error: Other bad thing [operator]\n" + f"{REL}:9: note: ignored\n" + "Found 2 errors in 1 file (checked 1 source file)\n" + ) + keyed = mypy_audit.parse_mypy_output(stdout, tmp_path) + assert sorted(lb.parse_key(k)[3] for k in keyed) == [0, 1] + assert {f.line for f in keyed.values()} == {5} + shifted = stdout.replace(":5:", ":8:") + _write(tmp_path, "\n\n\n" + BASE_SOURCE) + assert set(mypy_audit.parse_mypy_output(shifted, tmp_path)) == set(keyed) + + +def test_audit_check_pairs_edited_lines_one_for_one(): + stale = {"f.py: C414 @aaaaaaaaaaaa#0": "msg"} + new = {"f.py: C414 @bbbbbbbbbbbb#0": "msg", "f.py: C414 @cccccccccccc#0": "msg"} + edited, genuine = audit_check._classify(new, stale) + assert len(edited) == 1 and len(genuine) == 1 + + +@pytest.mark.parametrize("name", ["ruff_audit_baseline.json", "mypy_audit_baseline.json"]) +def test_committed_baselines_are_canonical_content_keys(name): + path = TESTS_DIR / name + text = path.read_text(encoding="utf-8") + baseline = json.loads(text) + assert lb.dump_baseline(baseline) == text, f"{name} is not in canonical (sorted) form" + for key in baseline: + file, code, digest, _occ = lb.parse_key(key) + assert lb.format_key(*lb.parse_key(key)) == key, key + assert file.startswith("gitgalaxy/") and code and len(digest) == lb.HASH_LEN, key + + +@pytest.mark.skipif(shutil.which("ruff") is None, reason="ruff not on PATH") +def test_real_ruff_line_shift(tmp_path): + (tmp_path / "m.py").write_text("def f():\n x = 1\n", encoding="utf-8") + before = ruff_audit.run_ruff_findings(scan_root=tmp_path, repo_root=tmp_path) + assert any(f.code == "F841" for f in before.values()) + (tmp_path / "m.py").write_text("import os\n\nos.getcwd()\n\n\ndef f():\n x = 1\n", encoding="utf-8") + after = ruff_audit.run_ruff_findings(scan_root=tmp_path, repo_root=tmp_path) + assert set(after) == set(before) + + +def _git(repo: Path, *args: str) -> str: + result = subprocess.run(["git", *args], cwd=repo, text=True, capture_output=True) + assert result.returncode == 0, f"git {args} failed:\n{result.stdout}\n{result.stderr}" + return result.stdout + + +def _commit_all(repo: Path, message: str) -> None: + _git(repo, "add", "-A") + _git(repo, "commit", "-q", "-m", message) + + +@pytest.mark.skipif(shutil.which("git") is None, reason="git not on PATH") +def test_two_branches_shifting_lines_in_same_file_merge_baseline_conflict_free(tmp_path): + """#3384 acceptance: each branch shifts lines in the same file and re-blesses; the merge is clean.""" + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-q", "-b", "main") + _git(repo, "config", "user.email", "test@example.com") + _git(repo, "config", "user.name", "Test") + _git(repo, "config", "commit.gpgsign", "false") + + source = BASE_SOURCE + "\n\n# end\n" + _write(repo, source) + (repo / "baseline.json").write_text(_bless(repo), encoding="utf-8") + _commit_all(repo, "base") + base_baseline = (repo / "baseline.json").read_text(encoding="utf-8") + + _git(repo, "checkout", "-q", "-b", "left") + _write(repo, source.replace("import os\n", "import os\nimport re\nimport sys\n")) + (repo / "baseline.json").write_text(_bless(repo), encoding="utf-8") + _commit_all(repo, "left shifts lines") + + _git(repo, "checkout", "-q", "main") + _git(repo, "checkout", "-q", "-b", "right") + _write(repo, source.replace("# end\n", "# end\n\n\ndef helper():\n return 42\n")) + _write(repo, (repo / REL).read_text().replace("def b(ys):\n", "# note\n# note 2\ndef b(ys):\n")) + (repo / "baseline.json").write_text(_bless(repo), encoding="utf-8") + _commit_all(repo, "right shifts lines") + + _git(repo, "merge", "-q", "--no-edit", "left") # asserts no conflict + merged = (repo / "baseline.json").read_text(encoding="utf-8") + assert merged == base_baseline + assert _bless(repo) == merged From a29b2bdd7d5ff24222f82a41abade940aa369a73 Mon Sep 17 00:00:00 2001 From: Joe Esquibel Date: Wed, 23 Sep 2026 14:15:45 -0400 Subject: [PATCH 2/2] tests: import lint_baseline once (CodeQL mixed-import note) Finding is now aliased from the module instead of a second 'from lint_baseline import Finding', in ruff_audit.py and mypy_audit.py. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/mypy_audit.py | 3 ++- tests/ruff_audit.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/mypy_audit.py b/tests/mypy_audit.py index 73a4e65262..8c8bf456b8 100644 --- a/tests/mypy_audit.py +++ b/tests/mypy_audit.py @@ -67,7 +67,8 @@ sys.path.insert(0, str(Path(__file__).resolve().parent)) import lint_baseline -from lint_baseline import Finding + +Finding = lint_baseline.Finding REPO_ROOT = Path(__file__).resolve().parent.parent SCAN_ROOT = REPO_ROOT / "gitgalaxy" diff --git a/tests/ruff_audit.py b/tests/ruff_audit.py index 135bee08c5..b395e91e6c 100644 --- a/tests/ruff_audit.py +++ b/tests/ruff_audit.py @@ -78,7 +78,8 @@ sys.path.insert(0, str(Path(__file__).resolve().parent)) import lint_baseline -from lint_baseline import Finding + +Finding = lint_baseline.Finding REPO_ROOT = Path(__file__).resolve().parent.parent SCAN_ROOT = REPO_ROOT / "gitgalaxy"