From d93655925b5ffa5ee7771a9d5b0900392d2f7d5b Mon Sep 17 00:00:00 2001 From: Wolfvin Date: Thu, 2 Jul 2026 17:59:23 +0000 Subject: [PATCH] feat(diff): --diff-base flag scopes analysis to git-changed files (closes #157) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add global `--diff-base ` flag that restricts any analysis command to only report findings from files changed relative to a git ref. Designed for CI PR checks where only NEW findings introduced by the PR should appear in the output. New files: - scripts/diff_scope.py — DiffScope utility class (312 lines) - from_ref(workspace, ref) factory: validates ref, computes changed-file set via git_aware.get_changed_files() + get_untracked_files() - allows(path) — path-in-allowlist check (handles abs/rel, both sep) - filter_findings(findings, file_keys) — drops findings from unchanged files; tries multiple key names (file/path/defined_in/file_path); keeps findings with no file key (workspace-level findings) - summary() — dict for embedding in command output - Cross-platform path normalization (handles \ and / on any OS) - tests/test_diff_scope.py — 44 tests (443 lines) - Construction, allows(), filter_findings() with all key variants - from_ref() against real temp git repos (uncommitted, untracked, HEAD~1, branch names — auto-detects default branch main/master) - Error cases: invalid ref, empty ref, non-git dir, missing workspace - CLI integration via subprocess: --help, invalid ref exit code, diff_scope in JSON output, empty diff early-exit, before-subcommand Modified files: - scripts/codelens.py (122 lines added): - Add --diff-base to global parser + every subparser (matches --db-path pattern) so it works both before and after the subcommand - Pre-parse loop recognizes --diff-base and --diff-base= - Build DiffScope after workspace resolution; attach to args - Empty diff → early exit with clear JSON message + exit 0 - Invalid ref → JSON error + exit 1 - Post-filter: after command runs, filter findings/leaks/hints/issues/ violations/matches/chains/results by changed-file set - Attach diff_scope summary (base_ref, changed_files, before/after counts) to result so CI/agents can see what was filtered - Graph-producing commands (scan, trace, impact, circular) are NOT filtered — their results are structural, filtering would corrupt - SKILL.md — add 'Diff-Scoped Analysis' to v8.2 capability pillars - SKILL-QUICK.md — add --diff-base row to flags table Design decisions: - POST-FILTER layer, not pre-filter. Commands still scan the full workspace, but findings from unchanged files are removed before output. This avoids touching every engine's os.walk() (collision risk with other PRs) and keeps the change to one file (codelens.py). Engine-level pre-filtering can be added incrementally later. - DiffScope is a reusable utility class — PR #141 (issue #57) can adopt it to replace the per-command --diff-vs/--diff-scan/--staged flags. - Graph commands excluded from filtering — filtering node/edge lists by file would silently corrupt the graph structure. - Findings with no file key (workspace-level) are KEPT — they may be legitimate (e.g., 'no .gitignore found'). Verified: - 44 new tests pass (test_diff_scope.py) - 173 passed regression check (test_formatters + test_cli + test_command_count + test_command_registry + test_git_aware — no regressions) - Command count unchanged at 70 (flag, not new command) - End-to-end: invalid ref exits 1, valid ref filters findings, empty diff early-exits, --diff-base works before and after subcommand Findings (per pre-flight SKILL.md — flag to BOS): - PR #141 (issue #57, open) implements similar functionality for the `check` command ONLY (--diff-scan, --staged, --diff-vs flags). This PR's --diff-base is a GLOBAL flag for ALL analysis commands. The two are complementary: PR #141 could adopt DiffScope to replace its per-command flags. No file collision (PR #141 touches check.py + baseline_diff.py; this PR touches codelens.py + new diff_scope.py). --- SKILL-QUICK.md | 1 + SKILL.md | 8 + scripts/codelens.py | 122 +++++++++++ scripts/diff_scope.py | 312 +++++++++++++++++++++++++++ tests/test_diff_scope.py | 443 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 886 insertions(+) create mode 100644 scripts/diff_scope.py create mode 100644 tests/test_diff_scope.py diff --git a/SKILL-QUICK.md b/SKILL-QUICK.md index f7944813..1fa77d35 100755 --- a/SKILL-QUICK.md +++ b/SKILL-QUICK.md @@ -32,6 +32,7 @@ $CLI list --limit 5 --offset 10 --format compact # → paginated + co | `--limit N` / `--offset N` | Pagination on list-type commands (`list`, `search`, `trace`, `symbols`, `outline`). Default limit=20 (issue #17). `--top N` is an alias for `--limit N --offset 0` | | `--deep` | Enable LSP-enhanced deep analysis (requires language server; check with `lsp-status`) | | `--db-path PATH` | Custom SQLite database path (default: `.codelens/codelens.db`) | +| `--diff-base REF` | Git ref (branch/tag/SHA/`HEAD~1`) to diff against. Only findings from files changed relative to REF are reported. Empty diff → early exit. Useful for CI PR checks. Works on all analysis commands (issue #157) | ### Lite Mode Per Command diff --git a/SKILL.md b/SKILL.md index 77b8be13..5ccdfd59 100755 --- a/SKILL.md +++ b/SKILL.md @@ -336,5 +336,13 @@ CodeLens v8.0+ adds 7 major capability pillars over v7.x: 5. **Cross-File Dataflow Engine** (`dataflow` v2) — Workspace-wide call graph with import resolution (`from/import`, `require` destructuring) and bidirectional taint propagation. 6. **OWASP Top 10 + Compliance Mapping** — 89 rules total (A01-A10 + PCI-DSS requirements 1-12 + HIPAA 45 CFR § 164.312). 7. **CI/CD Quality Gate** (`check` command) — Exits non-zero on failure, SARIF output for GitHub Advanced Security / VS Code. +8. **Diff-Scoped Analysis** (`--diff-base REF` flag, issue #157) — Global flag that restricts any analysis command to only report findings from files changed relative to a git ref (branch/tag/SHA/`HEAD~1`). Empty diff → early exit with clear message. Invalid ref → clear error + non-zero exit. Useful for CI PR checks where only NEW findings introduced by the PR should fail the gate. + +```bash +# CI PR check — only findings from files changed vs main +codelens check . --diff-base origin/main --format sarif > codelens.sarif +codelens taint . --diff-base origin/main +codelens secrets . --diff-base HEAD~1 +``` v8.1 follows up with F1 benchmark improvements (avg F1 0.803 → 0.872), circular engine depth fixes (F1 0.667 → 1.000), dead-code engine fixes (F1 0.800 → 0.952), and AST taint depth enhancements (return-value propagation, scope-hierarchical TaintState, branch condition refinement). diff --git a/scripts/codelens.py b/scripts/codelens.py index 437d3f79..1c88a516 100755 --- a/scripts/codelens.py +++ b/scripts/codelens.py @@ -907,6 +907,13 @@ def main(): sub.add_argument("--db-path", default=None, metavar="PATH", help="Custom path for SQLite database file") + # Issue #157: --diff-base on every subparser so it works both + # before and after the subcommand (matches --db-path / --format pattern). + if "diff_base" not in existing_dests: + sub.add_argument("--diff-base", default=None, metavar="REF", + help="Git ref to diff against — only findings from " + "changed files are reported (issue #157)") + # Global format option (works before subcommand) # Default: "ai" if CODELENS_AI_MODE is set (for AI consumers), else "json" _default_format = "ai" if os.environ.get("CODELENS_AI_MODE", "").lower() in ("1", "true", "yes") else "json" @@ -914,6 +921,14 @@ def main(): help=f"Output format (default: {_default_format}. Set CODELENS_AI_MODE=1 for ai default. compact = token-efficient single-char keys)") parser.add_argument("--db-path", default=None, help="Custom path for SQLite database (default: .codelens/codelens.db)") + # Issue #157: --diff-base restricts analysis to files changed + # relative to . Pre-filter layer: commands still scan the full + # workspace, but findings from unchanged files are filtered out of + # the result. Empty diff → early exit with a clear message. + parser.add_argument("--diff-base", default=None, metavar="REF", + help="Git ref (branch/tag/SHA/HEAD~1) to diff against. " + "Only findings from files changed relative to REF " + "are reported. Useful for CI PR checks. (issue #157)") # ─── Parse and dispatch ───────────────────────────── @@ -943,6 +958,7 @@ def main(): global_deep = False global_disable_suppression = False global_ignore_pattern = None + global_diff_base = None # issue #157 i = 1 while i < len(sys.argv): @@ -985,6 +1001,12 @@ def main(): global_ignore_pattern = sys.argv[i + 1] elif arg.startswith('--codelens-ignore-pattern='): global_ignore_pattern = arg.split('=', 1)[1] + # Issue #157: --diff-base (space form) + elif arg == '--diff-base' and i + 1 < len(sys.argv): + global_diff_base = sys.argv[i + 1] + # Issue #157: --diff-base= (equals form) + elif arg.startswith('--diff-base='): + global_diff_base = arg.split('=', 1)[1] i += 1 args = parser.parse_args() @@ -1034,6 +1056,53 @@ def main(): if workspace != (getattr(args, 'workspace', None) or ""): print(f"[CodeLens] Auto-detected workspace: {workspace}", file=sys.stderr) + # ─── Issue #157: --diff-base ─────────────────────────── + # Build the DiffScope once, before command execution. If the diff is + # empty, early-exit with a clear message. The scope is attached to + # ``args`` so commands that want to do in-engine pre-filtering can + # access it (none do yet — this is a post-filter layer for now). + diff_scope = None + # Resolve --diff-base: global pre-parse value or subparser value. + # argparse stores --diff-base as ``diff_base`` on the subparser too, + # but since it's a global flag, the pre-parse value is authoritative. + diff_base_ref = global_diff_base or getattr(args, 'diff_base', None) + if diff_base_ref: + from diff_scope import DiffScope, DiffScopeError + try: + diff_scope = DiffScope.from_ref(workspace, diff_base_ref) + except DiffScopeError as exc: + error_result = { + "status": "error", + "command": args.command, + "error": str(exc), + "error_type": "diff_scope_error", + "suggestion": ( + "Ensure the workspace is a git repository and the ref " + "exists. Use `git rev-parse --verify ` to check." + ), + } + print(format_output(error_result, args.format, args.command), file=sys.stderr) + sys.exit(1) + if diff_scope.is_empty: + # Empty diff → early exit per issue #157 DoD + empty_result = { + "status": "ok", + "command": args.command, + "message": f"No changed files relative to {diff_base_ref!r}", + "diff_scope": diff_scope.summary(), + "stats": {}, + "findings": [], + } + print(format_output(empty_result, args.format, args.command, workspace)) + sys.exit(0) + # Attach to args so commands can opt-in to in-engine pre-filtering + args.diff_scope = diff_scope + print( + f"[CodeLens] --diff-base {diff_base_ref!r}: {diff_scope.changed_count} " + f"file(s) in scope", + file=sys.stderr, + ) + # ─── Auto-setup: if command needs registry and none exists, bootstrap it ──── # Commands that need a registry to work meaningfully _REGISTRY_COMMANDS = { @@ -1276,6 +1345,59 @@ def main(): except Exception as e: logger.warning(f"Suppression processing failed: {e}", exc_info=True) + # ─── Issue #157: --diff-base post-filter ── + # Drop findings from files not in the changed-file allowlist. This + # is a post-filter layer — commands still scan the full workspace, + # but findings from unchanged files are removed before output. + # Commands that produce graph data (trace, impact, circular, scan) + # are NOT filtered because their results are structural (node/edge + # graphs) rather than file-keyed findings — filtering them would + # silently corrupt the graph. + if diff_scope is not None and isinstance(result, dict) and args.command in ( + "secrets", "smell", "complexity", "dead-code", "debug-leak", + "circular", "taint", "vuln-scan", "check", "analyze", + "missing-refs", "side-effect", "perf-hint", "regex-audit", + "a11y", "css-deep", "dataflow", "stack-trace", "config-drift", + "ownership", "test-map", + ): + _FILTER_KEYS = ( + "findings", "leaks", "hints", "issues", "violations", + "matches", "chains", "results", + ) + total_before = 0 + total_after = 0 + for key in _FILTER_KEYS: + val = result.get(key) + if isinstance(val, list): + before = len(val) + result[key] = diff_scope.filter_findings(val) + total_before += before + total_after += len(result[key]) + elif isinstance(val, dict): + # Category-keyed (dead-code by_category, smell by_category) + for sub_key, sub_val in val.items(): + if isinstance(sub_val, list): + before = len(sub_val) + val[sub_key] = diff_scope.filter_findings(sub_val) + total_before += before + total_after += len(val[sub_key]) + # Also filter the flat ``findings`` list that some commands + # (e.g., ``check``) produce at the top level. + if "findings" in result and isinstance(result["findings"], list): + # Already filtered above if ``findings`` is in _FILTER_KEYS, + # but ``check`` stores them under ``findings`` — covered. + pass + # Attach diff_scope summary so consumers can see what was filtered + result["diff_scope"] = diff_scope.summary() + result["diff_scope"]["findings_before_filter"] = total_before + result["diff_scope"]["findings_after_filter"] = total_after + if total_before != total_after: + print( + f"[CodeLens] --diff-base: {total_before - total_after} " + f"finding(s) from unchanged files filtered out", + file=sys.stderr, + ) + # ─── Format and print output ── # Some commands (doctor issue #64 Phase 1, sessions issue #64 # Phase 2) print their own human-readable output directly and diff --git a/scripts/diff_scope.py b/scripts/diff_scope.py new file mode 100644 index 00000000..2c68528f --- /dev/null +++ b/scripts/diff_scope.py @@ -0,0 +1,312 @@ +# @WHO: scripts/diff_scope.py +# @WHAT: DiffScope — git-diff-based file allowlist for --diff-base flag (issue #157) +# @PART: utils +# @ENTRY: DiffScope.from_ref() +""" +DiffScope — restrict CodeLens analysis to git-changed files only. + +Issue #157: ``--diff-base `` global flag. When set, CodeLens should +only report findings from files that changed relative to ````. This +matters in CI: a PR check should flag only NEW issues introduced by the +PR, not pre-existing issues in unchanged files. + +Design +------ +DiffScope is a thin wrapper around ``git_aware.get_changed_files()`` (and +``get_untracked_files()``). It validates the ref, computes the changed-file +set, and exposes: + +- ``DiffScope.from_ref(workspace, ref)`` — factory; returns a DiffScope or + raises ``DiffScopeError`` on invalid ref / not-a-git-repo +- ``scope.changed_files`` — frozenset of relative paths +- ``scope.is_empty`` — True if the diff is empty (caller should early-exit) +- ``scope.allows(path)`` — True if ``path`` is in the changed-file set +- ``scope.filter_findings(findings, file_key=...)`` — drop findings whose + file is not in the changed-file set + +The class is intentionally pure (no I/O beyond the one-time git diff call +in the factory). ``filter_findings`` handles both relative and absolute +file paths — engines are inconsistent (secrets uses rel_path, check uses +absolute file_path for rule-engine findings), so the filter normalizes both +to relative paths before comparing. + +@FLOW: DIFF_SCOPE_FILTER +@CALLS: git_aware.get_changed_files() -> List[str] +@CALLS: git_aware.get_untracked_files() -> List[str] +@MUTATES: none (pure utility — reads git, returns data) +""" + +from __future__ import annotations + +import os +from typing import Any, Dict, FrozenSet, Iterable, List, Optional + +# git_aware is in the same scripts/ directory; the CLI adds scripts/ to +# sys.path before importing. Lazy import so this module can be imported +# in test contexts where git_aware isn't on the path yet. + + +class DiffScopeError(Exception): + """Raised when a DiffScope cannot be constructed. + + Common causes: + - The workspace is not a git repository + - The ref does not exist (typo, wrong branch name) + - Git is not installed + """ + + +class DiffScope: + """An immutable allowlist of git-changed files for one workspace. + + Construct via :meth:`from_ref`. Once constructed, the instance is + safe to share across commands — the changed-file set is captured at + construction time and does not change. + """ + + __slots__ = ("_workspace", "_changed_files", "_base_ref") + + def __init__(self, workspace: str, changed_files: Iterable[str]) -> None: + self._workspace = os.path.abspath(workspace) + self._base_ref: Optional[str] = None + # Normalize: relative paths, forward slashes for cross-platform compare. + # Handle BOTH separators (os.sep + altsep) so backslash paths from + # Windows are normalized to forward slashes on any platform. + normalized = set() + for p in changed_files: + if not p: + continue + # Store as relative path with OS-native separators + if os.path.isabs(p): + try: + p = os.path.relpath(p, self._workspace) + except ValueError: + # On Windows, relpath across drives raises — keep as-is + pass + # Normalize to forward slashes for stable comparison. + # Replace both os.sep and os.altsep (Windows: \ and /). + p = p.replace("\\", "/") + if os.altsep and os.altsep != "/": + p = p.replace(os.altsep, "/") + normalized.add(p) + self._changed_files: FrozenSet[str] = frozenset(normalized) + + @property + def workspace(self) -> str: + """Absolute path to the workspace root.""" + return self._workspace + + @property + def changed_files(self) -> FrozenSet[str]: + """FrozenSet of changed file paths (relative, forward-slash).""" + return self._changed_files + + @property + def is_empty(self) -> bool: + """True if no files changed relative to the base ref. + + Callers should check this and early-exit with a clear message + rather than running analysis that would produce zero findings. + """ + return len(self._changed_files) == 0 + + @property + def changed_count(self) -> int: + """Number of changed files.""" + return len(self._changed_files) + + def allows(self, path: str) -> bool: + """Return True if ``path`` is in the changed-file allowlist. + + Handles both absolute and relative paths. Paths are normalized + to relative + forward-slash before comparison so the check works + cross-platform. + + Args: + path: A file path (absolute or relative to workspace). + + Returns: + True if the path is in the changed-file set. + """ + if not path: + return False + # Normalize to relative + forward slash (same logic as __init__) + p = path + if os.path.isabs(p): + try: + p = os.path.relpath(p, self._workspace) + except ValueError: + # Windows cross-drive — fall through with original + pass + p = p.replace("\\", "/") + if os.altsep and os.altsep != "/": + p = p.replace(os.altsep, "/") + return p in self._changed_files + + def filter_findings( + self, + findings: List[Dict[str, Any]], + file_keys: Optional[List[str]] = None, + ) -> List[Dict[str, Any]]: + """Drop findings whose file is not in the changed-file allowlist. + + Engines are inconsistent about the key name for the file path + (``file``, ``path``, ``defined_in``, ``file_path``) and about + whether it's absolute or relative. This function tries a list + of keys in order and uses the first one present on each finding. + + Findings with no recognizable file key are KEPT — they may be + workspace-level findings (e.g., "no .gitignore found") that + shouldn't be filtered out by a file-based diff scope. + + Args: + findings: List of finding dicts. + file_keys: Optional list of keys to try (default: ``file``, + ``path``, ``defined_in``, ``file_path``). + + Returns: + New list containing only findings from changed files (or + findings with no file key). + """ + if file_keys is None: + file_keys = ["file", "path", "defined_in", "file_path"] + + kept: List[Dict[str, Any]] = [] + for f in findings: + if not isinstance(f, dict): + kept.append(f) + continue + file_path: Optional[str] = None + for key in file_keys: + val = f.get(key) + if isinstance(val, str) and val: + file_path = val + break + if file_path is None: + # No file key — keep workspace-level findings + kept.append(f) + continue + if self.allows(file_path): + kept.append(f) + return kept + + def summary(self) -> Dict[str, Any]: + """Return a dict summary suitable for embedding in command output. + + Commands should add this to their result dict under a + ``diff_scope`` key so consumers (CI, agents) can see which files + were in scope. + """ + return { + "base_ref": self._base_ref, + "changed_files": sorted(self._changed_files), + "changed_count": self.changed_count, + "workspace": self._workspace, + } + + # ─── Factory ───────────────────────────────────────────── + + @classmethod + def from_ref( + cls, + workspace: str, + ref: str, + include_untracked: bool = True, + ) -> "DiffScope": + """Construct a DiffScope by diffing HEAD against ``ref``. + + Args: + workspace: Path to the workspace root (must be a git repo). + ref: Git ref to diff against (branch name, tag, SHA, ``HEAD~1``, + ``origin/main``, etc.). + include_untracked: If True (default), also include untracked + files (newly created files not yet ``git add``-ed). These + are part of the working-tree changes and should be in scope. + + Returns: + A DiffScope instance. + + Raises: + DiffScopeError: If the workspace is not a git repo, git is + unavailable, or ``ref`` does not exist. + """ + if not ref: + raise DiffScopeError("--diff-base requires a non-empty ref argument") + + workspace = os.path.abspath(workspace) + if not os.path.isdir(workspace): + raise DiffScopeError( + f"Workspace does not exist or is not a directory: {workspace}" + ) + + # Lazy import so this module can be imported in test contexts + try: + from git_aware import get_changed_files, get_untracked_files + except ImportError as exc: + raise DiffScopeError( + f"git_aware module unavailable — cannot compute diff: {exc}" + ) from exc + + # Validate the ref BEFORE calling get_changed_files. + # get_changed_files returns [] on invalid ref (same as "no changes"), + # which would silently produce an empty scope. We need to distinguish + # "invalid ref" from "valid ref with no changes". + _validate_git_ref(workspace, ref) + + changed = get_changed_files(workspace, since_sha=ref) + + if include_untracked: + untracked = get_untracked_files(workspace) + # Untracked files are returned as absolute paths by get_untracked_files + changed = list(changed) + [ + os.path.relpath(p, workspace) if os.path.isabs(p) else p + for p in untracked + ] + + scope = cls(workspace, changed) + # Attach the base ref so summary() can report it + scope._base_ref = ref + return scope + + +# ─── Internal helpers ──────────────────────────────────────── + + +def _validate_git_ref(workspace: str, ref: str) -> None: + """Verify that ``ref`` exists in the git repo at ``workspace``. + + Raises ``DiffScopeError`` if git is unavailable, the workspace is not + a git repo, or ``ref`` does not resolve to a valid commit. + + Args: + workspace: Absolute path to workspace root. + ref: Git ref (branch, tag, SHA, HEAD~1, etc.). + """ + import subprocess + + try: + result = subprocess.run( + ["git", "rev-parse", "--verify", ref], + cwd=workspace, + capture_output=True, + text=True, + timeout=10, + ) + except FileNotFoundError as exc: + raise DiffScopeError( + f"git command not found — cannot validate ref {ref!r}: {exc}" + ) from exc + except subprocess.TimeoutExpired as exc: + raise DiffScopeError( + f"git rev-parse timed out validating ref {ref!r}: {exc}" + ) from exc + except Exception as exc: + raise DiffScopeError( + f"Unexpected error validating ref {ref!r}: {exc}" + ) from exc + + if result.returncode != 0: + stderr = (result.stderr or "").strip() + raise DiffScopeError( + f"Invalid git ref {ref!r}: {stderr or 'ref does not exist'}" + ) diff --git a/tests/test_diff_scope.py b/tests/test_diff_scope.py new file mode 100644 index 00000000..0e59e01c --- /dev/null +++ b/tests/test_diff_scope.py @@ -0,0 +1,443 @@ +"""Tests for scripts/diff_scope.py — issue #157. + +Tests the DiffScope class and its factory from_ref(), including: +- Construction with explicit file lists +- Path normalization (absolute ↔ relative, OS separators) +- filter_findings with various file key names +- from_ref against real temporary git repos +- Error cases: invalid ref, empty ref, non-git directory, missing workspace +- Empty diff detection +- Summary dict structure +""" + +import os +import shutil +import subprocess +import sys +import tempfile +import unittest + +SCRIPT_DIR = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "scripts") +sys.path.insert(0, SCRIPT_DIR) + +from diff_scope import DiffScope, DiffScopeError + + +# ─── 1. Direct construction ────────────────────────────────── + + +class TestConstruction(unittest.TestCase): + """DiffScope constructed directly with a file list (no git).""" + + def test_basic_construction(self): + scope = DiffScope("/tmp/work", ["src/a.py", "src/b.py"]) + self.assertEqual(scope.workspace, "/tmp/work") + self.assertEqual(scope.changed_count, 2) + self.assertFalse(scope.is_empty) + + def test_absolute_paths_normalized_to_relative(self): + scope = DiffScope("/tmp/work", ["/tmp/work/src/a.py"]) + # Stored as relative with forward slashes + self.assertIn("src/a.py", scope.changed_files) + + def test_empty_file_list_is_empty_scope(self): + scope = DiffScope("/tmp/work", []) + self.assertTrue(scope.is_empty) + self.assertEqual(scope.changed_count, 0) + + def test_empty_strings_in_list_are_skipped(self): + scope = DiffScope("/tmp/work", ["", "src/a.py", ""]) + self.assertEqual(scope.changed_count, 1) + + def test_paths_normalized_to_forward_slash(self): + """Backslash separators (Windows) normalized to forward slash.""" + scope = DiffScope("/tmp/work", ["src\\nested\\a.py"]) + self.assertIn("src/nested/a.py", scope.changed_files) + + def test_changed_files_is_frozenset(self): + scope = DiffScope("/tmp/work", ["src/a.py"]) + self.assertIsInstance(scope.changed_files, frozenset) + + def test_workspace_is_absolute(self): + scope = DiffScope("relative/path", ["a.py"]) + self.assertTrue(os.path.isabs(scope.workspace)) + + +# ─── 2. allows() ───────────────────────────────────────────── + + +class TestAllows(unittest.TestCase): + """DiffScope.allows() path-matching logic.""" + + def setUp(self): + self.scope = DiffScope("/tmp/work", ["src/a.py", "src/b.py"]) + + def test_relative_path_in_set(self): + self.assertTrue(self.scope.allows("src/a.py")) + + def test_absolute_path_in_set(self): + self.assertTrue(self.scope.allows("/tmp/work/src/a.py")) + + def test_relative_path_not_in_set(self): + self.assertFalse(self.scope.allows("src/c.py")) + + def test_empty_path(self): + self.assertFalse(self.scope.allows("")) + + def test_none_path(self): + self.assertFalse(self.scope.allows(None)) # type: ignore[arg-type] + + def test_backslash_path_matches(self): + """Windows-style backslash path should match forward-slash entry.""" + scope = DiffScope("/tmp/work", ["src/a.py"]) + self.assertTrue(scope.allows("src\\a.py")) + + +# ─── 3. filter_findings() ──────────────────────────────────── + + +class TestFilterFindings(unittest.TestCase): + """DiffScope.filter_findings() drops findings from unchanged files.""" + + def setUp(self): + self.scope = DiffScope("/tmp/work", ["src/a.py", "src/b.py"]) + + def test_filters_by_file_key(self): + findings = [ + {"file": "src/a.py", "msg": "keep"}, + {"file": "src/unchanged.py", "msg": "drop"}, + ] + kept = self.scope.filter_findings(findings) + self.assertEqual(len(kept), 1) + self.assertEqual(kept[0]["msg"], "keep") + + def test_filters_by_path_key(self): + findings = [ + {"path": "src/a.py", "msg": "keep"}, + {"path": "src/unchanged.py", "msg": "drop"}, + ] + kept = self.scope.filter_findings(findings) + self.assertEqual(len(kept), 1) + + def test_filters_by_defined_in_key(self): + findings = [ + {"defined_in": "src/a.py", "msg": "keep"}, + {"defined_in": "src/unchanged.py", "msg": "drop"}, + ] + kept = self.scope.filter_findings(findings) + self.assertEqual(len(kept), 1) + + def test_filters_by_file_path_key(self): + findings = [ + {"file_path": "src/a.py", "msg": "keep"}, + {"file_path": "src/unchanged.py", "msg": "drop"}, + ] + kept = self.scope.filter_findings(findings) + self.assertEqual(len(kept), 1) + + def test_absolute_file_path_matches_relative_entry(self): + findings = [ + {"file": "/tmp/work/src/a.py", "msg": "keep"}, + {"file": "/tmp/work/src/unchanged.py", "msg": "drop"}, + ] + kept = self.scope.filter_findings(findings) + self.assertEqual(len(kept), 1) + + def test_findings_without_file_key_are_kept(self): + """Workspace-level findings (no file key) should be kept.""" + findings = [ + {"msg": "no .gitignore found", "severity": "low"}, + {"file": "src/a.py", "msg": "keep"}, + ] + kept = self.scope.filter_findings(findings) + self.assertEqual(len(kept), 2) + + def test_empty_file_value_treated_as_no_file(self): + """A finding with file="" should be kept (treated as no file key).""" + findings = [ + {"file": "", "msg": "keep"}, + {"file": "src/a.py", "msg": "keep"}, + ] + kept = self.scope.filter_findings(findings) + self.assertEqual(len(kept), 2) + + def test_empty_findings_list(self): + kept = self.scope.filter_findings([]) + self.assertEqual(kept, []) + + def test_non_dict_findings_kept(self): + """Non-dict entries (strings, None) are kept — caller's responsibility.""" + findings = ["raw string", None, {"file": "src/a.py", "msg": "keep"}] + kept = self.scope.filter_findings(findings) + self.assertEqual(len(kept), 3) + + def test_custom_file_keys_override_defaults(self): + """When custom file_keys are specified, only those keys are checked. + + A finding with a ``file`` key (default key) but no ``custom_key`` + should be KEPT (treated as no-file because the custom key isn't + present). + """ + findings = [ + {"custom_key": "src/a.py", "msg": "keep"}, + {"custom_key": "src/unchanged.py", "msg": "drop"}, + {"file": "src/a.py", "msg": "keep (no custom_key → treated as no-file)"}, + ] + kept = self.scope.filter_findings(findings, file_keys=["custom_key"]) + self.assertEqual(len(kept), 2) + msgs = {f["msg"] for f in kept} + self.assertIn("keep", msgs) + self.assertIn("keep (no custom_key → treated as no-file)", msgs) + + def test_does_not_mutate_input(self): + findings = [{"file": "src/a.py"}, {"file": "src/unchanged.py"}] + original = [dict(f) for f in findings] + self.scope.filter_findings(findings) + self.assertEqual(findings, original) + + +# ─── 4. summary() ──────────────────────────────────────────── + + +class TestSummary(unittest.TestCase): + """DiffScope.summary() returns a dict suitable for embedding in output.""" + + def test_summary_structure(self): + scope = DiffScope("/tmp/work", ["src/a.py", "src/b.py"]) + s = scope.summary() + self.assertIn("base_ref", s) + self.assertIn("changed_files", s) + self.assertIn("changed_count", s) + self.assertIn("workspace", s) + + def test_summary_changed_files_sorted(self): + scope = DiffScope("/tmp/work", ["src/b.py", "src/a.py"]) + s = scope.summary() + self.assertEqual(s["changed_files"], ["src/a.py", "src/b.py"]) + + def test_summary_base_ref_none_when_not_from_ref(self): + scope = DiffScope("/tmp/work", ["src/a.py"]) + self.assertIsNone(scope.summary()["base_ref"]) + + +# ─── 5. from_ref() against real git repos ─────────────────── + + +class TestFromRefRealGit(unittest.TestCase): + """DiffScope.from_ref() against real temporary git repositories.""" + + def setUp(self): + """Create a temp git repo with a couple of commits.""" + self.tmpdir = tempfile.mkdtemp(prefix="codelens_diffscope_test_") + self._run("git", "init", cwd=self.tmpdir) + self._run("git", "config", "user.email", "test@test.com", cwd=self.tmpdir) + self._run("git", "config", "user.name", "test", cwd=self.tmpdir) + # Initial commit + self._write("file1.py", "print('hello')\n") + self._write("file2.py", "print('world')\n") + self._run("git", "add", ".", cwd=self.tmpdir) + self._run("git", "commit", "-m", "initial", cwd=self.tmpdir) + self.first_sha = self._run("git", "rev-parse", "HEAD", cwd=self.tmpdir).strip() + + def tearDown(self): + shutil.rmtree(self.tmpdir, ignore_errors=True) + + def _run(self, *cmd, cwd=None): + return subprocess.run( + cmd, cwd=cwd, capture_output=True, text=True, check=True + ).stdout + + def _write(self, name, content): + with open(os.path.join(self.tmpdir, name), "w") as f: + f.write(content) + + def test_from_ref_with_no_changes(self): + """HEAD vs HEAD (no uncommitted changes) → empty diff.""" + scope = DiffScope.from_ref(self.tmpdir, "HEAD") + self.assertTrue(scope.is_empty) + self.assertEqual(scope.changed_count, 0) + + def test_from_ref_with_uncommitted_change(self): + """HEAD vs working tree (uncommitted edit) → 1 changed file.""" + self._write("file1.py", "print('changed')\n") + scope = DiffScope.from_ref(self.tmpdir, "HEAD") + self.assertFalse(scope.is_empty) + self.assertEqual(scope.changed_count, 1) + self.assertIn("file1.py", scope.changed_files) + + def test_from_ref_with_new_untracked_file(self): + """Untracked files should be included by default.""" + self._write("file3.py", "print('new')\n") + scope = DiffScope.from_ref(self.tmpdir, "HEAD") + self.assertIn("file3.py", scope.changed_files) + + def test_from_ref_exclude_untracked(self): + """include_untracked=False excludes untracked files.""" + self._write("file3.py", "print('new')\n") + scope = DiffScope.from_ref(self.tmpdir, "HEAD", include_untracked=False) + self.assertNotIn("file3.py", scope.changed_files) + self.assertTrue(scope.is_empty) + + def test_from_ref_against_first_commit(self): + """Diffing HEAD against the first commit should show all post-initial changes.""" + # Make a second commit + self._write("file3.py", "print('third')\n") + self._run("git", "add", ".", cwd=self.tmpdir) + self._run("git", "commit", "-m", "second", cwd=self.tmpdir) + scope = DiffScope.from_ref(self.tmpdir, self.first_sha) + self.assertIn("file3.py", scope.changed_files) + + def test_from_ref_sets_base_ref_in_summary(self): + scope = DiffScope.from_ref(self.tmpdir, "HEAD") + self.assertEqual(scope.summary()["base_ref"], "HEAD") + + def test_from_ref_head_tilde_1(self): + """HEAD~1 syntax works.""" + # Make a second commit + self._write("file3.py", "print('third')\n") + self._run("git", "add", ".", cwd=self.tmpdir) + self._run("git", "commit", "-m", "second", cwd=self.tmpdir) + scope = DiffScope.from_ref(self.tmpdir, "HEAD~1") + self.assertIn("file3.py", scope.changed_files) + self.assertEqual(scope.summary()["base_ref"], "HEAD~1") + + def test_from_ref_branch_name(self): + """Branch name as ref works (detects default branch — main or master).""" + # Detect the default branch created by `git init` + branch_out = subprocess.run( + ["git", "branch", "--show-current"], + cwd=self.tmpdir, capture_output=True, text=True, check=True, + ) + default_branch = branch_out.stdout.strip() + self.assertTrue(default_branch, "git should have a current branch") + scope = DiffScope.from_ref(self.tmpdir, default_branch) + # current_branch vs HEAD (no uncommitted changes) → empty + self.assertTrue(scope.is_empty) + self.assertEqual(scope.summary()["base_ref"], default_branch) + + +# ─── 6. from_ref() error cases ────────────────────────────── + + +class TestFromRefErrors(unittest.TestCase): + """DiffScope.from_ref() raises DiffScopeError on bad input.""" + + def test_invalid_ref_raises(self): + with tempfile.TemporaryDirectory() as tmpdir: + subprocess.run(["git", "init"], cwd=tmpdir, capture_output=True, check=True) + subprocess.run(["git", "config", "user.email", "t@t.com"], cwd=tmpdir, capture_output=True, check=True) + subprocess.run(["git", "config", "user.name", "t"], cwd=tmpdir, capture_output=True, check=True) + with open(os.path.join(tmpdir, "f.py"), "w") as f: + f.write("x = 1\n") + subprocess.run(["git", "add", "."], cwd=tmpdir, capture_output=True, check=True) + subprocess.run(["git", "commit", "-m", "init"], cwd=tmpdir, capture_output=True, check=True) + with self.assertRaises(DiffScopeError) as ctx: + DiffScope.from_ref(tmpdir, "nonexistent-ref-xyz") + self.assertIn("nonexistent-ref-xyz", str(ctx.exception)) + + def test_empty_ref_raises(self): + with tempfile.TemporaryDirectory() as tmpdir: + with self.assertRaises(DiffScopeError) as ctx: + DiffScope.from_ref(tmpdir, "") + self.assertIn("non-empty", str(ctx.exception)) + + def test_nonexistent_workspace_raises(self): + with self.assertRaises(DiffScopeError) as ctx: + DiffScope.from_ref("/nonexistent/path/xyz", "HEAD") + self.assertIn("does not exist", str(ctx.exception)) + + def test_non_git_directory_raises(self): + """A directory that exists but is not a git repo should raise.""" + with tempfile.TemporaryDirectory() as tmpdir: + with self.assertRaises(DiffScopeError): + DiffScope.from_ref(tmpdir, "HEAD") + + +# ─── 7. CLI integration (subprocess) ──────────────────────── + + +class TestCliIntegration(unittest.TestCase): + """End-to-end CLI tests via subprocess — --diff-base flag works.""" + + @classmethod + def setUpClass(cls): + """Use the CodeLens repo itself as the test workspace.""" + cls.codelens_repo = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + cls.cli = os.path.join(cls.codelens_repo, "scripts", "codelens.py") + + def _run_cli(self, *args): + env = os.environ.copy() + env["PYTHONUTF8"] = "1" + env.pop("CODELENS_AI_MODE", None) # ensure json default + proc = subprocess.run( + [sys.executable, self.cli] + list(args), + capture_output=True, text=True, env=env, timeout=60, + cwd=self.codelens_repo, + ) + return proc + + def test_diff_base_flag_in_help(self): + proc = self._run_cli("--help") + self.assertIn("--diff-base", proc.stdout) + + def test_invalid_ref_exits_nonzero(self): + proc = self._run_cli("secrets", "tests/fixtures", "--diff-base", "nonexistent-ref-xyz") + self.assertNotEqual(proc.returncode, 0) + self.assertIn("diff_scope_error", proc.stderr + proc.stdout) + + def test_valid_ref_produces_diff_scope_in_output(self): + """--diff-base HEAD~1 should add a diff_scope key to the JSON output.""" + proc = self._run_cli("secrets", "tests/fixtures", "--diff-base", "HEAD~1") + # Find JSON in output (skip stderr hint lines) + import json + out = proc.stdout + json_start = out.find("{") + self.assertGreater(json_start, -1, "No JSON in output") + data = json.loads(out[json_start:]) + self.assertIn("diff_scope", data) + self.assertIn("changed_count", data["diff_scope"]) + self.assertIn("findings_before_filter", data["diff_scope"]) + self.assertIn("findings_after_filter", data["diff_scope"]) + + def test_empty_diff_early_exit(self): + """--diff-base HEAD in a clean repo → early exit with 'No changed files' message.""" + with tempfile.TemporaryDirectory() as tmpdir: + subprocess.run(["git", "init"], cwd=tmpdir, capture_output=True, check=True) + subprocess.run(["git", "config", "user.email", "t@t.com"], cwd=tmpdir, capture_output=True, check=True) + subprocess.run(["git", "config", "user.name", "t"], cwd=tmpdir, capture_output=True, check=True) + with open(os.path.join(tmpdir, "f.py"), "w") as f: + f.write("x = 1\n") + subprocess.run(["git", "add", "."], cwd=tmpdir, capture_output=True, check=True) + subprocess.run(["git", "commit", "-m", "init"], cwd=tmpdir, capture_output=True, check=True) + + env = os.environ.copy() + env["PYTHONUTF8"] = "1" + env.pop("CODELENS_AI_MODE", None) + proc = subprocess.run( + [sys.executable, self.cli, "secrets", tmpdir, "--diff-base", "HEAD"], + capture_output=True, text=True, env=env, timeout=60, + cwd=self.codelens_repo, + ) + self.assertEqual(proc.returncode, 0) + import json + out = proc.stdout + json_start = out.find("{") + self.assertGreater(json_start, -1) + data = json.loads(out[json_start:]) + self.assertEqual(data["status"], "ok") + self.assertIn("No changed files", data.get("message", "")) + self.assertEqual(data["diff_scope"]["changed_count"], 0) + + def test_diff_base_before_subcommand(self): + """--diff-base works both before and after the subcommand.""" + proc = self._run_cli("--diff-base", "HEAD~1", "secrets", "tests/fixtures") + import json + out = proc.stdout + json_start = out.find("{") + self.assertGreater(json_start, -1) + data = json.loads(out[json_start:]) + self.assertIn("diff_scope", data) + + +if __name__ == "__main__": + unittest.main()