Skip to content

Commit 2992d9a

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 51f5c88 commit 2992d9a

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
@@ -67,7 +67,8 @@ jobs:
6767
- name: Check parity and publish comparison
6868
run: |
6969
.bench-gix/bin/python -m test.performance.compare_backends \
70-
benchmark-results/cli.json benchmark-results/gix.json > benchmark-results/comparison.md
70+
benchmark-results/cli.json benchmark-results/gix.json \
71+
--cli-budget test/performance/cli-budget.json > benchmark-results/comparison.md
7172
cat benchmark-results/comparison.md >> "$GITHUB_STEP_SUMMARY"
7273
.bench-gix/bin/python -m pyperf compare_to \
7374
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
@@ -5,6 +5,46 @@ including the [Prevent agent impersonation](CONTRIBUTING.md#prevent-agent-impers
55
section governing agent identification and separation of unaltered user statements
66
when communicating through a person's account.
77

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

1050
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
@@ -79,6 +79,53 @@ The `git.backend` DEBUG logger reports these decisions outside pytest.
7979
`git._backend.statistics()` returns a snapshot of the process-local counters.
8080
Zero observed calls means untested in that run, not unsupported.
8181

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

@@ -65,6 +66,24 @@ of the journey and parity checks. Include supported and fallback operations;
6566
the current patch measurement intentionally exercises fallback. Backend
6667
decisions are adapter events, not complete subprocess counts.
6768

69+
`cli_processes` counts successful launches through `Git.execute` during one
70+
warm invocation, including raw calls and the launch of persistent `cat-file`
71+
processes. Sending another request to an existing process does not increment
72+
it. Native/fallback decisions remain separate: the current warm Gix journey has
73+
two fallback decisions but launches only one process (the patch diff).
74+
Direct subprocesses in the harness and Git's own child processes are excluded.
75+
76+
The checked-in `cli-budget.json` sets maximum Gix launch counts for this pinned
77+
fixture. CI enforces the ceilings: reductions pass, increases fail with the
78+
measurement and counts. Lower ceilings when an optimization lands to retain
79+
the gain. The initial warm ceilings are journey/patch 1, opening 5, discovery 7,
80+
and zero for the other operations. New measurements need an explicit ceiling;
81+
do not automatically raise an existing ceiling to accept a regression.
82+
83+
The opening/discovery ceilings include a real CLI reference-format query
84+
(`GIX-1`). GixPython must expose the format before that call can disappear;
85+
Python inference does not count as a native implementation.
86+
6887
Timing is observational: shared CI runners and local activity introduce noise.
6988
The CI job publishes all means, standard deviations and ratios, retains raw
7089
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
@@ -155,6 +155,7 @@ def git(*arguments, cwd=args.repo):
155155
]
156156
)
157157
for name, operation, target in measurements:
158+
operation(target) # Warm persistent CLI processes before counting.
158159
# Untimed preflight validates parity and records native/fallback
159160
# decisions for ONE invocation, rather than calibrated loop counts.
160161
before = _backend.statistics()
@@ -167,6 +168,11 @@ def git(*arguments, cwd=args.repo):
167168
metadata = {
168169
"result_digest": sha256(json.dumps(result, sort_keys=True).encode()).hexdigest(),
169170
"backend_decisions": json.dumps(decisions, sort_keys=True),
171+
"cli_processes": sum(
172+
item["count"]
173+
for item in decisions
174+
if (item["method"], item["outcome"]) == ("Git.execute", "CLI process")
175+
),
170176
}
171177
runner.bench_func(name, operation, target, metadata=metadata)
172178

‎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)