From a3bc4410273c339ac3818037eadcdd8494c9561d Mon Sep 17 00:00:00 2001 From: jsboige Date: Mon, 28 Sep 2026 06:07:04 +0200 Subject: [PATCH] fix(harness,#18102): split the in_flight reason -- declared open PR vs comment-only cross-ref classify partitioned open PRs into one undifferentiated reason; a census comment on an unrelated open PR (permanent cross-referenced event, e.g. #17434 freezing #15173/#15703) read as "work in progress" where it was a contextual mention. The reason now names both origins separately: body/title mention = "declare this issue (plausible next phase)"; otherwise = "comments only (cross-referenced contextual mention, undeclared)". Verdict unchanged in both cases (reason change first per acceptance 1); unreadable body defaults to undeclared (fail-safe, #15060 philosophy). Driver enriches open PR bodies+titles too (_with_merged_pr_bodies -> _with_pr_bodies; closed-unmerged PRs stay unfetched). Title stays CLOSED for every labelling decision (#17759 arbitration); delivered not widened. Firsthand via the real chain: #18146 <- open #18181 renders in_flight "declare this issue in body/title (plausible next phase)"; the issue's named instances #15173/#15703 no longer reproduce as in_flight because #17434 merged since (measured 2026-09-28). 50 tests (44 + 6 new). Closes #18102 Co-Authored-By: Claude Sonnet 5 --- scripts/candidate_delivered.py | 66 +++++++++--- scripts/tests/test_candidate_delivered.py | 122 ++++++++++++++++++++++ 2 files changed, 175 insertions(+), 13 deletions(-) diff --git a/scripts/candidate_delivered.py b/scripts/candidate_delivered.py index 85cebe14c2..248e7d8cc4 100644 --- a/scripts/candidate_delivered.py +++ b/scripts/candidate_delivered.py @@ -51,6 +51,12 @@ must not produce the "probably delivered" label. Measured on #10984 (open #10986 + six merged PRs = a multi-phase rollout the old heuristic mislabeled). An open *issue* mention does NOT trigger this: only PR refs. + The reason SPLITS the two origins of the cross-referenced event (#18102): + a PR whose body/title declares the issue ("plausible next phase") vs a + comment-only mention on a PR that never names it (contextual, undeclared + -- e.g. a census comment on another family's PR, permanent while that PR + stays open, measured freezing #15173/#15703 out of the label forever). + Verdict unchanged in both cases; the title stays closed for labelling. - A merged PR counts ONLY if it carries a **declared delivery marker** (`See #N` / `Part of #N` / `Closes #N` / `Fixes #N` / `Refs #N`) in its CURRENT body (#15060, measured 2026-09-10). The timeline's @@ -267,10 +273,40 @@ def classify( # multi-phase rollout shape (e.g. #10984, referenced by open #10986 plus # six merged PRs). Partial deliveries write `See #N` correctly, so the # "merged + silent" heuristic alone mislabels the rollout as delivered. + # #18102: the REASON now separates the two ways a cross-referenced event + # comes to exist on an open PR. A PR whose body/title mentions the issue + # plausibly carries its next phase; a PR whose body/title never mention it + # was cross-referenced by a COMMENT alone (e.g. a census comment listing + # issues from other families -- permanent while the PR stays open) and the + # mention is contextual. The verdict stays in_flight in both cases (change + # of reason first, verdict unchanged); an unreadable body defaults to + # "undeclared", fail-safe like #15060. Reading the PR title here informs + # the reason of an already-decided verdict -- it is NOT the #17759 title + # channel, which stays closed for every labelling decision. open_prs = [r for r in cross_refs if r.get("is_pr") and r.get("state") == "open"] if open_prs: - prs = ", ".join(f"#{r['pr_number']}" for r in open_prs) - return ("in_flight", f"open PR(s) {prs} reference this issue") + target_n = issue.get("number") + anchor = re.compile(rf"#{target_n}\b") if target_n else None + + def _declared_open(r: dict) -> bool: + if anchor is None: + return False + return bool(anchor.search(r.get("body") or "") + or anchor.search(r.get("title") or "")) + + declared_open, comment_only = [], [] + for r in open_prs: + (declared_open if _declared_open(r) else comment_only).append(r) + parts = [] + if declared_open: + prs = ", ".join(f"#{r['pr_number']}" for r in declared_open) + parts.append(f"open PR(s) {prs} declare this issue in body/title " + f"(plausible next phase)") + if comment_only: + prs = ", ".join(f"#{r['pr_number']}" for r in comment_only) + parts.append(f"open PR(s) {prs} reference it via comments only " + f"(cross-referenced contextual mention, undeclared)") + return ("in_flight", "; ".join(parts)) merged = [r for r in cross_refs if r.get("merged_at")] if not merged: @@ -389,8 +425,8 @@ def _parse_cross_ref_events(events: list[dict]) -> list[dict]: Each ref carries ``"body": None``: the timeline payload does NOT embed the PR body (nor its edit history), and the network-free contract ends here. - The driver (:func:`_with_merged_pr_bodies`) replaces the value on the - merged refs, and :func:`classify` treats a missing body as NOT declared + The driver (:func:`_with_pr_bodies`) replaces the value on the + merged and open refs, and :func:`classify` treats a missing body as NOT declared (fail-safe, #15060). """ refs = [] @@ -432,15 +468,19 @@ def _parse_label_events(events: list[dict], label: str) -> list[dict]: return out -def _with_merged_pr_bodies( +def _with_pr_bodies( repo: str, refs: list[dict], cache: dict[int, tuple[str, str]], ) -> list[dict]: - """Attach the CURRENT body and title of each merged PR to its ref. + """Attach the CURRENT body and title of each merged OR open PR to its ref. - Body = the delivery-marker gate (#15060). Title = the ``no_delivery`` - title-only REPORT (#17759) -- never a labelling decision. The per-run - ``cache`` holds ``(body, title)`` tuples so a PR referenced by several - issues costs one REST call. + Merged PR body = the delivery-marker gate (#15060); merged PR title = the + ``no_delivery`` title-only REPORT (#17759) -- never a labelling decision. + OPEN PR body+title = the ``in_flight`` reason split (#18102): whether the + PR itself declares the issue or the cross-referenced event was posed by a + comment alone. Closed-unmerged PRs (abandoned lanes) are not fetched -- + they drive no verdict branch. The per-run ``cache`` holds + ``(body, title)`` tuples so a PR referenced by several issues costs one + REST call. A cross-referenced event is posed when the body FIRST mentions the issue and is never retracted when the mention later disappears -- #15149's @@ -451,12 +491,12 @@ def _with_merged_pr_bodies( A body that cannot be fetched (PR deleted, gh hiccup) reads as ``""`` (and its title as ``""``) -- NOT declared -- which is fail-safe: the sweep is advisory, and a ref it cannot verify must not produce a - candidate label. + candidate label, nor claim "plausible next phase" in a reason. """ out = [] for r in refs: r = dict(r) - if r.get("merged_at") and r.get("is_pr"): + if r.get("is_pr") and (r.get("merged_at") or r.get("state") == "open"): pr = r["pr_number"] if pr not in cache: try: @@ -593,7 +633,7 @@ def main(argv: list[str] | None = None) -> int: print(f" #{number:<6} SKIP ({exc})") continue try: - refs = _with_merged_pr_bodies(repo, refs, body_cache) + refs = _with_pr_bodies(repo, refs, body_cache) except Exception as exc: # body fetch failure -- fail safe, do not label print(f" #{number:<6} SKIP (pr body fetch failed: {exc})") continue diff --git a/scripts/tests/test_candidate_delivered.py b/scripts/tests/test_candidate_delivered.py index 9426af91ad..515841b4e6 100644 --- a/scripts/tests/test_candidate_delivered.py +++ b/scripts/tests/test_candidate_delivered.py @@ -640,3 +640,125 @@ def test_epic_verdict_wins_over_container(): if __name__ == "__main__": import pytest sys.exit(pytest.main([__file__, "-v"])) + + +# --- #18102 : la raison du verdict in_flight separe declare vs commentaire --- + +def test_in_flight_reason_declared_body_names_next_phase(): + # Controle negatif de l'acceptance : une PR ouverte qui DECLARE l'issue + # dans son body (rollout multi-phases, #10984/#10986) rend in_flight avec + # la raison « prochaine phase plausible ». + issue = _issue(title="rollout phase 7 of N", comments=[]) + refs = [ + {"pr_number": 10995, "merged_at": "2026-08-14T23:18:16Z", "is_pr": True, + "state": "closed", "body": "See #1."}, + {"pr_number": 10986, "merged_at": None, "is_pr": True, "state": "open", + "body": "Phase 2 of the rollout: continuing #1.", "title": "rollout p2"}, + ] + verdict, why = classify(issue, refs) + assert verdict == "in_flight" + assert "#10986" in why + assert "declare this issue in body/title" in why + assert "next phase" in why + assert "comments only" not in why + + +def test_in_flight_reason_declared_title_counts(): + # Le titre porte la reference canonique `fix(#N):` quand le body est vide : + # il informe la RAISON uniquement (#18102 nomme body/titre), jamais une + # decision de label (le canal titre reste ferme, arbitrage #17759). + issue = _issue(title="x", comments=[]) + refs = [{"pr_number": 2, "merged_at": None, "is_pr": True, "state": "open", + "body": "", "title": "fix(#1): repair the gate"}] + verdict, why = classify(issue, refs) + assert verdict == "in_flight" + assert "#2" in why + assert "declare this issue in body/title" in why + + +def test_in_flight_reason_comment_only_flags_contextual(): + # Controle positif de l'acceptance : #15173 <- #17434 -- un commentaire de + # census sur une PR ouverte d'une AUTRE famille pose un cross-referenced + # permanent alors que ni son body ni son titre ne citent l'issue. La + # raison nomme la PR et son caractere non declare (le masque « travail en + # cours » leve). + issue = _issue(title="delivered but frozen", comments=[]) + refs = [ + {"pr_number": 17434, "merged_at": None, "is_pr": True, "state": "open", + "body": "GenAI Image series maintenance notes.", "title": "genai(image): batch 4"}, + {"pr_number": 15800, "merged_at": "2026-09-10T00:00:00Z", "is_pr": True, + "state": "closed", "body": "See #1."}, + ] + verdict, why = classify(issue, refs) + assert verdict == "in_flight" + assert "#17434" in why + assert "comments only" in why + assert "undeclared" in why + assert "next phase" not in why + + +def test_in_flight_reason_mixed_open_prs_lists_both(): + # Deux PRs ouvertes, une declarante et une contextuelle : la raison cite + # les deux origines, chacune avec son caractere. + issue = _issue(title="x", comments=[]) + refs = [ + {"pr_number": 10, "merged_at": None, "is_pr": True, "state": "open", + "body": "Next: rework #1 benchmarks.", "title": "bench"}, + {"pr_number": 20, "merged_at": None, "is_pr": True, "state": "open", + "body": "Unrelated family.", "title": "other"}, + ] + verdict, why = classify(issue, refs) + assert verdict == "in_flight" + assert "#10" in why and "#20" in why + assert "declare this issue in body/title" in why + assert "comments only" in why + + +def test_in_flight_reason_unreadable_body_defaults_undeclared(): + # Body non transporté (None, shape _parse_cross_ref_events) : unverifiable + # => conservateur « undeclared », jamais « prochaine phase plausible » + # (fail-safe coherent #15060). + issue = _issue(title="x", comments=[]) + refs = [{"pr_number": 3, "merged_at": None, "is_pr": True, "state": "open"}] + verdict, why = classify(issue, refs) + assert verdict == "in_flight" + assert "comments only" in why and "undeclared" in why + + +def test_with_pr_bodies_fetches_open_prs_too(monkeypatch): + # Wiring #18102 : le driver attache body+title des PRs OUVERTES (raison + # in_flight), pas seulement des mergees (gate #15060). La PR fermee + # non-margee (voie abandonnee) n'est PAS consultee. + import candidate_delivered as cd + + fetched = [] + + def fake_gh_json(args): + assert args[0] == "pr" and args[-1] == "body,title" + fetched.append(int(args[2])) + bodies = {10: ("Phase 2: continuing #1.", "rollout p2"), + 20: ("GenAI census notes.", "genai(image): batch"), + 30: ("See #1.", "delivery")} + return dict(zip(("body", "title"), bodies[int(args[2])])) + + monkeypatch.setattr(cd, "_gh_json", fake_gh_json) + refs = [ + {"pr_number": 10, "merged_at": None, "is_pr": True, "state": "open", "body": None}, + {"pr_number": 20, "merged_at": None, "is_pr": True, "state": "open", "body": None}, + {"pr_number": 30, "merged_at": "2026-09-01T00:00:00Z", "is_pr": True, + "state": "closed", "body": None}, + {"pr_number": 40, "merged_at": None, "is_pr": True, "state": "closed", "body": None}, + ] + out = cd._with_pr_bodies("jsboige/CoursIA", refs, {}) + assert sorted(fetched) == [10, 20, 30] # 40 (fermee non-margee) epargnee + by_pr = {r["pr_number"]: r for r in out} + assert by_pr[10]["body"] == "Phase 2: continuing #1." + assert by_pr[20]["title"] == "genai(image): batch" + assert by_pr[40].get("body") is None # non enrichie + + # Et la raison suit : 10 declarante, 20 contextuelle. + issue = _issue(title="x", number=1, comments=[]) + verdict, why = cd.classify(issue, out) + assert verdict == "in_flight" + assert "#10" in why and "declare this issue in body/title" in why + assert "#20" in why and "comments only" in why