Skip to content

Commit 5a06caf

Browse files
codexByron
authored andcommitted
test(gix): count CLI process launches and enforce benchmark ceilings
Count successful `Git.execute` launches separately from native/fallback decisions, including raw commands and persistent-process starts. Report pytest setup/call/teardown totals and optional ceilings, and put warm per-measurement launch counts and ceilings into the existing `pyperf` CI comparison. Commands that fail after spawning count; failed creation and Git's own children do not. Require Gix-backed implementations in `AGENTS.md` from this commit onward. Python may adapt inputs/results and manage native handles, but cannot provide missing Git semantics or fabricate success. Missing or unclear Gix capabilities retain CLI fallback with actionable upstream documentation. For the pinned fixture, warm CLI/Gix launch counts are 23/1 for the journey, 11/5 for opening and 13/7 for discovery. The opening/discovery Gix ceilings include the restored reference-format query (`GIX-1`), one more call than the previous fabricated answer. This commit itself reduces CLI calls by zero. Keep the earlier 30,898-launch full-suite report explicitly historical; it does not validate the rewritten implementation. Validation: 13 counter/ceiling/comparison tests passed with Gix first (0.06s) then CLI (0.05s). All 11 existing benchmark operations passed an untimed Gix-then-CLI result-digest comparison and the corrected CLI ceilings. `git diff --check` passed. Only existing CPython 3.12.14 was used. Clarify the native-location boundary following the metadata review: adapting `Repository.git_dir()` with the fixed `modules` and `COMMIT_EDITMSG` leaf names is permitted, as is canonicalizing an input path before reopening through Gix. Both require Git parity tests; neither permits a general Python Git-path resolver. This follow-up changes policy text only and passes `git diff --check`. Define `native` explicitly as Gitoxide execution through GixPython, never a Python implementation. Require each removed CLI call to map to identified Gix calls. Treat observed Git/Gix divergences as compatibility bugs, including intentional differences, and require equivalent behavior or a mode as strict as Git. Record expected/actual behavior and reproducible evidence, and keep an upstream bug open when a Gix-based adapter workaround restores local parity. Missing bindings and unverified guards remain distinguishable from reproduced bugs. This clarification changes documentation only; `git diff --check` passes.
1 parent 5b663d3 commit 5a06caf

11 files changed

Lines changed: 290 additions & 19 deletions

File tree

‎.github/workflows/backend-benchmark.yml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ jobs:
6464
- name: Check parity and publish comparison
6565
run: |
6666
.bench-gix/bin/python -m test.performance.compare_backends \
67-
benchmark-results/cli.json benchmark-results/gix.json > benchmark-results/comparison.md
67+
benchmark-results/cli.json benchmark-results/gix.json \
68+
--cli-budget test/performance/cli-budget.json > benchmark-results/comparison.md
6869
cat benchmark-results/comparison.md >> "$GITHUB_STEP_SUMMARY"
6970
.bench-gix/bin/python -m pyperf compare_to \
7071
benchmark-results/cli.json benchmark-results/gix.json --table --table-format md \

‎AGENTS.md‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,46 @@ Before starting work, read and follow [CONTRIBUTING.md](CONTRIBUTING.md),
44
including the [Prevent agent impersonation](CONTRIBUTING.md#prevent-agent-impersonation)
55
section governing identification when communicating through a person's account.
66

7+
# Gix backend implementations
8+
9+
Gix backend operations must use Gix through the supported GixPython (`gix`)
10+
APIs. Python/stdlib implementations, direct filesystem inference, and custom
11+
native implementations outside Gix are not substitutes for a missing Gix
12+
operation. Do not implement Git behavior independently to avoid CLI calls.
13+
14+
"Native" in code, documentation, reports and benchmarks means execution by
15+
Gitoxide through GixPython's `gix` API. It never means Python-implemented Git
16+
behavior. Every eliminated CLI call must be backed by identified Gix calls.
17+
18+
Python glue may validate inputs, adapt Gix results to GitPython's return types
19+
and formatting, manage the retained `gix.Repository`, and select a CLI fallback.
20+
It must not invent a successful result or repair missing Git semantics with a
21+
separate implementation. Retaining and explicitly recreating a Gix repository
22+
through the shared accessor is allowed.
23+
24+
Adapting native locations may append the fixed metadata leaf names `modules`
25+
and `COMMIT_EDITMSG` to `Repository.git_dir()`, whose location Gix resolves.
26+
Input paths may be canonicalized before reopening through Gix; return metadata
27+
from that native handle. These adaptations must match Git in regression tests
28+
and do not authorize a Python implementation of the general `--git-path` rules.
29+
30+
If Gix lacks a capability, differs from the required behavior, or its use is
31+
unclear, keep the existing Git CLI path and document the gap in
32+
`doc/gix-backend.md`. State the expected behavior, available evidence, and what
33+
GixPython or Gitoxide needs to expose or fix upstream. Tests, coverage reports
34+
and benchmark ceilings must reflect that fallback; a lower CLI count alone
35+
does not demonstrate a Gix implementation.
36+
37+
Track every observed difference from the equivalent Git operation as a Git
38+
compatibility bug, even if the Gix behavior is intentional. Gix must provide
39+
matching behavior, at least through a mode as strict as Git. Record Git's
40+
expected behavior, Gix's actual behavior, versions, reproduction or regression,
41+
and the affected adapter/fallback in the ledger. Keep these bugs open when a
42+
Gix-based adapter workaround restores parity; close them only after verifying
43+
the upstream fix or compatible mode. Distinguish demonstrated mismatches from
44+
missing APIs and unverified behavior. Do not repair a compatibility bug by
45+
implementing Git semantics in Python.
46+
747
# Commit messages
848

949
Follow Conventional Commits for every commit. Every commit must have a

‎doc/gix-backend.md‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,53 @@ The `git.backend` DEBUG logger reports these decisions outside pytest.
7777
`git._backend.statistics()` returns a snapshot of the process-local counters.
7878
Zero observed calls means untested in that run, not unsupported.
7979

80+
Successful `Git.execute` launches also appear as `CLI process` records. This
81+
includes raw `repo.git` calls and failed Git commands that started a process,
82+
but excludes failed process creation, direct subprocesses in test helpers,
83+
other Python processes and Git's own child processes. A persistent `cat-file`
84+
launch counts once; subsequent requests through it do not. These counts apply
85+
to both installations and are separate from native/fallback decisions.
86+
87+
Pytest prints session launch totals split into setup, call, teardown and
88+
collection/session work. `--backend-report` keeps its existing record-list
89+
format and adds `pytest.*` phase records. The `Git.execute` record is
90+
process-lifetime cumulative (including import-time probes); `pytest.session_total`
91+
is the current session's total. Optional ceilings fail the test session on an
92+
increase while allowing reductions:
93+
94+
```sh
95+
.tox/gix/bin/python test/run-local.py --no-cov -q \
96+
--backend-report=.cache/gix-processes.json \
97+
--max-cli-processes=TOTAL --max-cli-test-processes=CALLS
98+
```
99+
100+
Replace `TOTAL`/`CALLS` with baselines for the same selected tests and platform.
101+
Setup is deliberately counted separately because fixture improvements and
102+
test additions can change it independently of native backend capability.
103+
These are pytest phases: `unittest.TestCase.setUp()`/`tearDown()` run inside
104+
the call phase, so that column can still include their fixture work. Counts
105+
cover the current Python process; they are not aggregated across xdist workers.
106+
For a stable CI assertion, the pinned repository benchmark uses per-measurement
107+
ceilings in `test/performance/cli-budget.json`: the warm Gix journey launches
108+
one CLI process, opening launches five, and nested discovery launches seven.
109+
Reduce these ceilings as conversions land. The other operation rows have zero
110+
ceilings except the one-process patch diff.
111+
112+
Before the Gix-only audit, this instrumentation on CPython 3.12.14/macOS arm64 and official
113+
GixPython 0.1.0 recorded a full suite without coverage passing 1,674 tests and 38
114+
subtests (79 skipped, one xfailed) in 408.81 seconds. It recorded **30,898
115+
`Git.execute` launches**: 6,523 in pytest setup and 24,375 in call phases,
116+
with zero in teardown or collection/session phases. The process-lifetime
117+
counter is 30,899 because the import-time Git probe precedes pytest session
118+
accounting. The same run recorded 21,409 fallback decisions. These totals
119+
include explicit CLI tests and fixture commands, including non-Git commands
120+
passed directly to `Git.execute`; they are a local baseline, not a count of
121+
only fallback calls. The fixed warm benchmark instead isolates a user journey:
122+
23 CLI launches versus one Gix launch, opening 11 versus five, and nested
123+
discovery 13 versus seven after restoring the reference-format fallback.
124+
The historical full-suite counts above predate this correction. A constant
125+
`files` answer was not a Gix query; `GIX-1` records the missing binding.
126+
80127
| GitPython entry point / managed command | Native implementation | Remaining CLI cases |
81128
| --- | --- | --- |
82129
| `Git.get_object_header`, ODB `info` | Object resolution and native header lookup | Custom storage, unsupported revision grammar |

‎git/cmd.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1604,6 +1604,7 @@ def execute(
16041604
except cmd_not_found_exception as err:
16051605
raise GitCommandNotFound(redacted_command, err) from err
16061606
else:
1607+
_backend.record("Git.execute", "CLI process")
16071608
# Replace with a typeguard for Popen[bytes]?
16081609
proc.stdout = cast(BinaryIO, proc.stdout)
16091610
proc.stderr = cast(BinaryIO, proc.stderr)

‎test/conftest.py‎

Lines changed: 69 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Report native coverage when the optional backend is installed."""
22

3+
from collections import Counter
34
import json
45
from pathlib import Path
56

@@ -11,22 +12,85 @@
1112

1213
def pytest_addoption(parser):
1314
parser.addoption("--backend-report", help="Write native/CLI operation counts as JSON")
15+
parser.addoption("--max-cli-processes", type=int, help="Maximum Git.execute launches in the pytest session")
16+
parser.addoption("--max-cli-test-processes", type=int, help="Maximum Git.execute launches in test call phases")
17+
18+
19+
def cli_processes():
20+
return _backend.statistics().get(("Git.execute", "CLI process"), 0)
21+
22+
23+
def pytest_configure(config):
24+
for option in ("--max-cli-processes", "--max-cli-test-processes"):
25+
limit = config.getoption(option)
26+
if limit is not None and limit < 0:
27+
raise pytest.UsageError(option + " must be nonnegative")
28+
config._cli_process_start = cli_processes()
29+
config._cli_process_phases = Counter()
30+
31+
32+
@pytest.hookimpl(hookwrapper=True)
33+
def pytest_runtest_setup(item):
34+
before = cli_processes()
35+
yield
36+
item.config._cli_process_phases["setup"] += cli_processes() - before
37+
38+
39+
@pytest.hookimpl(hookwrapper=True)
40+
def pytest_runtest_call(item):
41+
before = cli_processes()
42+
yield
43+
item.config._cli_process_phases["call"] += cli_processes() - before
44+
45+
46+
@pytest.hookimpl(hookwrapper=True)
47+
def pytest_runtest_teardown(item):
48+
before = cli_processes()
49+
yield
50+
item.config._cli_process_phases["teardown"] += cli_processes() - before
51+
52+
53+
def pytest_sessionfinish(session, exitstatus):
54+
config = session.config
55+
actual = {
56+
"--max-cli-processes": cli_processes() - config._cli_process_start,
57+
"--max-cli-test-processes": config._cli_process_phases["call"],
58+
}
59+
config._cli_process_failures = [
60+
f"{option}: {count} launches exceed ceiling {config.getoption(option)}"
61+
for option, count in actual.items()
62+
if config.getoption(option) is not None and count > config.getoption(option)
63+
]
64+
if config._cli_process_failures:
65+
session.exitstatus = pytest.ExitCode.TESTS_FAILED
1466

1567

1668
def pytest_report_header(config):
1769
return "GitPython backend: " + _backend.name
1870

1971

2072
def pytest_terminal_summary(terminalreporter, exitstatus, config):
21-
if _backend.name != "gix":
22-
return
73+
total = cli_processes() - config._cli_process_start
74+
phases = config._cli_process_phases
75+
terminalreporter.section("Git CLI process launches")
76+
terminalreporter.write_line(
77+
f"{total} total: {phases['setup']} setup, {phases['call']} call, "
78+
f"{phases['teardown']} teardown, {total - sum(phases.values())} collection/session"
79+
)
80+
for failure in config._cli_process_failures:
81+
terminalreporter.write_line(failure, red=True)
2382
records = [
2483
{"method": method, "outcome": outcome, "count": count}
2584
for (method, outcome), count in sorted(_backend.statistics().items())
2685
]
27-
terminalreporter.section("GixPython operation coverage")
28-
for item in records:
29-
terminalreporter.write_line("{method}: {outcome} ({count})".format(**item))
86+
if _backend.name == "gix":
87+
terminalreporter.section("GixPython operation coverage")
88+
for item in records:
89+
terminalreporter.write_line("{method}: {outcome} ({count})".format(**item))
90+
records.extend(
91+
{"method": "pytest." + phase, "outcome": "CLI process", "count": count}
92+
for phase, count in sorted({**phases, "session_total": total}.items())
93+
)
3094
path = config.getoption("--backend-report")
3195
if path:
3296
Path(path).parent.mkdir(parents=True, exist_ok=True)

‎test/performance/README.md‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ Run from the source root, sequentially to avoid competing for CPU and disk:
4949
--repo "$BENCH_REPO" --expect-backend gix -o /tmp/gix.json
5050
.venv/bin/python -m test.performance.bench_repository \
5151
--repo "$BENCH_REPO" --expect-backend cli -o /tmp/cli.json
52-
.tox/gix/bin/python -m test.performance.compare_backends /tmp/cli.json /tmp/gix.json
52+
.tox/gix/bin/python -m test.performance.compare_backends /tmp/cli.json /tmp/gix.json \
53+
--cli-budget test/performance/cli-budget.json
5354
.tox/gix/bin/python -m pyperf compare_to /tmp/cli.json /tmp/gix.json --table --table-format md
5455
```
5556

@@ -76,6 +77,24 @@ of the journey and parity checks. Include supported and fallback operations;
7677
the current patch measurement intentionally exercises fallback. Backend
7778
decisions are adapter events, not complete subprocess counts.
7879

80+
`cli_processes` counts successful launches through `Git.execute` during one
81+
warm invocation, including raw calls and the launch of persistent `cat-file`
82+
processes. Sending another request to an existing process does not increment
83+
it. Native/fallback decisions remain separate: the current warm Gix journey has
84+
two fallback decisions but launches only one process (the patch diff).
85+
Direct subprocesses in the harness and Git's own child processes are excluded.
86+
87+
The checked-in `cli-budget.json` sets maximum Gix launch counts for this pinned
88+
fixture. CI enforces the ceilings: reductions pass, increases fail with the
89+
measurement and counts. Lower ceilings when an optimization lands to retain
90+
the gain. The initial warm ceilings are journey/patch 1, opening 5, discovery 7,
91+
and zero for the other operations. New measurements need an explicit ceiling;
92+
do not automatically raise an existing ceiling to accept a regression.
93+
94+
The opening/discovery ceilings include a real CLI reference-format query
95+
(`GIX-1`). GixPython must expose the format before that call can disappear;
96+
Python inference does not count as a native implementation.
97+
7998
Timing is observational: shared CI runners and local activity introduce noise.
8099
The CI job publishes all means, standard deviations and ratios, retains raw
81100
artifacts, and uses `pyperf compare_to` for significance reporting. It fails on

‎test/performance/bench_repository.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ def git(*arguments, cwd=args.repo):
149149
]
150150
)
151151
for name, operation, target in measurements:
152+
operation(target) # Warm persistent CLI processes before counting.
152153
# Untimed preflight validates parity and records native/fallback
153154
# decisions for ONE invocation, rather than calibrated loop counts.
154155
before = _backend.statistics()
@@ -161,6 +162,11 @@ def git(*arguments, cwd=args.repo):
161162
metadata = {
162163
"result_digest": sha256(json.dumps(result, sort_keys=True).encode()).hexdigest(),
163164
"backend_decisions": json.dumps(decisions, sort_keys=True),
165+
"cli_processes": sum(
166+
item["count"]
167+
for item in decisions
168+
if (item["method"], item["outcome"]) == ("Git.execute", "CLI process")
169+
),
164170
}
165171
runner.bench_func(name, operation, target, metadata=metadata)
166172

‎test/performance/cli-budget.json‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"fixture_revision": "6ba2c0a2f9ee7feffd7e079621c4845820180c9a",
3+
"max_gix_cli_processes": {
4+
"journey": 1,
5+
"inventory": 0,
6+
"history_25": 0,
7+
"browse_tree_and_blob": 0,
8+
"read_index": 0,
9+
"revision_graph": 0,
10+
"diff_and_stats": 0,
11+
"patch_diff": 1,
12+
"worktree_status": 0,
13+
"open_repository": 5,
14+
"discover_repository": 7
15+
}
16+
}

‎test/performance/compare_backends.py‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
"""Validate benchmark parity and print every measurement, including slowdowns."""
22

3+
import argparse
34
import json
4-
import sys
5+
from pathlib import Path
56

67
import pyperf
78

89

9-
def compare(cli_path, gix_path):
10+
def compare(cli_path, gix_path, cli_budget=None):
1011
cli, gix = pyperf.BenchmarkSuite.load(str(cli_path)), pyperf.BenchmarkSuite.load(str(gix_path))
12+
budget = json.loads(Path(cli_budget).read_text()) if cli_budget else None
1113
if cli.get_benchmark_names() != gix.get_benchmark_names():
1214
raise ValueError("The backends measured different journeys")
1315
rows = []
@@ -30,20 +32,34 @@ def compare(cli_path, gix_path):
3032
if lm[key] != rm[key]:
3133
raise ValueError(f"{name}: backends differ in {key}: {lm[key]!r} != {rm[key]!r}")
3234
decisions = json.loads(rm["backend_decisions"])
35+
if budget is not None:
36+
if rm["fixture_revision"] != budget["fixture_revision"]:
37+
raise ValueError("CLI process budget belongs to a different fixture")
38+
limit = budget["max_gix_cli_processes"][name]
39+
if rm["cli_processes"] > limit:
40+
raise ValueError(f"{name}: {rm['cli_processes']} CLI launches exceed ceiling {limit}")
3341
native = sum(item["count"] for item in decisions if item["outcome"] == "native")
3442
fallback = sum(item["count"] for item in decisions if item["outcome"].startswith("CLI:"))
3543
rows.append(
3644
f"| {name} | {left.mean() * 1000:.2f} ± {left.stdev() * 1000:.2f} "
3745
f"| {right.mean() * 1000:.2f} ± {right.stdev() * 1000:.2f} "
38-
f"| {left.mean() / right.mean():.2f}× | {native} / {fallback} |"
46+
f"| {left.mean() / right.mean():.2f}× | {native} / {fallback} "
47+
f"| {lm['cli_processes']} / {rm['cli_processes']} |"
3948
)
40-
print("| Measurement | CLI mean ± stdev (ms) | GixPython mean ± stdev (ms) | CLI / Gix | Native / fallback |")
41-
print("| --- | ---: | ---: | ---: | ---: |")
49+
print(
50+
"| Measurement | CLI mean ± stdev (ms) | GixPython mean ± stdev (ms) "
51+
"| CLI / Gix | Native / fallback | CLI launches: CLI / Gix |"
52+
)
53+
print("| --- | ---: | ---: | ---: | ---: | ---: |")
4254
print("\n".join(rows))
43-
print("\nRatios above 1 favor GixPython. Decisions are per invocation, not subprocess counts.")
55+
print("\nRatios above 1 favor GixPython. Decisions are per invocation; CLI launches count Git.execute processes.")
4456
print("\nWarm-cache, read-only measurements; checkout, imports and preflight are outside timing.")
4557
print("The journey reuses one open Repo; opening and nested discovery are measured separately.")
4658

4759

4860
if __name__ == "__main__":
49-
compare(*sys.argv[1:])
61+
parser = argparse.ArgumentParser(description=__doc__)
62+
parser.add_argument("cli_path")
63+
parser.add_argument("gix_path")
64+
parser.add_argument("--cli-budget", help="JSON ceilings for GixPython's launches on the fixed fixture")
65+
compare(**vars(parser.parse_args()))

‎test/performance/test_benchmark_comparison.py‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
"""The benchmark comparison must reject incompatible workloads and results."""
22

3+
import json
4+
35
import pytest
46

57
pyperf = pytest.importorskip("pyperf")
68

79

8-
@pytest.mark.parametrize("changed", [None, "result_digest", "fixture_revision", "backend"])
10+
@pytest.mark.parametrize("changed", [None, "result_digest", "fixture_revision", "backend", "cli_processes"])
911
def test_comparison_requires_parity(tmp_path, capsys, changed):
1012
from test.performance.compare_backends import compare
1113

@@ -20,18 +22,23 @@ def test_comparison_requires_parity(tmp_path, capsys, changed):
2022
"python_version": "3.12.14",
2123
"result_digest": "equal",
2224
"backend_decisions": "[]",
25+
"cli_processes": 0,
2326
"name": "journey",
2427
}
2528
paths = [tmp_path / "cli.json", tmp_path / "gix.json"]
2629
for backend, path in zip(("cli", "gix"), paths):
2730
values = {**metadata, "backend": backend}
2831
if backend == "gix" and changed:
29-
values[changed] = "different"
32+
values[changed] = 1 if changed == "cli_processes" else "different"
3033
run = pyperf.Run([0.1, 0.2], metadata=values, collect_metadata=False)
3134
pyperf.BenchmarkSuite([pyperf.Benchmark([run])]).dump(str(path))
35+
budget = tmp_path / "budget.json"
36+
budget.write_text(json.dumps({"fixture_revision": "fixture", "max_gix_cli_processes": {"journey": 0}}))
3237
if changed:
33-
with pytest.raises(ValueError, match="differ|Expected"):
34-
compare(*paths)
38+
with pytest.raises(ValueError, match="differ|Expected|exceed"):
39+
compare(*paths, cli_budget=budget)
3540
else:
36-
compare(*paths)
41+
compare(*paths, cli_budget=budget)
3742
assert "journey | 150.00" in capsys.readouterr().out
43+
budget.write_text(json.dumps({"fixture_revision": "fixture", "max_gix_cli_processes": {"journey": 1}}))
44+
compare(*paths, cli_budget=budget) # A reduction stays within the ceiling.

0 commit comments

Comments
 (0)