From 3087322b44979c28ad0be8176450a76c3fa9b27e Mon Sep 17 00:00:00 2001 From: tzhouam Date: Tue, 29 Sep 2026 18:23:13 +0800 Subject: [PATCH 1/2] Judge each rule on the PRs it cites, with full upstream diffs The L2 judge saw each merged PR as an 8 KB diff excerpt, and saw the whole batch's evidence for every rule (about 105 KB for 10 PRs). In the first live run, "the supplied diff ends before ..." was the most common reason for an unsure or failed verdict. - New kb_service/evidence.for_rule narrows the evidence to the PRs a rule cites (^[PR #N]) and the PRs its lifecycle footer names as evidence (a retirement's justification). When the rule cites none we hold, it keeps all of them. - It replaces each upstream merged-PR excerpt with the full per-file diffs of the merged commit, read from the service's own upstream mirror (new MirrorObserver.file_diff). - Order: files the rule names first, then source, then tests, then docs and configuration. - Limits: 24 KB per file and 48 KB per rule. A cut is marked, and files past the budget are listed in diffs_omitted. - Anything the mirror cannot answer keeps its recorded excerpt. Only upstream merged-PR evidence (with merged_at) is expanded. - merged_pr events now record merge_commit_sha. Older events look it up through pull(). - run_gate(evidence_for=...) is wired from gate_and_stage. The facts check and the evidence expansion share one observer, so the mirror syncs once. Measured with the real judge (codex gpt-6-sol, medium) on the 7 rules disputed in the first live run: recorded evidence: 0 pass, 3 fail, 4 unsure (3 of them "diff truncated") full diffs: 1 pass, 5 fail, 1 unsure (a specific gap, no truncation) - The 3 rules with real errors (EXEC-17a, SERV-SLP-1a, MOSSTTS-5a) fail for the same reason in both arms. - COSYVOICE3-1d and MOSSTTS-4c now fail on concrete scope errors that the judge could see only in the full diff. - SENSENOVA-1d passes, verified against the pipeline diff. - SERV-VD-1 stays unsure for a specific, stated reason. - Evidence per rule: about 11-57 KB instead of 105 KB. A first version ordered files as GitHub lists them, so docs and tests used up the budget before the adapter code (MOSSTTS-4c). Ranking source first fixed that. Calibration cases carry no upstream evidence and are unaffected. Tests (test_kb_judge_evidence.py): only the cited PRs are shown; full diffs replace the excerpt, named files first; the byte limits hold and say so; source comes before tests and docs; an unreadable mirror keeps the excerpt; older evidence asks for its merge SHA; run-lesson evidence is left alone; each rule's judge prompt carries only its own PR; recorded evidence carries merge_commit_sha. Co-Authored-By: Claude Opus 5.5 --- doc/architecture/SPEC/kb_service.md | 9 + .../kb_service/evidence.py | 104 ++++++++++++ src/infermatrix_copilot/kb_service/gate.py | 13 +- src/infermatrix_copilot/kb_service/runtime.py | 12 +- src/infermatrix_copilot/kb_service/sources.py | 1 + .../kb_service/upstream_facts.py | 19 +++ test/test_kb_judge_evidence.py | 160 ++++++++++++++++++ 7 files changed, 313 insertions(+), 5 deletions(-) create mode 100644 src/infermatrix_copilot/kb_service/evidence.py create mode 100644 test/test_kb_judge_evidence.py diff --git a/doc/architecture/SPEC/kb_service.md b/doc/architecture/SPEC/kb_service.md index e578693b..3513957c 100644 --- a/doc/architecture/SPEC/kb_service.md +++ b/doc/architecture/SPEC/kb_service.md @@ -282,3 +282,12 @@ issue（同一报告只开一个），标签不存在时不带标签重试，绝 的任何变更都会被永久挡住并白白消耗精炼轮次）。没有写出规则 ID 的冲突仍算本次改动的（失败即关闭）。评审提示中带 `changed_rule_ids`。校准不传 `changed`，保持整目录严格判定。首个线上运行即遇到此情况：configuration 目录中 CONF-1a..4a 在 两页重复、DIFF-2s/2t、SERV-4o/4r5 在 main 上已冲突，导致两批 intake 全部 `refine_exhausted`。 + +## 2026-09-29 L2 按规则取证据 +`evidence.for_rule(rule_text, evidence, observer)`：L2 评审每条规则时只给它引用的 PR（`^[PR #N]`；一个都不在批次里则 +全部给），并把这些上游合并 PR 的 8 KB 摘录换成合并提交的完整逐文件 diff（服务自己的上游镜像，`MirrorObserver.file_diff`）： +规则提到的文件在前，每文件 24 KB、每条规则 48 KB，截断处注明；超出预算的文件列在 `diffs_omitted`。镜像读不到或没有 +diff 时保留原摘录（证据不会因此变得更"好过"）。只扩展带 `merged_at` 的上游 PR 证据；Copilot 运行经验与外部 PR 证据不变。 +`merged_pr` 事件的证据新增 `merge_commit_sha`（旧事件经 `pull()` 查询）。`run_gate(evidence_for=...)` 由 `gate_and_stage` +与事实证明共用同一个观测者（一次镜像同步）。首个线上运行中"所给 diff 在…之前截断"是 L2 判 unsure/fail 的主要原因， +而整批 10 个 PR 的证据（约 105 KB）对每条规则都重复发送；按规则取证据后约 11 KB + 所引 PR 的完整 diff。 diff --git a/src/infermatrix_copilot/kb_service/evidence.py b/src/infermatrix_copilot/kb_service/evidence.py new file mode 100644 index 00000000..87d045aa --- /dev/null +++ b/src/infermatrix_copilot/kb_service/evidence.py @@ -0,0 +1,104 @@ +"""What the L2 judge sees for ONE rule: the upstream PRs it cites, with full diffs. + +An intake event stores a merged PR with an 8 KB diff excerpt (enough to draft +from), and a change set judges every rule against the evidence of its whole +batch. The judge then reads ten PRs' worth of excerpts and still misses the +code a rule describes: "the supplied diff ends before ..." was the most common +reason for an unsure or failed verdict in the first live run. + +``for_rule`` narrows the evidence to the PRs the rule cites (``^[PR #N]``, and +the PRs its lifecycle footer names as evidence; all of them when it cites none +we hold) and replaces each excerpt with the full +per-file diffs of the PR as GitHub shows it (its head against its merge +base with main), read from the service's own upstream +mirror: files the rule names first, then source, tests, docs and configuration, within ``PER_FILE`` and +``PER_RULE`` bytes. Only upstream merged-PR evidence is expanded; anything the +mirror cannot answer keeps its excerpt (the judge says "unsure" rather than +guessing), so expansion never makes a verdict easier than today. +""" + +from __future__ import annotations + +import re + +from ..knowledge_service.facts import CODE_SPAN, EVIDENCE_PR, PR_CITATION, FactsError +from ..knowledge_service.lifecycle import FOOTER + +PER_FILE = 24 * 1024 +PER_RULE = 48 * 1024 +NOTE = ("full per-file diffs of the merged commit (files the rule names first); " + "a file cut at the byte limit says so") + + +def _named(text: str) -> list[str]: + out = [] + for m in CODE_SPAN.finditer(text): + token = m.group("tok").strip().partition("::")[0].strip("/") + if "/" in token or re.search(r"\.[A-Za-z0-9]{1,5}$", token): + out.append(token) + return out + + +def _rank(path: str) -> int: + """Source before tests before docs and configuration (the code a rule describes first).""" + parts = path.lower().split("/") + if any(p in ("docs", "doc", "recipes", "examples") for p in parts[:-1]) or \ + parts[-1].endswith((".md", ".rst", ".txt", ".yaml", ".yml", ".json", ".toml")): + return 2 + if any(p in ("tests", "test") for p in parts[:-1]) or parts[-1].startswith("test_"): + return 1 + return 0 + + +def _matches(path: str, names: list[str]) -> bool: + return any(path == n or path.endswith("/" + n) or n.endswith("/" + path) for n in names) + + +def _cut(text: str, limit: int) -> str: + data = text.encode("utf-8") + if len(data) <= limit: + return text + return data[:limit].decode("utf-8", "ignore") + f"\n[... cut at {limit} bytes of {len(data)}]\n" + + +def for_rule(text: str, evidence: list[dict], observer) -> list[dict]: + """The evidence for one rule (``text``: the rule as proposed, or as it was).""" + cited = {f"PR #{m.group('n')}" for m in PR_CITATION.finditer(text)} + # a retirement's justification lives in its footer (evidence="PR #N"), not in a citation + for line in text.splitlines(): + footer = FOOTER.match(line.strip()) + if footer: + cited |= {f"PR #{m.group('n')}" for m in EVIDENCE_PR.finditer(footer.group("attrs"))} + items = [e for e in evidence if e.get("source_reference") in cited] or list(evidence) + if observer is None: + return items + names, budget, out = _named(text), PER_RULE, [] + for item in items: + number = re.fullmatch(r"PR #(\d+)", str(item.get("source_reference") or "")) + if not number or "merged_at" not in item or budget <= 0: + out.append(item) # not an upstream merged PR (or no room left): as recorded + continue + try: + sha = str(item.get("merge_commit_sha") or observer.pull(int(number.group(1))).get("merge_commit_sha") or "") + if not sha: + raise FactsError("no merge commit") + files = list(item.get("changed_files") or []) + ordered = sorted(files, key=lambda f: (not _matches(f, names), _rank(f))) + diffs: dict[str, str] = {} + for path in ordered: + if budget <= 0: + break + diff = _cut(observer.pr_diff(int(number.group(1)), sha, path), min(PER_FILE, budget)) + if diff: + diffs[path] = diff + budget -= len(diff.encode("utf-8")) + except FactsError: + out.append(item) + continue + if not diffs: + out.append(item) + continue + expanded = {k: v for k, v in item.items() if k != "diff_excerpt"} + out.append({**expanded, "diffs": diffs, "diffs_note": NOTE, + "diffs_omitted": [f for f in ordered if f not in diffs]}) + return out diff --git a/src/infermatrix_copilot/kb_service/gate.py b/src/infermatrix_copilot/kb_service/gate.py index e896e7c2..79a6fcdb 100644 --- a/src/infermatrix_copilot/kb_service/gate.py +++ b/src/infermatrix_copilot/kb_service/gate.py @@ -129,10 +129,14 @@ def _neighbours(files: dict[str, str], path: str, rule_id: str, limit: int = 30) def judge_block(block: Block, *, base: dict[str, str], head: dict[str, str], evidence: list[dict], - gateway: ModelGateway, judge: ModelRole) -> BlockVerdict: + gateway: ModelGateway, judge: ModelRole, evidence_for=None) -> BlockVerdict: + """``evidence_for(rule_text, evidence)`` narrows and expands the evidence for + this one block (None: the change set's evidence as recorded).""" dimensions = DIMENSIONS[block.op] before = _section_text(base, block.path, block.rule_id) if block.op != "add" else "" after = _section_text(head, block.path, block.rule_id) if block.op != "purge" else "" + if evidence_for is not None and block.kind == "rule": + evidence = evidence_for(after or before, evidence) payload = { "change": {"op": block.op, "page": block.path, "rule_id": block.rule_id, "before": before, "after": after}, @@ -253,7 +257,8 @@ def run_gate(*, base: dict[str, str], head: dict[str, str], changes: list[Change external_texts: dict[str, str], evidence: list[dict], gateway: ModelGateway, judge: ModelRole, release: str, repo_dir: str, protected_rules: tuple[str, ...] = (), - retire_ratio: float = 0.10, max_files: int = 50, facts=None) -> GateDecision: + retire_ratio: float = 0.10, max_files: int = 50, facts=None, + evidence_for=None) -> GateDecision: """``facts`` observes the repository's public upstream (None: the repository has no upstream to attest, or it publishes nothing).""" l1 = check_changeset(base, head, changes, external_texts=external_texts, release=release) @@ -288,8 +293,8 @@ def run_gate(*, base: dict[str, str], head: dict[str, str], changes: list[Change reasons.append(f"circuit breaker: {removed} of {active_before} rules retired/purged") if len(changes) > max_files: reasons.append(f"circuit breaker: {len(changes)} files changed (limit {max_files})") - blocks = [judge_block(b, base=base, head=head, evidence=evidence, gateway=gateway, judge=judge) - for b in l1.blocks] + blocks = [judge_block(b, base=base, head=head, evidence=evidence, gateway=gateway, judge=judge, + evidence_for=evidence_for) for b in l1.blocks] if any(b.verdict == "fail" for b in blocks): return GateDecision("fail", reasons + [f"L2 rejected {b.block.rule_id or b.block.path}" for b in blocks if b.verdict == "fail"], l1, blocks, diff --git a/src/infermatrix_copilot/kb_service/runtime.py b/src/infermatrix_copilot/kb_service/runtime.py index 0ba04bfb..1fc14a9e 100644 --- a/src/infermatrix_copilot/kb_service/runtime.py +++ b/src/infermatrix_copilot/kb_service/runtime.py @@ -295,13 +295,14 @@ def gate_and_stage(rt: KbRuntime, lifecycle: RepoLifecycle, owner: str, *, kind: rt.ledger.heartbeat(owner) changeset_id = rt.ledger.new_changeset_id(lifecycle.repo, kind) rule_ids = sorted({op.new_rule_id or op.rule_id for op in operations if (op.new_rule_id or op.rule_id)}) + observer = rt.upstream_facts(lifecycle) # one mirror sync for facts and evidence with trace_context(changeset_id=changeset_id, rule_ids=rule_ids, step="gate"): decision = run_gate( base=base, head=head, changes=changes_between(base, head), external_texts=external, evidence=evidence, gateway=rt.gateway, judge=rt.judge, release=release, repo_dir=lifecycle.knowledge_dir, protected_rules=lifecycle.protected_rules, retire_ratio=lifecycle.retire_ratio, max_files=lifecycle.max_files, - facts=rt.upstream_facts(lifecycle)) + facts=observer, evidence_for=_evidence_for(observer)) if force_human and decision.status == "pass": decision.status = "human" decision.reasons.append(force_human) @@ -397,6 +398,15 @@ def _pr_body(changeset_id: str, detail: dict) -> str: return "\n".join(lines) +def _evidence_for(observer): + """Per-rule evidence with full upstream diffs (the judge's view; see kb_service.evidence).""" + if observer is None: + return None + from .evidence import for_rule + + return lambda text, evidence: for_rule(text, evidence, observer) + + def service_observer(state_dir: Path, lifecycle: RepoLifecycle, github: GitHubReader): """The service's view of a public upstream: its own mirror (shared with the release sweep) and the read-only GitHub reader. A repository without an diff --git a/src/infermatrix_copilot/kb_service/sources.py b/src/infermatrix_copilot/kb_service/sources.py index 7c82e087..6e6c33d3 100644 --- a/src/infermatrix_copilot/kb_service/sources.py +++ b/src/infermatrix_copilot/kb_service/sources.py @@ -206,6 +206,7 @@ def evidence(self) -> dict: "title": self.title, "body": self.body[:MAX_BODY_CHARS], "merged_at": self.merged_at, + "merge_commit_sha": self.merge_commit_sha, "changed_files": list(self.changed_files[:MAX_CHANGED_PATHS]), "diff_excerpt": self.diff_excerpt, } diff --git a/src/infermatrix_copilot/kb_service/upstream_facts.py b/src/infermatrix_copilot/kb_service/upstream_facts.py index defe31a8..1e160dbd 100644 --- a/src/infermatrix_copilot/kb_service/upstream_facts.py +++ b/src/infermatrix_copilot/kb_service/upstream_facts.py @@ -73,6 +73,25 @@ def file_text(self, sha: str, path: str) -> str | None: proc = self._git("cat-file", "blob", f"{sha}:{path}", ok=(0, 1, 128)) return proc.stdout if proc.returncode == 0 else None + def pr_diff(self, number: int, merge_sha: str, path: str) -> str: + """What PR ``number`` changed in ``path``, as GitHub shows it: from the + merge base of its head with main before it landed (``merge_sha^1``) to + its head. Right for squash, merge-commit and rebase merges alike (for a + rebase merge ``merge_sha`` is only the LAST rebased commit).""" + base, head = self._pr_range(number, merge_sha) + return self._git("diff", "--no-color", "-U3", base, head, "--", path).stdout + + def _pr_range(self, number: int, merge_sha: str) -> tuple[str, str]: + cache = self.__dict__.setdefault("_ranges", {}) + if number not in cache: + self._commit(merge_sha) + ref = f"refs/kb-pr/{number}" + self._git("fetch", "--quiet", "origin", f"+refs/pull/{number}/head:{ref}") + head = self._git("rev-parse", f"{ref}^{{commit}}").stdout.strip() + base = self._git("merge-base", f"{merge_sha}^1", head).stdout.strip() + cache[number] = (base, head) + return cache[number] + def pull(self, number: int) -> dict: try: data = self._pull(number) diff --git a/test/test_kb_judge_evidence.py b/test/test_kb_judge_evidence.py new file mode 100644 index 00000000..6cd392dc --- /dev/null +++ b/test/test_kb_judge_evidence.py @@ -0,0 +1,160 @@ +"""The L2 judge's evidence per rule: only the PRs it cites, with full upstream diffs.""" + +from __future__ import annotations + +import json + +from infermatrix_copilot.kb_service import evidence as ev +from infermatrix_copilot.kb_service.gate import changes_between, run_gate +from infermatrix_copilot.knowledge_service.facts import FactsError +from infermatrix_copilot.knowledge_service.ops import KnowledgeOperation as Op, apply_operations +from test_kb_intake_gate import JUDGE, PAGE, ScriptedGateway, _judge_all, _rule, _tree + + +def _pr(n, files, sha="m" * 40): + return {"source_reference": f"PR #{n}", "title": f"pr {n}", "merged_at": "2026-09-29T00:00:00Z", + "merge_commit_sha": sha, "changed_files": files, "diff_excerpt": "--- cut short"} + + +class Mirror: + def __init__(self, diffs, *, down=False): + self.diffs, self.down, self.asked = diffs, down, [] + + def pr_diff(self, number, sha, path): + if self.down: + raise FactsError("mirror unreadable") + self.asked.append(path) + return self.diffs.get(path, "") + + def pull(self, number): + return {"merged": True, "merge_commit_sha": "p" * 40} + + +EVIDENCE = [_pr(10, ["docs/a.md", "pkg/serving_speech.py", "tests/test_x.py"]), _pr(11, ["pkg/other.py"]), + {"source_reference": "run r1", "title": "a lesson"}] + + +def test_only_the_cited_prs_are_shown_or_everything_when_none_is_held(): + assert [e["source_reference"] for e in ev.for_rule("x ^[PR #11]", EVIDENCE, None)] == ["PR #11"] + assert ev.for_rule("x ^[PR #99]", EVIDENCE, None) == EVIDENCE + + +def test_full_diffs_replace_the_excerpt_named_files_first(): + mirror = Mirror({"docs/a.md": "+doc\n", "pkg/serving_speech.py": "+handler\n", "tests/test_x.py": "+test\n"}) + (item,) = ev.for_rule("见 `serving_speech.py::delete_voice` ^[PR #10]", EVIDENCE, mirror) + assert "diff_excerpt" not in item and list(item["diffs"]) == ["pkg/serving_speech.py", "tests/test_x.py", + "docs/a.md"] + assert item["diffs"]["pkg/serving_speech.py"] == "+handler\n" and item["diffs_omitted"] == [] + + +def test_the_byte_limits_hold_and_say_so(monkeypatch): + monkeypatch.setattr(ev, "PER_FILE", 10) + monkeypatch.setattr(ev, "PER_RULE", 15) + mirror = Mirror({"docs/a.md": "x" * 50, "pkg/serving_speech.py": "y" * 5, "tests/test_x.py": "z" * 50}) + (item,) = ev.for_rule("`pkg/serving_speech.py` ^[PR #10]", EVIDENCE, mirror) + assert item["diffs"]["pkg/serving_speech.py"] == "y" * 5 + assert item["diffs"]["tests/test_x.py"].startswith("z" * 10) and "cut at 10 bytes of 50" in item["diffs"]["tests/test_x.py"] + assert item["diffs_omitted"] == ["docs/a.md"] + + +def test_source_comes_before_tests_and_docs_when_the_rule_names_no_file(): + files = ["docs/guide.md", "tests/test_a.py", "pkg/deploy/x.yaml", "pkg/adapter.py"] + mirror = Mirror({f: "+\n" for f in files}) + (item,) = ev.for_rule("^[PR #10]", [_pr(10, files)], mirror) + assert list(item["diffs"]) == ["pkg/adapter.py", "tests/test_a.py", "docs/guide.md", "pkg/deploy/x.yaml"] + + +def test_an_unreadable_mirror_keeps_the_recorded_excerpt(): + (item,) = ev.for_rule("^[PR #10]", EVIDENCE, Mirror({}, down=True)) + assert item == EVIDENCE[0] + (item,) = ev.for_rule("^[PR #10]", EVIDENCE, Mirror({})) # nothing to show: as recorded + assert item == EVIDENCE[0] + + +def test_older_evidence_without_a_merge_sha_asks_for_it_and_lessons_are_untouched(): + old = {k: v for k, v in EVIDENCE[0].items() if k != "merge_commit_sha"} + mirror = Mirror({"docs/a.md": "+doc\n"}) + (item,) = ev.for_rule("^[PR #10]", [old], mirror) + assert item["diffs"] == {"docs/a.md": "+doc\n"} + assert ev.for_rule("no citation", [EVIDENCE[2]], mirror) == [EVIDENCE[2]] + + +def test_each_rule_is_judged_on_its_own_evidence(): + base = _tree() + head = {**base, **apply_operations(base, [Op("add", PAGE, "DEMO-2a", _rule("DEMO-2a", "PR #11"))], + release="v1", today="2026-09-29").files} + gateway = ScriptedGateway(_judge_all("yes")) + decision = run_gate(base=base, head=head, changes=changes_between(base, head), external_texts={}, + evidence=EVIDENCE, gateway=gateway, judge=JUDGE, release="v1", repo_dir="repos/demo", + evidence_for=lambda text, items: ev.for_rule(text, items, Mirror({"pkg/other.py": "+o\n"}))) + assert decision.status == "pass" + prompts = [p for _r, p in gateway.calls if '"dimensions_to_answer"' in p] + payload = json.loads(prompts[0].split("\n", 1)[1].rsplit("\n", 1)[0]) + assert [e["source_reference"] for e in payload["evidence"]] == ["PR #11"] + assert payload["evidence"][0]["diffs"] == {"pkg/other.py": "+o\n"} + + +def test_recorded_pr_evidence_carries_its_merge_commit(): + from infermatrix_copilot.kb_service.sources import PullRequest + + pr = PullRequest(7, "t", "b", "2026-09-29", "c" * 40, "dev", ("a.py",), "diff") + assert pr.evidence()["merge_commit_sha"] == "c" * 40 + + + +def _git(cwd, *args): + import subprocess + + return subprocess.run(["git", "-C", str(cwd), *args], check=True, capture_output=True, text=True).stdout.strip() + + +def _commit(repo, path, text, message): + target = repo / path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(text) + _git(repo, "add", "-A") + _git(repo, "-c", "user.name=t", "-c", "user.email=t@e", "commit", "-q", "-m", message) + return _git(repo, "rev-parse", "HEAD") + + +def test_a_rebase_merged_pr_shows_every_commit_not_only_the_last(tmp_path): + from infermatrix_copilot.kb_service.upstream_facts import MirrorObserver + + up = tmp_path / "up" + up.mkdir() + _git(up, "init", "-q", "-b", "main") + _commit(up, "README.md", "x\n", "base") + _git(up, "checkout", "-q", "-b", "feature") + _commit(up, "pkg/feature.py", "def feature():\n return 1\n", "A: the behaviour") + head = _commit(up, "tests/test_feature.py", "def test_feature():\n pass\n", "B: its test") + _git(up, "update-ref", "refs/pull/7/head", head) + _git(up, "checkout", "-q", "main") + _commit(up, "other.py", "y\n", "main moves on") + _git(up, "-c", "user.name=t", "-c", "user.email=t@e", "cherry-pick", "feature~1", "feature") + merged = _git(up, "rev-parse", "HEAD") # the LAST rebased commit only + observer = MirrorObserver(tmp_path / "mirror.git", "org/up", lambda n: {}, url=str(up)) + assert "def feature" in observer.pr_diff(7, merged, "pkg/feature.py") # from commit A + assert "def test_feature" in observer.pr_diff(7, merged, "tests/test_feature.py") + assert observer.pr_diff(7, merged, "other.py") == "" # main's own change is not the PR's + (item,) = ev.for_rule("^[PR #7]", [_pr(7, ["pkg/feature.py", "tests/test_feature.py"], sha=merged)], observer) + assert list(item["diffs"]) == ["pkg/feature.py", "tests/test_feature.py"] + + +def test_a_pr_head_that_cannot_be_fetched_keeps_the_excerpt(tmp_path): + from infermatrix_copilot.kb_service.upstream_facts import MirrorObserver + + up = tmp_path / "up" + up.mkdir() + _git(up, "init", "-q", "-b", "main") + merged = _commit(up, "pkg/a.py", "a\n", "squashed (#8)") + observer = MirrorObserver(tmp_path / "mirror.git", "org/up", lambda n: {}, url=str(up)) + (item,) = ev.for_rule("^[PR #8]", [_pr(8, ["pkg/a.py"], sha=merged)], observer) # no refs/pull/8/head + assert item["diff_excerpt"] == "--- cut short" and "diffs" not in item + + + +def test_a_retirement_keeps_the_pr_that_justifies_it(): + retired = ("## DEMO-1a — old\n\n- 强制：旧规则。 ^[PR #10]\n\n" + '\n') + shown = [e["source_reference"] for e in ev.for_rule(retired, EVIDENCE, None)] + assert shown == ["PR #10", "PR #11"] From 0ca048927329b2e913160aff587338d8142f1e3f Mon Sep 17 00:00:00 2001 From: tzhouam Date: Tue, 29 Sep 2026 18:26:28 +0800 Subject: [PATCH 2/2] Keep rename history in the judge's per-file PR diffs A path-filtered `git diff base head -- path` turns rename detection off. A renamed and edited file then showed up as all-new content, and the lines it removed were lost, so L2 could miss a behaviour removal. That is worse than the excerpt it replaces. MirrorObserver.pr_diff now lists the PR's files once with NUL-separated `--name-status -M -z` (renames paired; spaces and non-ASCII names come through exactly, no quote parsing), then diffs each file with both its old and new path in the pathspec, which keeps rename detection on. Test: with real git, a PR that renames a file and drops a function keeps "rename from", keeps the removed lines, does not show the file as all new, and is found under both names. Co-Authored-By: Claude Opus 5.5 --- .../kb_service/upstream_facts.py | 26 ++++++++++- test/test_kb_judge_evidence.py | 45 +++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/infermatrix_copilot/kb_service/upstream_facts.py b/src/infermatrix_copilot/kb_service/upstream_facts.py index 1e160dbd..44c84922 100644 --- a/src/infermatrix_copilot/kb_service/upstream_facts.py +++ b/src/infermatrix_copilot/kb_service/upstream_facts.py @@ -79,7 +79,31 @@ def pr_diff(self, number: int, merge_sha: str, path: str) -> str: its head. Right for squash, merge-commit and rebase merges alike (for a rebase merge ``merge_sha`` is only the LAST rebased commit).""" base, head = self._pr_range(number, merge_sha) - return self._git("diff", "--no-color", "-U3", base, head, "--", path).stdout + pair = self._pr_files(number, base, head).get(path) + if pair is None: + return "" + # both sides of a rename in the pathspec: rename detection stays on + return self._git("diff", "--no-color", "-U3", "-M", base, head, "--", *dict.fromkeys(pair)).stdout + + def _pr_files(self, number: int, base: str, head: str) -> dict[str, tuple[str, ...]]: + """Every path the PR touches -> (old path, new path), from NUL-separated + name-status (no quoting, so spaces and non-ASCII names are exact).""" + cache = self.__dict__.setdefault("_files", {}) + if number not in cache: + fields = self._git("diff", "-M", "--name-status", "-z", base, head).stdout.split("\0") + pairs: dict[str, tuple[str, ...]] = {} + k = 0 + while k < len(fields) and fields[k]: + status = fields[k] + if status[:1] in ("R", "C"): + old, new = fields[k + 1], fields[k + 2] + k += 3 + else: + old = new = fields[k + 1] + k += 2 + pairs[old] = pairs[new] = (old, new) + cache[number] = pairs + return cache[number] def _pr_range(self, number: int, merge_sha: str) -> tuple[str, str]: cache = self.__dict__.setdefault("_ranges", {}) diff --git a/test/test_kb_judge_evidence.py b/test/test_kb_judge_evidence.py index 6cd392dc..9dfda50c 100644 --- a/test/test_kb_judge_evidence.py +++ b/test/test_kb_judge_evidence.py @@ -158,3 +158,48 @@ def test_a_retirement_keeps_the_pr_that_justifies_it(): '\n') shown = [e["source_reference"] for e in ev.for_rule(retired, EVIDENCE, None)] assert shown == ["PR #10", "PR #11"] + + + +def test_a_renamed_and_edited_file_keeps_its_removed_lines(tmp_path): + from infermatrix_copilot.kb_service.upstream_facts import MirrorObserver + + up = tmp_path / "up" + up.mkdir() + _git(up, "init", "-q", "-b", "main") + body = "".join(f"line {i}\n" for i in range(40)) + _commit(up, "pkg/old_name.py", body + "def removed_behaviour():\n pass\n", "base") + _git(up, "checkout", "-q", "-b", "feature") + _git(up, "mv", "pkg/old_name.py", "pkg/new_name.py") + head = _commit(up, "pkg/new_name.py", body, "rename, drop removed_behaviour") + _git(up, "update-ref", "refs/pull/9/head", head) + _git(up, "checkout", "-q", "main") + _git(up, "-c", "user.name=t", "-c", "user.email=t@e", "merge", "-q", "--squash", "feature") + merged = _commit(up, "pkg/new_name.py", body, "squash (#9)") + observer = MirrorObserver(tmp_path / "mirror.git", "org/up", lambda n: {}, url=str(up)) + patch = observer.pr_diff(9, merged, "pkg/new_name.py") + assert "rename from pkg/old_name.py" in patch and "-def removed_behaviour():" in patch + assert "+line 0" not in patch # not shown as all-new + assert observer.pr_diff(9, merged, "pkg/old_name.py") == patch + + + +def test_paths_with_spaces_and_non_ascii_names_are_found(tmp_path): + from infermatrix_copilot.kb_service.upstream_facts import MirrorObserver + + up = tmp_path / "up" + up.mkdir() + _git(up, "init", "-q", "-b", "main") + _commit(up, "README.md", "x\n", "base") + _git(up, "checkout", "-q", "-b", "feature") + _commit(up, "docs/my guide.md", "spaced\n", "a spaced name") + head = _commit(up, "docs/说明.md", "非 ASCII\n", "a non-ASCII name") + _git(up, "update-ref", "refs/pull/5/head", head) + _git(up, "checkout", "-q", "main") + _git(up, "-c", "user.name=t", "-c", "user.email=t@e", "merge", "-q", "--squash", "feature") + _git(up, "-c", "user.name=t", "-c", "user.email=t@e", "commit", "-q", "-m", "squash (#5)") + merged = _git(up, "rev-parse", "HEAD") + observer = MirrorObserver(tmp_path / "mirror.git", "org/up", lambda n: {}, url=str(up)) + assert "+spaced" in observer.pr_diff(5, merged, "docs/my guide.md") + assert "+非 ASCII" in observer.pr_diff(5, merged, "docs/说明.md") + assert observer.pr_diff(5, merged, "README.md") == "" # not the PR's