From f63d6b0cb5787f893c3ba40dfa961a44beefa0a3 Mon Sep 17 00:00:00 2001 From: Tim Stranske Date: Sun, 23 Aug 2026 08:52:34 -0500 Subject: [PATCH 1/2] fix(evidence): drop the dead combined-haystack binding, and record why it stays dead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_dedupe_candidate` built `haystack = f"#{issue_number} {issue_title}"` and then searched `issue_title` and `candidate["seed"]` only. ruff F841 flagged the unused local; PR #58 removed the binding as lint. This decides the question that removal left open — was the combined form the intent? No, and it is inert rather than merely unused: * Substituting `haystack` for `issue_title` in the first `_contains_pr_ref` adds EXACTLY one condition, `issue_number == pr_number`. It does not widen the text searched. Checked by brute force over 200k random (issue_number, pr_number, title) triples: 0 mismatches against `ref(title, pr) or issue_number == pr`. * That condition cannot occur: GitHub draws issue and PR numbers from one per-repo sequence. Over recorded history — 202 dedupe-eligible candidates (reverted/abandoned keepalive rows carrying a PR number, 2026-01-08..2026-08-22, 11 repos) against 4,031 issues in those same repos — 0 number collisions. * And it is already covered even if it could: every seed `evidence_for_repo` builds opens with "PR #{pr_number}", so the adjacent `_contains_pr_ref(candidate["seed"], issue_number)` disjunct is already true in exactly that case. Differential run of the real `_dedupe_candidate` against a haystack-wired copy over 120k real-shaped candidates, with `issue_number == pr_number` forced in ~40% of search hits: 0 behaviour differences. So the effect on matching is +0 candidates, not merely over history but under any input. The one way it could ever fire is as a false positive claiming duplication from a numeric coincidence. Also corrects the premise behind the question: `_contains_pr_ref` ALREADY matches a bare "#N" in an issue title — the "#" in `#?{n}` is optional and `(? title+body, which widens the TEXT. The explanation sits ABOVE the matching loop rather than in the deleted line's slot, so an identical deletion on both sides merges cleanly with PR #58 instead of conflicting. Selftest pins the decision with two assertions and a deliberate-break -> revert demonstration: wiring the haystack in fails assertion 1 (exit 1); removing "PR #" from the reversal seed fails assertion 2 (exit 1); revert restores byte-for-byte and both pass (exit 0). Co-Authored-By: Claude Opus 5 --- keepalive_evidence.py | 47 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/keepalive_evidence.py b/keepalive_evidence.py index c63dbbd..203fa65 100644 --- a/keepalive_evidence.py +++ b/keepalive_evidence.py @@ -219,10 +219,19 @@ def _dedupe_candidate( seen_issue_numbers.add(issue_number) issues.append(issue) seed_tokens = _tokens(title) or _tokens(candidate["seed"]) + # Matching below is on `issue_title` and on `candidate["seed"]` -- deliberately NOT on a + # combined f"#{issue_number} {issue_title}". Prepending the issue's OWN number widens the + # match by exactly one condition, issue_number == pr_number, and that condition is dead twice + # over: GitHub draws issue and PR numbers from a single per-repo sequence so the two can never + # be equal, and the seed disjunct already covers it anyway, since every seed built in + # evidence_for_repo opens with "PR #{pr_number}". If it ever did fire it would claim + # duplication from a bare numeric coincidence. Note `_contains_pr_ref` already matches a bare + # "#N" inside a title (the "#" in `#?` is optional), so the combined form buys nothing there + # either. To genuinely match more, widen the TEXT searched -- title -> title+body, as + # durability_sweep does. The selftest pins both halves of this. for issue in issues or []: issue_number = issue.get("number") issue_title = str(issue.get("title") or "") - haystack = f"#{issue_number} {issue_title}" if _contains_pr_ref(issue_title, pr_number) or _contains_pr_ref( candidate["seed"], issue_number ): @@ -634,6 +643,42 @@ def linked_issues(repo: str, pr: int) -> list[dict]: assert clean["candidates"] == [], clean assert clean["process_signals"] == [], clean assert clean["signals"]["durable_rate"] == 1.0, clean["signals"] + + # _dedupe_candidate deliberately does NOT search a combined "#{issue_number} {title}". + # Two assertions pin that decision so a later lint or tidy pass cannot quietly wire it in. + # 1. The isolated case the combined form -- and only the combined form -- would match: an + # issue whose own number equals the candidate PR's, with an unrelated title and a seed + # that does not name the number. It must NOT be called a duplicate. + collision = { + "type": "reversal", + "severity": "HIGH", + "seed": "A merged change was REVERTED; open a durable-fix + regression-test issue.", + "evidence": {"run_id": "x", "pr": 4242, "agent": "codex", "durability": "reverted"}, + "possible_duplicate": False, + "dup_issue": None, + } + _dedupe_candidate( + collision, + "o/r", + issue_search_fn=lambda _repo, _query: [ + {"number": 4242, "title": "unrelated maintenance chore"} + ], + linked_issues_fn=lambda _repo, _pr: [], + ) + assert collision["possible_duplicate"] is False, collision + assert collision["dup_issue"] is None, collision + # 2. ...and that stays a no-op in production only because every seed evidence_for_repo + # builds opens with "PR #", so the seed disjunct already covers issue_number == + # pr_number. If a seed ever stops naming its PR, the decision needs re-examining. + seeded = [ + candidate + for candidate in result["candidates"] + if (candidate.get("evidence") or {}).get("pr") is not None + ] + assert len(seeded) == 2, seeded + for candidate in seeded: + assert _contains_pr_ref(candidate["seed"], candidate["evidence"]["pr"]), candidate + print("keepalive_evidence.py selftest: OK") finally: feedback.DB_PATH = old_db From b29de064cac6c3afe0c73984eb9df61bf35e21bf Mon Sep 17 00:00:00 2001 From: Tim Stranske Date: Sun, 23 Aug 2026 08:52:34 -0500 Subject: [PATCH 2/2] chore(lint): take PR #58's other F841 in this file, so its diff here is redundant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #58's entire change to keepalive_evidence.py is two deletions: the dead `haystack` binding (decided in the previous commit) and this unused `repo = target.split("#", 1)[0]` in the selftest loop — `record_run` is passed `target`, never `repo`. Taking both makes this branch's copy of the file a superset of #58's, and since both are now identical deletions on both sides, the file merges with zero conflicts in either order — verified with a 3-way merge against origin/claude/ci-conform-format-lint. `ruff check --select E4,E7,E9,F` is clean for this file and `black --line-length 100 --check` leaves it unchanged. #58 is separately CONFLICTING against main (it predates #59 and is a tree-wide black pass), so it needs a rebase regardless — that is not caused by this branch. Co-Authored-By: Claude Opus 5 --- keepalive_evidence.py | 1 - 1 file changed, 1 deletion(-) diff --git a/keepalive_evidence.py b/keepalive_evidence.py index 203fa65..fbaec59 100644 --- a/keepalive_evidence.py +++ b/keepalive_evidence.py @@ -526,7 +526,6 @@ def _selftest() -> None: ] rows = [row if len(row) == 7 else (*row, None) for row in rows] for run_id, target, agent, pr_number, durability, work_type, notes in rows: - repo = target.split("#", 1)[0] feedback.record_run( run_id, target,