From d48227754a56d9592be1213ad6ca05aa0b99e78a Mon Sep 17 00:00:00 2001 From: nia-sg-bot Date: Sat, 15 Aug 2026 15:06:23 +0530 Subject: [PATCH 1/2] feat(cli): emit canonical artifacts for commit ranges --- README.md | 18 +++++-- diffgraph/artifact.py | 8 ++- diffgraph/cli.py | 102 ++++++++++++++++++++++++++++++--------- diffgraph/structural.py | 40 ++++++++++++--- tests/test_structural.py | 93 +++++++++++++++++++++++++++++++++-- 5 files changed, 224 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 547e93f..986200a 100644 --- a/README.md +++ b/README.md @@ -83,6 +83,8 @@ wild diff --format terminal wild --structural-json diffgraph.json diff wild --structural-json staged.json diff --staged -- src/ wild --structural-json - diff -- path/to/file.py +wild diff --format json HEAD~2..HEAD +wild diff --format html main...feature -- src/ ``` `--structural-json PATH` remains a compatibility alias for canonical JSON with @@ -90,11 +92,17 @@ that destination. Do not combine it with `--format` or `--output`; ambiguous combinations are usage errors. Likewise, `--format terminal` cannot use `--output`. Terminal-only `--compact` and `--all` flags follow `diff`. -This increment intentionally supports only local unstaged (`index` → working -tree) and staged (`HEAD` → index) snapshots. Put pathspecs after `--`. -Pathspecs are interpreted relative to the directory where `wild` is invoked, -matching Git's command-line behavior. Commit ranges are rejected rather than -analyzed with guessed semantics. +Canonical output supports local unstaged (`index` → working tree), staged +(`HEAD` → index), explicit two-dot (`BASE..HEAD`), and explicit three-dot +(`BASE...HEAD`) comparisons. Two-dot resolves both refs to immutable commits; +three-dot resolves their merge base and compares it with the immutable head, +matching Git diff semantics. The artifact records those exact comparison OIDs +and every file's pre/post blob identities. Invalid refs and unavailable merge +bases produce structured warnings instead of false changes. + +Put pathspecs after `--`. Pathspecs are interpreted relative to the directory +where `wild` is invoked, matching Git's command-line behavior. Both endpoints +of a commit range are required; implicit-ref forms are rejected. Python (`.py`) is the only language with structural symbol/import extraction in this baseline. Other changed files remain in `files[]` and receive a scoped diff --git a/diffgraph/artifact.py b/diffgraph/artifact.py index f754bdf..8e5248d 100644 --- a/diffgraph/artifact.py +++ b/diffgraph/artifact.py @@ -11,7 +11,7 @@ import os import tempfile from pathlib import Path -from typing import Sequence +from typing import Optional, Sequence from diffgraph.contract import ValidatedArtifact from diffgraph.structural import analyze_local_diff @@ -22,6 +22,9 @@ def build_validated_artifact( *, staged: bool = False, pathspecs: Sequence[str] = (), + base_ref: Optional[str] = None, + head_ref: Optional[str] = None, + three_dot: bool = False, wild_version: str, ) -> ValidatedArtifact: """Construct and validate exactly one local structural artifact.""" @@ -29,6 +32,9 @@ def build_validated_artifact( repository, staged=staged, pathspecs=pathspecs, + base_ref=base_ref, + head_ref=head_ref, + three_dot=three_dot, wild_version=wild_version, ) return ValidatedArtifact.from_value(artifact) diff --git a/diffgraph/cli.py b/diffgraph/cli.py index ed56348..0ea0b24 100644 --- a/diffgraph/cli.py +++ b/diffgraph/cli.py @@ -1,9 +1,10 @@ import subprocess import sys +from dataclasses import dataclass from pathlib import Path import click from click.core import ParameterSource -from typing import List, Dict +from typing import Dict, List, Optional, Tuple import os from diffgraph import __version__ from diffgraph.artifact import ( @@ -139,8 +140,8 @@ def _terminal_options(diff_args): return remaining, compact, show_all -def _separator_follows_diff(raw_args) -> bool: - """Return whether the raw CLI placed ``--`` after the ``diff`` operand.""" +def _pathspecs_after_diff_separator(raw_args) -> Optional[Tuple[str, ...]]: + """Return raw pathspecs after ``diff --``, preserving range-like names.""" value_options = {"--api-key", "--output", "-o", "--structural-json", "--format"} index = 0 @@ -156,29 +157,80 @@ def _separator_follows_diff(raw_args) -> bool: index += 1 continue if argument == "diff": - return "--" in raw_args[index + 1 :] + trailing = raw_args[index + 1 :] + if "--" not in trailing: + return None + separator_index = trailing.index("--") + return tuple(trailing[separator_index + 1 :]) index += 1 - return False + return None -def _structural_scope(diff_args: List[str], separator_present: bool = False): - """Accept only the exact local snapshot modes implemented by this increment.""" +@dataclass(frozen=True) +class _StructuralScope: + staged: bool = False + pathspecs: Tuple[str, ...] = () + base_ref: Optional[str] = None + head_ref: Optional[str] = None + three_dot: bool = False + + +def _range_operand(argument: str): + """Return exact two/three-dot endpoints, or ``None`` for another operand.""" + separator = "..." if "..." in argument else ".." if ".." in argument else None + if separator is None: + return None + endpoints = argument.split(separator) + if len(endpoints) != 2 or not all(endpoints): + raise click.UsageError( + "commit ranges require explicit non-empty BASE..HEAD or BASE...HEAD refs" + ) + return endpoints[0], endpoints[1], separator == "..." + + +def _structural_scope( + diff_args: List[str], explicit_pathspecs: Optional[Tuple[str, ...]] = None +): + """Parse the deterministic local or immutable commit comparison scope.""" + arguments = list(diff_args) + if "--" in arguments: + separator_index = arguments.index("--") + explicit_pathspecs = tuple(arguments[separator_index + 1 :]) + arguments = arguments[:separator_index] + elif explicit_pathspecs is not None and explicit_pathspecs: + count = len(explicit_pathspecs) + if tuple(arguments[-count:]) != explicit_pathspecs: + raise click.UsageError("could not preserve pathspec scope after '--'") + arguments = arguments[:-count] + staged = False - pathspecs = [] - after_separator = separator_present - for argument in diff_args: - if argument == "--": - after_separator = True - elif argument in ("--staged", "--cached") and not after_separator: + base_ref = head_ref = None + three_dot = False + for argument in arguments: + if argument in ("--staged", "--cached"): + if base_ref is not None: + raise click.UsageError( + "--staged/--cached cannot be combined with a commit range" + ) staged = True - elif after_separator: - pathspecs.append(argument) - else: + continue + commit_range = _range_operand(argument) + if commit_range is None: raise click.UsageError( - "--structural-json currently supports only unstaged or --staged/--cached " - "local diffs; put pathspecs after '--'" + "canonical output supports unstaged, --staged/--cached, or explicit " + "BASE..HEAD/BASE...HEAD diffs; put pathspecs after '--'" ) - return staged, pathspecs + if staged or base_ref is not None: + raise click.UsageError("use exactly one local mode or commit range") + base_ref, head_ref, three_dot = commit_range + + return _StructuralScope( + staged, + explicit_pathspecs or (), + base_ref, + head_ref, + three_dot, + ) def _validate_structural_artifact(artifact): @@ -324,12 +376,18 @@ def main( if output_format == "terminal": diff_args, compact, show_all = _terminal_options(diff_args) raw_args = click.get_current_context().meta.get("raw_args", ()) - staged, pathspecs = _structural_scope( - diff_args, separator_present=_separator_follows_diff(raw_args) + scope = _structural_scope( + diff_args, explicit_pathspecs=_pathspecs_after_diff_separator(raw_args) ) try: artifact = build_validated_artifact( - ".", staged=staged, pathspecs=pathspecs, wild_version=__version__ + ".", + staged=scope.staged, + pathspecs=scope.pathspecs, + base_ref=scope.base_ref, + head_ref=scope.head_ref, + three_dot=scope.three_dot, + wild_version=__version__, ) except ( GitSnapshotError, diff --git a/diffgraph/structural.py b/diffgraph/structural.py index b1d4ed3..1aef770 100644 --- a/diffgraph/structural.py +++ b/diffgraph/structural.py @@ -26,6 +26,7 @@ SnapshotEntry, read_worktree_blob, repository_root, + resolve_commit_range, resolve_staged, resolve_unstaged, run_git, @@ -281,12 +282,31 @@ def _keyed_imports(items: List[_Import]) -> Dict[Tuple[str, int], _Import]: def analyze_local_diff( repository: str = ".", *, staged: bool = False, - pathspecs: Optional[Sequence[str]] = None, wild_version: str = package_version, + pathspecs: Optional[Sequence[str]] = None, + base_ref: Optional[str] = None, + head_ref: Optional[str] = None, + three_dot: bool = False, + wild_version: str = package_version, ) -> Dict: - """Build a schema-v2 structural artifact for HEAD→index or index→worktree.""" + """Build a schema-v2 artifact for a local snapshot or immutable commit range.""" started = time.monotonic() root = repository_root(repository) - resolution = (resolve_staged if staged else resolve_unstaged)(repository, pathspecs) + if (base_ref is None) != (head_ref is None): + raise ValueError("base_ref and head_ref must be supplied together") + is_commit_range = base_ref is not None + if is_commit_range and staged: + raise ValueError("staged and commit-range comparisons are mutually exclusive") + resolution = ( + resolve_commit_range( + repository, + base_ref, + head_ref, + three_dot=three_dot, + pathspecs=pathspecs, + ) + if is_commit_range + else (resolve_staged if staged else resolve_unstaged)(repository, pathspecs) + ) warnings = [_resolution_warning(item) for item in resolution.warnings] files: List[Dict] = [] symbols: List[Dict] = [] @@ -299,7 +319,11 @@ def analyze_local_diff( raise RuntimeError("snapshot entry has neither an old nor a new path") try: old = _blob(root, entry.old_oid) - new = _blob(root, entry.new_oid) if staged else _worktree_bytes(root, entry) + new = ( + _blob(root, entry.new_oid) + if staged or is_commit_range + else _worktree_bytes(root, entry) + ) except (OSError, GitSnapshotError) as error: old = new = None warnings.append(_warning("PARTIAL_ANALYSIS", path, "snapshot read failed: {}".format(error))) @@ -466,8 +490,12 @@ def analyze_local_diff( "generated_at": datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ"), "wild_version": wild_version, "diff_ref": { - "kind": "staged" if staged else "unstaged", "base_ref": "HEAD" if staged else None, - "head_ref": None, "pathspecs": list(pathspecs or []), "repo_root": root, + "kind": "commit_range" if is_commit_range else "staged" if staged else "unstaged", + "base_ref": ( + resolution.comparison_base_oid if is_commit_range else "HEAD" if staged else None + ), + "head_ref": resolution.head_oid if is_commit_range else None, + "pathspecs": list(pathspecs or []), "repo_root": root, }, "files": files, "symbols": symbols, "relationships": relationships, "summary": None, diff --git a/tests/test_structural.py b/tests/test_structural.py index 5a6d4cc..296360f 100644 --- a/tests/test_structural.py +++ b/tests/test_structural.py @@ -326,17 +326,104 @@ def test_cli_legacy_html_retains_no_change_compatibility(monkeypatch): assert "No changes to analyze" in result.output -def test_cli_structural_json_rejects_unimplemented_commit_ranges(tmp_path, monkeypatch): +def test_cli_structural_json_resolves_immutable_two_dot_range(tmp_path, monkeypatch): from click.testing import CliRunner from diffgraph.cli import main root = repo(tmp_path) write(root, "cli.py", "def value():\n return 1\n") commit(root) + base_oid = git(root, "rev-parse", "HEAD") + old_blob = git(root, "rev-parse", "HEAD:cli.py") + write(root, "cli.py", "def value():\n return 2\n") + commit(root) + head_oid = git(root, "rev-parse", "HEAD") + new_blob = git(root, "rev-parse", "HEAD:cli.py") monkeypatch.chdir(root) + result = CliRunner().invoke(main, ["--structural-json", "-", "diff", "HEAD~1..HEAD"]) - assert result.exit_code == 2 - assert "currently supports only unstaged" in result.output + + assert result.exit_code == 0, result.output + artifact = json.loads(result.output) + assert artifact["diff_ref"]["kind"] == "commit_range" + assert artifact["diff_ref"]["base_ref"] == base_oid + assert artifact["diff_ref"]["head_ref"] == head_oid + provenance = json.loads(artifact["files"][0]["evidence"][0]["detail"]) + assert provenance["old_oid"] == old_blob + assert provenance["new_oid"] == new_blob + + +def test_cli_structural_json_three_dot_uses_merge_base_and_pathspec(tmp_path, monkeypatch): + from click.testing import CliRunner + from diffgraph.cli import main + + root = repo(tmp_path) + write(root, "included.py", "def value():\n return 1\n") + write(root, "ignored.py", "def ignored():\n return 1\n") + commit(root) + base_oid = git(root, "rev-parse", "HEAD") + write(root, "included.py", "def value():\n return 2\n") + write(root, "ignored.py", "def ignored():\n return 2\n") + commit(root) + head_oid = git(root, "rev-parse", "HEAD") + monkeypatch.chdir(root) + + result = CliRunner().invoke( + main, + ["--structural-json", "-", "diff", "HEAD~1...HEAD", "--", "included.py"], + ) + + assert result.exit_code == 0, result.output + artifact = json.loads(result.output) + assert artifact["diff_ref"] == { + "kind": "commit_range", + "base_ref": base_oid, + "head_ref": head_oid, + "pathspecs": ["included.py"], + "repo_root": str(root), + } + assert [item["path"] for item in artifact["files"]] == ["included.py"] + + +def test_cli_structural_json_preserves_range_like_pathspec(tmp_path, monkeypatch): + from click.testing import CliRunner + from diffgraph.cli import main + + root = repo(tmp_path) + write(root, "name..with.py", "def value():\n return 1\n") + commit(root) + write(root, "name..with.py", "def value():\n return 2\n") + monkeypatch.chdir(root) + + result = CliRunner().invoke( + main, ["--structural-json", "-", "diff", "--", "name..with.py"] + ) + + assert result.exit_code == 0, result.output + artifact = json.loads(result.output) + assert artifact["diff_ref"]["kind"] == "unstaged" + assert artifact["diff_ref"]["pathspecs"] == ["name..with.py"] + + +def test_cli_structural_json_preserves_invalid_ref_warning(tmp_path, monkeypatch): + from click.testing import CliRunner + from diffgraph.cli import main + + root = repo(tmp_path) + write(root, "app.py", "def value():\n return 1\n") + commit(root) + monkeypatch.chdir(root) + + result = CliRunner().invoke( + main, ["--structural-json", "-", "diff", "missing-ref..HEAD"] + ) + + assert result.exit_code == 0, result.output + artifact = json.loads(result.output) + assert artifact["files"] == [] + warning = artifact["metadata"]["warnings"][0] + assert warning["code"] == "UNKNOWN" + assert warning["detail"].startswith("invalid_base_ref:") def test_cli_structural_json_requires_separator_before_pathspecs(tmp_path, monkeypatch): From bda5677cebc93964c6b6f4d75dbc2f96c1eb13f2 Mon Sep 17 00:00:00 2001 From: nia-sg-bot Date: Sat, 15 Aug 2026 18:01:19 +0530 Subject: [PATCH 2/2] fix(cli): preserve range and pathspec semantics --- diffgraph/cli.py | 33 +++++++++++++++++++------ tests/test_structural.py | 52 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 75 insertions(+), 10 deletions(-) diff --git a/diffgraph/cli.py b/diffgraph/cli.py index 0ea0b24..22874cd 100644 --- a/diffgraph/cli.py +++ b/diffgraph/cli.py @@ -125,19 +125,31 @@ def parse_args(self, ctx, args): return super().parse_args(ctx, args) -def _terminal_options(diff_args): - """Remove terminal-only display flags from a ``diff`` invocation.""" +def _terminal_options( + diff_args, explicit_pathspecs: Optional[Tuple[str, ...]] = None +): + """Remove terminal-only flags without consuming pathspecs after ``--``.""" + option_args = list(diff_args) + pathspec_args = [] + if explicit_pathspecs: + pathspec_count = len(explicit_pathspecs) + if tuple(option_args[-pathspec_count:]) == explicit_pathspecs: + option_args, pathspec_args = ( + option_args[:-pathspec_count], + option_args[-pathspec_count:], + ) + remaining = [] compact = False show_all = False - for arg in diff_args: + for arg in option_args: if arg == "--compact": compact = True elif arg == "--all": show_all = True else: remaining.append(arg) - return remaining, compact, show_all + return remaining + pathspec_args, compact, show_all def _pathspecs_after_diff_separator(raw_args) -> Optional[Tuple[str, ...]]: @@ -177,6 +189,10 @@ class _StructuralScope: def _range_operand(argument: str): """Return exact two/three-dot endpoints, or ``None`` for another operand.""" + if "...." in argument: + raise click.UsageError( + "commit ranges require explicit non-empty BASE..HEAD or BASE...HEAD refs" + ) separator = "..." if "..." in argument else ".." if ".." in argument else None if separator is None: return None @@ -373,11 +389,14 @@ def main( if structural_json is not None or output_format in ("html", "terminal", "json"): compact = False show_all = False - if output_format == "terminal": - diff_args, compact, show_all = _terminal_options(diff_args) raw_args = click.get_current_context().meta.get("raw_args", ()) + explicit_pathspecs = _pathspecs_after_diff_separator(raw_args) + if output_format == "terminal": + diff_args, compact, show_all = _terminal_options( + diff_args, explicit_pathspecs=explicit_pathspecs + ) scope = _structural_scope( - diff_args, explicit_pathspecs=_pathspecs_after_diff_separator(raw_args) + diff_args, explicit_pathspecs=explicit_pathspecs ) try: artifact = build_validated_artifact( diff --git a/tests/test_structural.py b/tests/test_structural.py index 296360f..c272c77 100644 --- a/tests/test_structural.py +++ b/tests/test_structural.py @@ -268,6 +268,27 @@ def test_cli_terminal_all_disables_review_item_cap(tmp_path, monkeypatch): assert "more" not in result.output +@pytest.mark.parametrize("pathspec", ["--compact", "--all"]) +def test_cli_terminal_preserves_flag_like_pathspec_after_separator( + pathspec, tmp_path, monkeypatch +): + from click.testing import CliRunner + from diffgraph.cli import main + + root = repo(tmp_path) + write(root, pathspec, "before\n") + commit(root) + write(root, pathspec, "after\n") + monkeypatch.chdir(root) + + result = CliRunner().invoke( + main, ["diff", "--format", "terminal", "--", pathspec] + ) + + assert result.exit_code == 0, result.output + assert pathspec in result.output + + def test_cli_git_passthrough_preserves_all(monkeypatch): from click.testing import CliRunner import diffgraph.cli as cli @@ -361,7 +382,14 @@ def test_cli_structural_json_three_dot_uses_merge_base_and_pathspec(tmp_path, mo write(root, "included.py", "def value():\n return 1\n") write(root, "ignored.py", "def ignored():\n return 1\n") commit(root) - base_oid = git(root, "rev-parse", "HEAD") + merge_base_oid = git(root, "rev-parse", "HEAD") + + git(root, "checkout", "-b", "left") + write(root, "left.py", "def left():\n return 1\n") + commit(root) + left_oid = git(root, "rev-parse", "HEAD") + + git(root, "checkout", "-b", "right", merge_base_oid) write(root, "included.py", "def value():\n return 2\n") write(root, "ignored.py", "def ignored():\n return 2\n") commit(root) @@ -370,18 +398,19 @@ def test_cli_structural_json_three_dot_uses_merge_base_and_pathspec(tmp_path, mo result = CliRunner().invoke( main, - ["--structural-json", "-", "diff", "HEAD~1...HEAD", "--", "included.py"], + ["--structural-json", "-", "diff", "left...right", "--", "included.py"], ) assert result.exit_code == 0, result.output artifact = json.loads(result.output) assert artifact["diff_ref"] == { "kind": "commit_range", - "base_ref": base_oid, + "base_ref": merge_base_oid, "head_ref": head_oid, "pathspecs": ["included.py"], "repo_root": str(root), } + assert artifact["diff_ref"]["base_ref"] != left_oid assert [item["path"] for item in artifact["files"]] == ["included.py"] @@ -405,6 +434,23 @@ def test_cli_structural_json_preserves_range_like_pathspec(tmp_path, monkeypatch assert artifact["diff_ref"]["pathspecs"] == ["name..with.py"] +def test_cli_structural_json_rejects_four_dot_range(tmp_path, monkeypatch): + from click.testing import CliRunner + from diffgraph.cli import main + + root = repo(tmp_path) + write(root, "app.py", "def value():\n return 1\n") + commit(root) + monkeypatch.chdir(root) + + result = CliRunner().invoke( + main, ["--structural-json", "-", "diff", "HEAD....HEAD"] + ) + + assert result.exit_code == 2 + assert "explicit non-empty BASE..HEAD or BASE...HEAD refs" in result.output + + def test_cli_structural_json_preserves_invalid_ref_warning(tmp_path, monkeypatch): from click.testing import CliRunner from diffgraph.cli import main