Skip to content

Commit f46557c

Browse files
committed
test(diff): assert no contained/unmerged fragments; QA.md sweep notes
1 parent 08f3795 commit f46557c

2 files changed

Lines changed: 79 additions & 5 deletions

File tree

QA.md

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,15 @@ shadow:
140140
```bash
141141
cd test-repos/<repo>
142142
git pull --ff-only
143-
# Hard-cap runtime. As of 3725f51b the CLI enforces `--timeout` as a true total
144-
# wall-clock bound (worker thread + watchdog) and fast-fails rc=124 with the
145-
# actionable-error contract instead of hanging to OOM (#70) — BUT the external
146-
# perl cap is still needed when smoking the **pipx** binary until that fix ships
147-
# in a release (pipx 1.10.0 has no watchdog). macOS has no `timeout`; use:
143+
# Hard-cap runtime. `--timeout` (true wall-clock bound, worker thread + watchdog,
144+
# fast-fails instead of hanging to OOM, #70, 3725f51b) lives ONLY on the
145+
# standalone Rust binary `diffctx/src/main.rs`. The **Python pipx wrapper does
146+
# NOT expose `--timeout` at all** — verified on the released `1.10.2`:
147+
# `diffctx --diff --help | grep timeout` → empty, and `grep -r timeout
148+
# src/diffctx/cli.py` → empty (the flag never crossed into the Python CLI). So
149+
# the external perl cap is MANDATORY for every pipx smoke regardless of release,
150+
# not just "until it ships" — the wrapper needs its own watchdog around the
151+
# pyo3 `compute_scored_state` call first. macOS has no `timeout`; use:
148152
perl -e 'alarm 200; exec @ARGV' /Users/nikolay/.local/bin/diffctx . --diff HEAD~1
149153
# (or, on the dev build, just pass `--timeout 200` — the watchdog bounds it.)
150154
```
@@ -174,6 +178,26 @@ Either outcome — **all clean OR ≥1 issue filed** — is a valid completion o
174178
this step; the QA round proceeds to its remaining items either way. Do not let
175179
a found issue halt the round, and do not keep sweeping after one is filed.
176180

181+
## Sweep Discriminator: EOF-Append to a Flat Data-List File (#103)
182+
183+
A distinct symptom the file-count/token discriminators alone miss, surfaced
184+
sweeping `elasticsearch`: a diff whose ONLY change is a few lines **appended at
185+
EOF** of a large flat YAML/data-list file (e.g. `muted-tests.yml`, ~790 lines of
186+
top-level `- key: …` entries) produced an output with **zero `role: "changed"`
187+
fragments** — the changed lines never surfaced as a labeled change — while the
188+
whole file was exploded into ~118 per-entry `definition` context fragments plus
189+
a couple of unrelated files. Two separable defects: (A) **the change signal is
190+
lost** (correctness — worse than over-selection; `grep -c 'role:' out.yaml` → 0,
191+
and the max rendered line range stops BELOW the appended lines), and (B)
192+
whole-file fragmentation of a flat list (over-selection, overlaps #65). Tracked
193+
on #103, primarily for (A).
194+
195+
**Add to the per-repo sweep judgment:** on any non-empty diff, assert
196+
`grep -c 'role: "changed"'` ≥ 1 AND that the output's max line range reaches the
197+
diff's changed lines. A clean rc=0 with a plausible token count still hides this
198+
class — `role:`-absence is the tell. Contrast a healthy run (`numpy HEAD~1`:
199+
5 `role: "changed"` fragments, context scoped to 2 tightly-related stubs).
200+
177201
## Local `which diffctx` Trap — diffctx specifics
178202

179203
See `/qa` skill: Packaging QA (`which`-vs-pipx). Concretely: when this project's

tests/test_diffctx_invariants.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import itertools
34
import re
45
import subprocess
56
import sys
@@ -243,6 +244,55 @@ def test_diff_context_merges_contiguous_fragments(tmp_path):
243244
assert start <= end, f"fragment line range inverted: {f['lines']}"
244245

245246

247+
def test_diff_context_no_contained_or_unmerged_fragments(tmp_path):
248+
"""Regression (f1a1647d): the render merge pass must, per (path, role),
249+
drop any fragment fully contained in another and merge line-contiguous
250+
runs. A change editing several functions in one file produces multiple
251+
same-role fragments, exercising that pass; the output must never carry a
252+
range nested inside a sibling nor an unmerged `next.start == cur.end + 1`
253+
pair (both are pure per-fragment scaffolding duplication)."""
254+
import json
255+
from collections import defaultdict
256+
257+
repo = Pygit2Repo(tmp_path / "repo")
258+
base_src = "".join(f"def f{i}(x):\n y = x + {i}\n return y\n\n" for i in range(12))
259+
repo.add_file("src/mod.py", base_src)
260+
repo.add_file("src/other.py", "from mod import f0, f5, f11\n\ndef run():\n return f0(1) + f5(2) + f11(3)\n")
261+
base = repo.commit("init")
262+
edited_src = "".join(
263+
(
264+
f"def f{i}(x):\n y = x * {i}\n z = y + 1\n return z\n\n"
265+
if i in (2, 5, 9)
266+
else f"def f{i}(x):\n y = x + {i}\n return y\n\n"
267+
)
268+
for i in range(12)
269+
)
270+
repo.add_file("src/mod.py", edited_src)
271+
head = repo.commit("edit several functions")
272+
273+
stdout, _ = _run(repo.path, [".", "--diff", f"{base}..{head}", "--budget", "8000", "-f", "json"])
274+
fragments = json.loads(stdout)["fragments"]
275+
276+
by_key: dict[tuple[str, str | None], list[tuple[int, int]]] = defaultdict(list)
277+
for f in fragments:
278+
start, end = (int(x) for x in f["lines"].split("-"))
279+
by_key[(f["path"], f.get("role"))].append((start, end))
280+
281+
assert any(
282+
len(ranges) >= 2 for ranges in by_key.values()
283+
), "fixture must yield a multi-fragment group to exercise the merge pass"
284+
285+
for (path, role), ranges in by_key.items():
286+
ordered = sorted(ranges)
287+
for (a_start, a_end), (b_start, b_end) in itertools.pairwise(ordered):
288+
assert not (
289+
b_start >= a_start and b_end <= a_end
290+
), f"contained fragment {(b_start, b_end)} inside {(a_start, a_end)} for {path} role={role}"
291+
assert (
292+
b_start != a_end + 1
293+
), f"unmerged contiguous fragments {(a_start, a_end)} and {(b_start, b_end)} for {path} role={role}"
294+
295+
246296
def test_diff_context_scopes_markdown_preamble_change_not_whole_file(tmp_path):
247297
"""Regression (#91): a change to a lone H1's own preamble (before its
248298
first `##` child heading) used to select a fragment spanning the H1's

0 commit comments

Comments
 (0)