From 0cf1284442a04a4ed699e5cd31a6062e07dc2080 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Mon, 5 Oct 2026 15:30:57 -0300 Subject: [PATCH 01/22] feat(engine): flag repos with a degraded PR read in the org outputs A repository whose PR read failed showed 0 PRs in the org report and in org-metrics.json with no mark, while its own report carried the caveat. The org report now names those repositories next to the non-code note, and each org-metrics.json repo entry carries pr_enrichment_degraded when its read degraded. Clean runs produce the same outputs as before. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Sonnet 5.5 --- iris/i18n.py | 9 +++ iris/reports/org_writer.py | 12 ++++ tests/test_org_pr_read_degraded.py | 107 +++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+) create mode 100644 tests/test_org_pr_read_degraded.py diff --git a/iris/i18n.py b/iris/i18n.py index 4522bb8..6cb08c9 100644 --- a/iris/i18n.py +++ b/iris/i18n.py @@ -527,6 +527,10 @@ "project manifest, so their metrics describe documentation rather than " "delivery: {repos}." ), + "org_pr_read_degraded": ( + "PR data is incomplete this run for {count} repositories ({repos}): " + "their PR counts and PR-derived metrics below may be missing or skewed." + ), "org_ai_impact_title": "AI Impact Across Organization", "org_ai_impact_body": ( "Of {total_repos} repositories, {ai_repos} contain AI-assisted commits. " @@ -1417,6 +1421,11 @@ "de projeto, então suas métricas descrevem documentação e não " "entrega: {repos}." ), + "org_pr_read_degraded": ( + "Os dados de PR estão incompletos nesta execução para {count} " + "repositórios ({repos}): as contagens de PR e as métricas derivadas " + "de PR abaixo podem faltar ou sair distorcidas." + ), "org_ai_impact_title": "Impacto de IA na Organização", "org_ai_impact_body": ( "De {total_repos} repositórios, {ai_repos} contêm commits assistidos por IA. " diff --git a/iris/reports/org_writer.py b/iris/reports/org_writer.py index b66eac0..abc7b9c 100644 --- a/iris/reports/org_writer.py +++ b/iris/reports/org_writer.py @@ -117,6 +117,16 @@ def write_org_report( ) lines.extend([f"> {excluded}", ""]) + # A repo whose PR read degraded shows 0 PRs (or partial PR data) that reads + # as "no PRs"; name it so the PR counts below are not taken at face value. + pr_degraded_repos = [r for r in org_result.repos if r.metrics.pr_enrichment_degraded] + if pr_degraded_repos: + degraded_note = s["org_pr_read_degraded"].format( + count=len(pr_degraded_repos), + repos=", ".join(sorted(r.repo_name for r in pr_degraded_repos)), + ) + lines.extend([f"> {degraded_note}", ""]) + # AI Impact Across Organization (conditional — only when AI commits exist) ai_repos = [ r for r in code_repos @@ -323,6 +333,8 @@ def write_org_metrics( } if r.metrics.repo_kind is not None: entry["repo_kind"] = r.metrics.repo_kind + if r.metrics.pr_enrichment_degraded: + entry["pr_enrichment_degraded"] = r.metrics.pr_enrichment_degraded if r.trend and r.trend.has_sufficient_data: entry["attention_signal"] = _attention_label(r, lang=lang) repo_entries.append(entry) diff --git a/tests/test_org_pr_read_degraded.py b/tests/test_org_pr_read_degraded.py new file mode 100644 index 0000000..80cd488 --- /dev/null +++ b/tests/test_org_pr_read_degraded.py @@ -0,0 +1,107 @@ +"""Tests that the org outputs flag repositories whose PR read degraded. + +A repository whose PR read failed shows 0 PRs in the org report; without a +mark that reads as "no PRs" rather than "PR data missing". + +Runnable as a plain script: `python tests/test_org_pr_read_degraded.py`. +No external test framework required. +""" + +import json +import sys +import tempfile +from pathlib import Path + +# Allow running from repo root without installation. +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from iris.models.metrics import ReportMetrics +from iris.models.org import OrgResult, RepoResult +from iris.reports.org_writer import write_org_metrics, write_org_report + +_EN_NOTE = "PR data is incomplete this run for" +_PT_NOTE = "Os dados de PR estão incompletos nesta execução para" + + +def _metrics(pr_enrichment_degraded: list[str] | None = None) -> ReportMetrics: + return ReportMetrics( + commits_total=100, + commits_revert=10, + revert_rate=0.1, + churn_events=5, + churn_lines_affected=500, + files_touched=50, + files_stabilized=30, + stabilization_ratio=0.6, + repo_kind="CODE", + pr_enrichment_degraded=pr_enrichment_degraded, + ) + + +def _org(*repos: RepoResult) -> OrgResult: + return OrgResult( + org_name="acme", + repos=list(repos), + change_attribution="", + attention_signals=[], + delivery_narrative="", + ) + + +_DEGRADED = RepoResult("widgets", _metrics(["basic"]), trend=None) +_CLEAN = RepoResult("gadgets", _metrics(), trend=None) + + +def _report_text(org: OrgResult, out_dir: Path, lang: str = "en") -> str: + path = write_org_report( + org, out_dir=str(out_dir), days=90, recent_days=30, lang=lang + ) + return Path(path).read_text() + + +def _metrics_json(org: OrgResult, out_dir: Path) -> dict: + path = write_org_metrics(org, out_dir=str(out_dir), days=90) + return json.loads(Path(path).read_text()) + + +def test_org_report_names_repos_with_degraded_pr_read(tmp_path: Path) -> None: + text = _report_text(_org(_DEGRADED, _CLEAN), tmp_path) + assert _EN_NOTE in text + note = text.split(_EN_NOTE)[1].split("\n")[0] + assert "1 repositories (widgets)" in note + assert "gadgets" not in note + + +def test_org_report_has_no_pr_read_note_when_clean(tmp_path: Path) -> None: + text = _report_text(_org(_CLEAN), tmp_path) + assert _EN_NOTE not in text + + +def test_org_metrics_carries_the_degradation_per_repo(tmp_path: Path) -> None: + data = _metrics_json(_org(_DEGRADED, _CLEAN), tmp_path) + entries = {entry["name"]: entry for entry in data["repos"]} + assert entries["widgets"]["pr_enrichment_degraded"] == ["basic"] + assert "pr_enrichment_degraded" not in entries["gadgets"] + + +def test_org_pr_read_note_is_translated(tmp_path: Path) -> None: + text = _report_text(_org(_DEGRADED, _CLEAN), tmp_path, lang="pt-br") + assert _PT_NOTE in text + assert _EN_NOTE not in text + + +if __name__ == "__main__": + tests = [fn for name, fn in globals().items() if name.startswith("test_")] + failed = 0 + for fn in tests: + with tempfile.TemporaryDirectory() as tmp: + try: + fn(Path(tmp)) + print(f"ok {fn.__name__}") + except AssertionError as exc: + failed += 1 + print(f"FAIL {fn.__name__}: {exc}") + if failed: + print(f"\n{failed} failure(s)") + sys.exit(1) + print(f"\n{len(tests)} tests passed") From 3969f6e7c079bdf8f06018578910bb4851cf3c87 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Mon, 5 Oct 2026 15:54:48 -0300 Subject: [PATCH 02/22] fix(engine): make the org PR-read note plural-safe "1 repositories" read wrongly for a single repository; the note now says "repository(ies)" like the sibling non-code note. "below" becomes "in this report", since the trend sections above the note can also carry PR deltas. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Sonnet 5.5 --- iris/i18n.py | 8 ++++---- tests/test_org_pr_read_degraded.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/iris/i18n.py b/iris/i18n.py index 6cb08c9..433d31b 100644 --- a/iris/i18n.py +++ b/iris/i18n.py @@ -528,8 +528,8 @@ "delivery: {repos}." ), "org_pr_read_degraded": ( - "PR data is incomplete this run for {count} repositories ({repos}): " - "their PR counts and PR-derived metrics below may be missing or skewed." + "PR data is incomplete this run for {count} repository(ies) ({repos}): " + "their PR counts and PR-derived metrics in this report may be missing or skewed." ), "org_ai_impact_title": "AI Impact Across Organization", "org_ai_impact_body": ( @@ -1423,8 +1423,8 @@ ), "org_pr_read_degraded": ( "Os dados de PR estão incompletos nesta execução para {count} " - "repositórios ({repos}): as contagens de PR e as métricas derivadas " - "de PR abaixo podem faltar ou sair distorcidas." + "repositório(s) ({repos}): as contagens de PR e as métricas derivadas " + "de PR neste relatório podem faltar ou sair distorcidas." ), "org_ai_impact_title": "Impacto de IA na Organização", "org_ai_impact_body": ( diff --git a/tests/test_org_pr_read_degraded.py b/tests/test_org_pr_read_degraded.py index 80cd488..a7e216a 100644 --- a/tests/test_org_pr_read_degraded.py +++ b/tests/test_org_pr_read_degraded.py @@ -68,7 +68,7 @@ def test_org_report_names_repos_with_degraded_pr_read(tmp_path: Path) -> None: text = _report_text(_org(_DEGRADED, _CLEAN), tmp_path) assert _EN_NOTE in text note = text.split(_EN_NOTE)[1].split("\n")[0] - assert "1 repositories (widgets)" in note + assert "1 repository(ies) (widgets)" in note assert "gadgets" not in note From cad1c6ef60ead5da09fd50b83f221840ea98eb7c Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 20:01:28 -0300 Subject: [PATCH 03/22] fix(engine): omit acceptance and staleness when enrichment failed Acceptance matches commits to PRs through commit_refs, and open-PR staleness reads the last commit push from them. Both come from the enrichment pass. With it failed a real run reported 0 commits in PRs against 30 clean, and underestimated last activity, inflating stalled PRs. Omit acceptance_by_origin/by_tool whole and the four staleness fields; open-PR count and ages stay. Documented in METRICS.md. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Sonnet 5.5 --- docs/METRICS.md | 26 ++++++++++++++---------- iris/metrics/aggregator.py | 18 ++++++++++++----- tests/test_pr_fetch_degradation.py | 32 ++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 15 deletions(-) diff --git a/docs/METRICS.md b/docs/METRICS.md index cf8cda9..2f39105 100644 --- a/docs/METRICS.md +++ b/docs/METRICS.md @@ -236,13 +236,15 @@ its PR (if any) and aggregated by origin and AI tool. | Field | Unit | Source | Nullable when | |---|---|---|---| -| `acceptance_by_origin` | `Record` | `analysis/acceptance_rate.py` | no PR data, or < 5 commits per origin | -| `acceptance_by_tool` | `Record` | same | < 5 commits per tool | +| `acceptance_by_origin` | `Record` | `analysis/acceptance_rate.py` | no PR data, or < 5 commits per origin, or `enrichment` degraded | +| `acceptance_by_tool` | `Record` | same | < 5 commits per tool, or `enrichment` degraded | `AcceptanceMetrics`: `total_commits`, `commits_in_prs`, `pr_rate` (commits in PRs / total), `single_pass_rate` (PRs merged with zero `CHANGES_REQUESTED` / PRs), `median_review_rounds`. The last two are omitted when `reviews` is in -`pr_enrichment_degraded` (§15). +`pr_enrichment_degraded` (§15). Both maps are omitted entirely when `enrichment` +is in `pr_enrichment_degraded`: commits are matched to PRs through the commit +refs that pass reads, so without them every group would read 0 commits in PRs. --- @@ -361,8 +363,8 @@ All fields require GitHub PR data. | `pr_cycle_time_buckets` | `{same_day, one_day, two_to_three_days, four_to_seven_days, seven_plus_days}` ints | same | same | | `pr_median_size_files` | int ≥ 0 | same | same | | `pr_median_size_lines` | int ≥ 0 | same | same | -| `pr_review_rounds_median` | float ≥ 0 | same | same, or `reviews` degraded | -| `pr_single_pass_rate` | float `0.0–1.0` | same | same, or `reviews` degraded | +| `pr_review_rounds_median` | float ≥ 0 | same | same, or `reviews` or `enrichment` degraded | +| `pr_single_pass_rate` | float `0.0–1.0` | same | same, or `reviews` or `enrichment` degraded | | `pr_enrichment_degraded` | list of `basic\|enrichment\|reviews\|fetch` | `ingestion/github_reader.py` | every read step succeeded, or PRs are absent by design | `pr_review_rounds_median` — median count of `CHANGES_REQUESTED` reviews @@ -398,7 +400,11 @@ coverage and 100% single-pass an empty review list would give: The review-anchored flow and staleness fields go too: the whole Flow Efficiency block (§25) would put every PR's open-to-merge window into "awaiting first review", and staleness is measured from the last review or -commit. Open PR count and ages (`open_pr_count`, `median_open_pr_age_days`, +commit. With `enrichment` failed the commit pushes are missing, so the last +activity would be underestimated and the stalled share inflated: the staleness +fields are omitted then too, and `acceptance_by_origin` and `acceptance_by_tool` +(§9) are omitted whole, since they match commits to PRs through those same +refs. Open PR count and ages (`open_pr_count`, `median_open_pr_age_days`, `p90_open_pr_age_days`, `median_open_pr_age_by_intent`) are kept, since age depends only on the creation time. It is the first field to read when two runs on the same commit disagree. @@ -816,11 +822,11 @@ Aging = "do trabalho que ainda não saiu, está represado?". | `open_pr_count` | int | `analysis/open_pr_aging.py` | no eligible open PR (all drafts/bots) | | `median_open_pr_age_days` | float ≥ 0 | same | same | | `p90_open_pr_age_days` | float ≥ 0 | same | same | -| `stale_open_pr_pct` | float `0.0–1.0` | same | same, or `reviews` degraded | -| `very_stale_open_pr_pct` | float `0.0–1.0` | same | same, or `reviews` degraded | -| `abandonment_risk_pct` | float `0.0–1.0` | same | same, or `reviews` degraded | +| `stale_open_pr_pct` | float `0.0–1.0` | same | same, or `reviews` or `enrichment` degraded | +| `very_stale_open_pr_pct` | float `0.0–1.0` | same | same, or `reviews` or `enrichment` degraded | +| `abandonment_risk_pct` | float `0.0–1.0` | same | same, or `reviews` or `enrichment` degraded | | `median_open_pr_age_by_intent` | `Record` | same | < `min_sample` (default 5) PRs in segment | -| `stale_open_pr_pct_by_origin` | `Record` | same | no `commit_origin_map`, or < `min_sample` PRs in segment, or `reviews` degraded | +| `stale_open_pr_pct_by_origin` | `Record` | same | no `commit_origin_map`, or < `min_sample` PRs in segment, or `reviews` or `enrichment` degraded | Per-PR signals (intermediate, **never persisted**): diff --git a/iris/metrics/aggregator.py b/iris/metrics/aggregator.py index 6b615c5..286f39e 100644 --- a/iris/metrics/aggregator.py +++ b/iris/metrics/aggregator.py @@ -79,6 +79,10 @@ def aggregate( # With the reviews pass failed every PR has no reviews: review-derived # fields would read as fabricated facts, so they are left out. reviews_degraded = DEGRADED_REVIEWS in pr_fetch_degraded + # Acceptance and open-PR staleness read the commit pushes (commit_refs) + # that come from the enrichment pass; without them acceptance would read + # 0 commits in PRs and staleness would be inflated. + enrichment_degraded = DEGRADED_ENRICHMENT in pr_fetch_degraded churn_result = calculate_churn(commits, churn_days) stab_result = calculate_stabilization(commits, churn_days) @@ -282,7 +286,7 @@ def aggregate( # Acceptance rate (requires PR data with commit hashes) acceptance_kwargs: dict = {} - if prs: + if prs and not enrichment_degraded: acceptance_result = calculate_acceptance_rate(commits, prs) if acceptance_result: if acceptance_result.by_origin: @@ -392,8 +396,8 @@ def aggregate( aging_result.median_open_pr_age_by_intent ) # Staleness is measured from the last review or commit; without - # the reviews it would read older than it is. - if not reviews_degraded: + # the reviews or the commit pushes it would read older than it is. + if not reviews_degraded and not enrichment_degraded: open_pr_aging_kwargs["stale_open_pr_pct"] = ( aging_result.stale_open_pr_pct ) @@ -403,7 +407,11 @@ def aggregate( open_pr_aging_kwargs["abandonment_risk_pct"] = ( aging_result.abandonment_risk_pct ) - if not reviews_degraded and aging_result.stale_open_pr_pct_by_origin: + if ( + not reviews_degraded + and not enrichment_degraded + and aging_result.stale_open_pr_pct_by_origin + ): open_pr_aging_kwargs["stale_open_pr_pct_by_origin"] = ( aging_result.stale_open_pr_pct_by_origin ) @@ -417,7 +425,7 @@ def aggregate( merge_strategy_result = detect_merge_strategy( prs, commits, - enrichment_degraded=DEGRADED_ENRICHMENT in pr_fetch_degraded, + enrichment_degraded=enrichment_degraded, ) merge_strategy_kwargs["merge_strategy"] = merge_strategy_result.merge_strategy merge_strategy_kwargs["commit_metrics_reliable"] = ( diff --git a/tests/test_pr_fetch_degradation.py b/tests/test_pr_fetch_degradation.py index 23cccca..6d55f08 100644 --- a/tests/test_pr_fetch_degradation.py +++ b/tests/test_pr_fetch_degradation.py @@ -643,3 +643,35 @@ def metrics(single_pass): names = [d.metric for d in trend.deltas] assert "pr_time_to_merge" in names assert "pr_single_pass" not in names + + +# --- a failed enrichment omits what depends on commit_refs ------------------ +# +# Acceptance matches commits to PRs through commit_refs, and open-PR +# staleness reads the last commit push from them. Without the enrichment the +# first reads 0 commits in PRs and the second counts too many PRs as stalled. + + +def test_degraded_enrichment_omits_acceptance_and_staleness(): + payload = aggregate( + _stamped_commits(), churn_days=14, prs=_reviewed_prs() + _open_prs(), + pr_fetch_degraded=("enrichment",), + ).to_dict() + + assert "acceptance_by_origin" not in payload + assert "acceptance_by_tool" not in payload + for key in _STALENESS_KEYS: + assert key not in payload, key + for key in _AGE_KEYS: + assert key in payload, key + + +def test_clean_run_keeps_acceptance_and_staleness(): + payload = aggregate( + _stamped_commits(), churn_days=14, prs=_reviewed_prs() + _open_prs(), + ).to_dict() + + assert payload["acceptance_by_origin"] + for key in _STALENESS_KEYS: + if key != "stale_open_pr_pct_by_origin": + assert key in payload, key From 3c21f069f4c31b307fe43d8e2d6d4aa495f3a290 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 20:01:40 -0300 Subject: [PATCH 04/22] fix(engine): treat null GraphQL fields as an incomplete enrichment A null data.repository, or a node with commits: null, raised AttributeError, which dropped the whole PR read to the fetch step. A non-dict at those points is now a missing page (read incomplete, what was already read is kept) or a node with no commits. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Sonnet 5.5 --- iris/ingestion/github_reader.py | 17 +++++++++-------- tests/test_pr_fetch_degradation.py | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/iris/ingestion/github_reader.py b/iris/ingestion/github_reader.py index e3d6df6..5fb27cf 100644 --- a/iris/ingestion/github_reader.py +++ b/iris/ingestion/github_reader.py @@ -285,13 +285,12 @@ def _fetch_pr_enrichment_graphql( except json.JSONDecodeError: return by_pr, False - page = ( - data.get("data", {}) - .get("repository", {}) - .get("pullRequests") - if data.get("data") else None - ) - if not page: + # Any non-dict on the way down (null repository, null data) is a + # missing page, not a reason to lose the whole PR read. + payload = data.get("data") if isinstance(data, dict) else None + repository = payload.get("repository") if isinstance(payload, dict) else None + page = repository.get("pullRequests") if isinstance(repository, dict) else None + if not page or not isinstance(page, dict): return by_pr, False for node in page.get("nodes", []): @@ -299,7 +298,9 @@ def _fetch_pr_enrichment_graphql( if number is None: continue commits = [] - for entry in node.get("commits", {}).get("nodes", []): + commits_conn = node.get("commits") + entries = commits_conn.get("nodes") if isinstance(commits_conn, dict) else None + for entry in entries or []: commit = entry.get("commit") or {} oid = commit.get("oid", "") if not oid: diff --git a/tests/test_pr_fetch_degradation.py b/tests/test_pr_fetch_degradation.py index 6d55f08..016b9e8 100644 --- a/tests/test_pr_fetch_degradation.py +++ b/tests/test_pr_fetch_degradation.py @@ -675,3 +675,27 @@ def test_clean_run_keeps_acceptance_and_staleness(): for key in _STALENESS_KEYS: if key != "stale_open_pr_pct_by_origin": assert key in payload, key + + +# --- null GraphQL fields are an incomplete read, not a crash ---------------- + + +def test_enrichment_with_null_repository_is_incomplete(monkeypatch): + _fake_gh(monkeypatch, graphql=[{"data": {"repository": None}}]) + + by_pr, complete = github_reader._fetch_pr_enrichment_graphql("acme/widgets", "merged", 500) + + assert by_pr == {} + assert complete is False + + +def test_enrichment_node_with_null_commits_has_no_commits(monkeypatch): + page = _graphql_page([1, 2]) + page["data"]["repository"]["pullRequests"]["nodes"][0]["commits"] = None + _fake_gh(monkeypatch, graphql=[page]) + + by_pr, complete = github_reader._fetch_pr_enrichment_graphql("acme/widgets", "merged", 500) + + assert by_pr[1]["commits"] == [] + assert len(by_pr[2]["commits"]) == 1 + assert complete is True From 2e7c67477b20039591327e2000c8b78de85fe600 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 20:01:46 -0300 Subject: [PATCH 05/22] fix(engine): pass the GraphQL cursor as a raw field gh api -F reads a file when the value starts with "@", so a cursor beginning with that character would be misread. -f sends plain text. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Sonnet 5.5 --- iris/ingestion/github_reader.py | 2 +- tests/test_pr_fetch_degradation.py | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/iris/ingestion/github_reader.py b/iris/ingestion/github_reader.py index 5fb27cf..f33eaf0 100644 --- a/iris/ingestion/github_reader.py +++ b/iris/ingestion/github_reader.py @@ -274,7 +274,7 @@ def _fetch_pr_enrichment_graphql( "-f", f"states[]={graphql_state}", ] if end_cursor: - args.extend(["-F", f"cursor={end_cursor}"]) + args.extend(["-f", f"cursor={end_cursor}"]) result = _run_gh_with_one_retry(args) if result is None: diff --git a/tests/test_pr_fetch_degradation.py b/tests/test_pr_fetch_degradation.py index 016b9e8..3b6798b 100644 --- a/tests/test_pr_fetch_degradation.py +++ b/tests/test_pr_fetch_degradation.py @@ -699,3 +699,26 @@ def test_enrichment_node_with_null_commits_has_no_commits(monkeypatch): assert by_pr[1]["commits"] == [] assert len(by_pr[2]["commits"]) == 1 assert complete is True + + +# --- the cursor is sent as a raw field -------------------------------------- + + +def test_cursor_is_passed_as_a_raw_field(monkeypatch): + seen: list[list[str]] = [] + answers = [ + _graphql_page([1], has_next=True, cursor="@page2"), + _graphql_page([2]), + ] + + def fake_run(cmd, **kwargs): + seen.append(cmd) + return _ok(answers.pop(0)) + + monkeypatch.setattr(github_reader.subprocess, "run", fake_run) + + github_reader._fetch_pr_enrichment_graphql("acme/widgets", "merged", 500) + + # `-F` would read a file named page2 for a value starting with "@". + assert "cursor=@page2" in seen[1] + assert seen[1][seen[1].index("cursor=@page2") - 1] == "-f" From e757dbabbe3d5e86015c95d86f6ccbf43b7ffb24 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 20:01:59 -0300 Subject: [PATCH 06/22] fix(engine): say when the PR read was incomplete on the console The console said "failed ... continuing without PR data" when only a secondary pass failed on a window with no PRs, and said nothing when PRs were found but the read had degraded. Now PRs plus degradation print the found count with the incomplete steps; no PRs with only secondary steps degraded say the read was incomplete; basic or fetch degraded keep the failure message. Clean runs print as before. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Sonnet 5.5 --- iris/cli.py | 10 +++++-- iris/i18n.py | 4 +++ tests/test_pr_fetch_degradation.py | 42 ++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/iris/cli.py b/iris/cli.py index 755eb41..9014aad 100644 --- a/iris/cli.py +++ b/iris/cli.py @@ -16,6 +16,7 @@ from iris.ingestion.github_reader import read_pull_requests_with_fallback from iris.metrics.aggregator import aggregate from iris.models.context import AnalysisContext +from iris.models.pull_request import DEGRADED_BASIC, DEGRADED_FETCH from iris.reports.narrative import generate_narrative from iris.reports.writer import write_output @@ -498,10 +499,15 @@ def _tick(label: str) -> None: with span("ingestion.pull_requests", {"repo": repo_name}): pr_fetch = read_pull_requests_with_fallback(repo, days=args.days) prs = pr_fetch.prs - if prs: + steps = ", ".join(pr_fetch.degraded) + if prs and pr_fetch.degraded: + print(s["cli_prs_found_degraded"].format(count=len(prs), steps=steps)) + elif prs: print(s["cli_prs_found"].format(count=len(prs))) + elif DEGRADED_BASIC in pr_fetch.degraded or DEGRADED_FETCH in pr_fetch.degraded: + print(s["cli_prs_failed"].format(steps=steps)) elif pr_fetch.degraded: - print(s["cli_prs_failed"].format(steps=", ".join(pr_fetch.degraded))) + print(s["cli_prs_none_degraded"].format(steps=steps)) else: print(s["cli_prs_skipped"]) _tick("pull requests fetched") diff --git a/iris/i18n.py b/iris/i18n.py index 433d31b..a26ba53 100644 --- a/iris/i18n.py +++ b/iris/i18n.py @@ -341,6 +341,8 @@ "cli_prs_found": "{count} merged PRs found.", "cli_prs_skipped": "skipped (no GitHub remote or gh CLI).", "cli_prs_failed": "failed ({steps}) — continuing without PR data.", + "cli_prs_found_degraded": "{count} merged PRs found — PR read incomplete ({steps}).", + "cli_prs_none_degraded": "no PRs in the window — PR read incomplete ({steps}).", "cli_classifying": "Classifying commits...", "cli_classified": "{count} commits classified.", "cli_non_code_repo": ( @@ -1234,6 +1236,8 @@ "cli_prs_found": "{count} PRs merged encontrados.", "cli_prs_skipped": "pulado (sem remote GitHub ou gh CLI).", "cli_prs_failed": "falhou ({steps}) — seguindo sem dados de PR.", + "cli_prs_found_degraded": "{count} PRs merged encontrados — leitura de PR incompleta ({steps}).", + "cli_prs_none_degraded": "nenhum PR na janela — leitura de PR incompleta ({steps}).", "cli_classifying": "Classificando commits...", "cli_classified": "{count} commits classificados.", "cli_non_code_repo": ( diff --git a/tests/test_pr_fetch_degradation.py b/tests/test_pr_fetch_degradation.py index 3b6798b..d8b8baf 100644 --- a/tests/test_pr_fetch_degradation.py +++ b/tests/test_pr_fetch_degradation.py @@ -722,3 +722,45 @@ def fake_run(cmd, **kwargs): # `-F` would read a file named page2 for a value starting with "@". assert "cursor=@page2" in seen[1] assert seen[1][seen[1].index("cursor=@page2") - 1] == "-f" + + +# --- the console says what the PR read missed ------------------------------- + + +def _console_for(tmp_path, monkeypatch, capsys, fetch: PullRequestFetch) -> str: + from iris import cli + + repo = tmp_path / "repo" + repo.mkdir() + _build_repo(repo) + monkeypatch.setattr( + cli, "read_pull_requests_with_fallback", lambda *a, **k: fetch, + ) + args = argparse.Namespace( + repo_path=str(repo), days=30, churn_days=14, lang="en", recent_days=30, + verbose=False, trend=False, out=str(tmp_path / "out"), no_push=True, + ) + cli._run_single_repo(args) + return capsys.readouterr().out + + +def test_console_found_prs_with_a_degraded_read_warns(tmp_path, monkeypatch, capsys): + out = _console_for(tmp_path, monkeypatch, capsys, PullRequestFetch( + prs=_merged_prs(), degraded=("reviews",))) + + assert "6 merged PRs found — PR read incomplete (reviews)." in out + + +def test_console_no_prs_and_only_a_secondary_step_degraded(tmp_path, monkeypatch, capsys): + out = _console_for(tmp_path, monkeypatch, capsys, PullRequestFetch( + prs=[], degraded=("enrichment",))) + + assert "no PRs in the window — PR read incomplete (enrichment)." in out + assert "continuing without PR data" not in out + + +def test_console_no_prs_and_degraded_basic_keeps_the_failure(tmp_path, monkeypatch, capsys): + out = _console_for(tmp_path, monkeypatch, capsys, PullRequestFetch( + prs=[], degraded=("basic", "enrichment"))) + + assert "failed (basic, enrichment) — continuing without PR data." in out From ff3cc9b80bb27edd66fb6555627e760f54689256 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 20:02:14 -0300 Subject: [PATCH 07/22] refactor(engine): build acceptance groups with one helper The by-origin and by-tool blocks repeated the same comprehension plus an update loop for the review-derived keys. One small helper now builds both. No behavior change: the key order in each group is unchanged, so a clean run stays byte-identical. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Sonnet 5.5 --- iris/metrics/aggregator.py | 50 +++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/iris/metrics/aggregator.py b/iris/metrics/aggregator.py index 286f39e..7ade07c 100644 --- a/iris/metrics/aggregator.py +++ b/iris/metrics/aggregator.py @@ -47,6 +47,22 @@ logger = logging.getLogger(__name__) +def _acceptance_groups(groups, reviews_degraded: bool) -> dict: + """Acceptance metrics per group; review-derived keys only when reviews were read.""" + result = {} + for g in groups: + entry = { + "total_commits": g.total_commits, + "commits_in_prs": g.commits_in_prs, + "pr_rate": g.pr_rate, + } + if not reviews_degraded: + entry["single_pass_rate"] = g.single_pass_rate + entry["median_review_rounds"] = g.median_review_rounds + result[g.group] = entry + return result + + def aggregate( commits: list[Commit], churn_days: int, @@ -290,35 +306,13 @@ def aggregate( acceptance_result = calculate_acceptance_rate(commits, prs) if acceptance_result: if acceptance_result.by_origin: - acceptance_kwargs["acceptance_by_origin"] = { - g.group: { - "total_commits": g.total_commits, - "commits_in_prs": g.commits_in_prs, - "pr_rate": g.pr_rate, - } - for g in acceptance_result.by_origin - } - if not reviews_degraded: - for g in acceptance_result.by_origin: - acceptance_kwargs["acceptance_by_origin"][g.group].update({ - "single_pass_rate": g.single_pass_rate, - "median_review_rounds": g.median_review_rounds, - }) + acceptance_kwargs["acceptance_by_origin"] = _acceptance_groups( + acceptance_result.by_origin, reviews_degraded + ) if acceptance_result.by_tool: - acceptance_kwargs["acceptance_by_tool"] = { - g.group: { - "total_commits": g.total_commits, - "commits_in_prs": g.commits_in_prs, - "pr_rate": g.pr_rate, - } - for g in acceptance_result.by_tool - } - if not reviews_degraded: - for g in acceptance_result.by_tool: - acceptance_kwargs["acceptance_by_tool"][g.group].update({ - "single_pass_rate": g.single_pass_rate, - "median_review_rounds": g.median_review_rounds, - }) + acceptance_kwargs["acceptance_by_tool"] = _acceptance_groups( + acceptance_result.by_tool, reviews_degraded + ) # Flow Efficiency — active vs wait decomposition of merged PR lifecycle. # Its phases anchor on the first review: with the reviews read failed, From 2300c81fa3ea9c6483998b0379d0d4a7dac32e54 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 20:05:09 -0300 Subject: [PATCH 08/22] fix(engine): skip the origin funnel when enrichment failed The funnel's "In PR" stage reads acceptance_by_origin and, when it is absent, assumes every commit is in a PR. With the enrichment failed that field is now omitted, so the funnel reported 100% In PR and inflated every later conversion. Return no funnel then (the stages chain, so dropping one would break its meaning). Repos with no PR data keep the old default. Also fix two METRICS.md rows that claimed the review-round and single-pass fields drop on enrichment; only reviews gates them. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Sonnet 5.5 --- docs/METRICS.md | 10 +++-- iris/analysis/origin_funnel.py | 8 +++- tests/test_pr_fetch_degradation.py | 69 ++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 4 deletions(-) diff --git a/docs/METRICS.md b/docs/METRICS.md index 2f39105..5af5805 100644 --- a/docs/METRICS.md +++ b/docs/METRICS.md @@ -256,7 +256,11 @@ dataclass is always `None`; the field only exists in the emitted JSON. | Field | Unit | Source | Nullable when | |---|---|---|---| -| `origin_funnel` | `Record` | `analysis/origin_funnel.py` (via `reports/writer.py`) | no `commit_origin_distribution` | +| `origin_funnel` | `Record` | `analysis/origin_funnel.py` (via `reports/writer.py`) | no `commit_origin_distribution`, or `enrichment` degraded | + +The whole funnel is omitted when `enrichment` is in `pr_enrichment_degraded` +(§15): its `In PR` stage reads `acceptance_by_origin`, which is omitted then, +and every later conversion chains from that stage. Stages: `Committed` → `In PR` → `Stabilized` → `Lines Surviving` (the last stage only when durability data is available). Each stage carries @@ -363,8 +367,8 @@ All fields require GitHub PR data. | `pr_cycle_time_buckets` | `{same_day, one_day, two_to_three_days, four_to_seven_days, seven_plus_days}` ints | same | same | | `pr_median_size_files` | int ≥ 0 | same | same | | `pr_median_size_lines` | int ≥ 0 | same | same | -| `pr_review_rounds_median` | float ≥ 0 | same | same, or `reviews` or `enrichment` degraded | -| `pr_single_pass_rate` | float `0.0–1.0` | same | same, or `reviews` or `enrichment` degraded | +| `pr_review_rounds_median` | float ≥ 0 | same | same, or `reviews` degraded | +| `pr_single_pass_rate` | float `0.0–1.0` | same | same, or `reviews` degraded | | `pr_enrichment_degraded` | list of `basic\|enrichment\|reviews\|fetch` | `ingestion/github_reader.py` | every read step succeeded, or PRs are absent by design | `pr_review_rounds_median` — median count of `CHANGES_REQUESTED` reviews diff --git a/iris/analysis/origin_funnel.py b/iris/analysis/origin_funnel.py index 5d56a38..5ea0d12 100644 --- a/iris/analysis/origin_funnel.py +++ b/iris/analysis/origin_funnel.py @@ -19,6 +19,7 @@ from dataclasses import dataclass from iris.models.metrics import ReportMetrics +from iris.models.pull_request import DEGRADED_ENRICHMENT @dataclass(frozen=True) @@ -56,8 +57,13 @@ def calculate_origin_funnel(metrics: ReportMetrics) -> FunnelResult | None: metrics: ReportMetrics with origin, acceptance, and durability data. Returns: - FunnelResult with per-origin funnels, or None if no origin data. + FunnelResult with per-origin funnels, or None if no origin data, or + if the PR enrichment failed: the "In PR" stage reads acceptance, + which is omitted then, and every later conversion chains from it. """ + if DEGRADED_ENRICHMENT in (metrics.pr_enrichment_degraded or []): + return None + origin_dist = metrics.commit_origin_distribution if not origin_dist: return None diff --git a/tests/test_pr_fetch_degradation.py b/tests/test_pr_fetch_degradation.py index d8b8baf..0f60500 100644 --- a/tests/test_pr_fetch_degradation.py +++ b/tests/test_pr_fetch_degradation.py @@ -764,3 +764,72 @@ def test_console_no_prs_and_degraded_basic_keeps_the_failure(tmp_path, monkeypat prs=[], degraded=("basic", "enrichment"))) assert "failed (basic, enrichment) — continuing without PR data." in out + + +# --- a failed enrichment leaves out the origin funnel ----------------------- +# +# The funnel's "In PR" stage reads acceptance, which is omitted without the +# enrichment; it would fall back to "every commit is in a PR" and chain that +# into every later conversion. + +from iris.analysis.origin_funnel import calculate_origin_funnel + + +def _commits_with_ai() -> list[Commit]: + # The funnel needs an origin distribution, which only exists when some + # commit is AI-assisted or bot-made. + ai = Commit(hash="c7", author="dev", date=_NOW + timedelta(hours=7), + message="change 7 (#6)", + attribution_trailers=["Claude "]) + return _stamped_commits() + [ai] + + +def test_origin_funnel_is_none_when_enrichment_degraded(): + metrics = aggregate( + _commits_with_ai(), churn_days=14, prs=_reviewed_prs(), + pr_fetch_degraded=("enrichment",), + ) + + assert metrics.commit_origin_distribution + assert calculate_origin_funnel(metrics) is None + + +def test_origin_funnel_is_kept_on_a_clean_run(): + metrics = aggregate(_commits_with_ai(), churn_days=14, prs=_reviewed_prs()) + + assert calculate_origin_funnel(metrics) is not None + + +def _funnel_in_cli_metrics(tmp_path, monkeypatch, fetch: PullRequestFetch) -> dict: + from iris import cli + + repo = tmp_path / "repo" + repo.mkdir() + _build_repo(repo) + (repo / "a.py").write_text("x = 99\n", encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=repo, check=True, capture_output=True) + subprocess.run( + ["git", "commit", "-q", "-m", + "feat: ai change\n\nCo-Authored-By: Claude "], + cwd=repo, check=True, capture_output=True) + monkeypatch.setattr(cli, "read_pull_requests_with_fallback", lambda *a, **k: fetch) + args = argparse.Namespace( + repo_path=str(repo), days=30, churn_days=14, lang="en", recent_days=30, + verbose=False, trend=False, out=str(tmp_path / "out"), no_push=True, + ) + cli._run_single_repo(args) + return _metrics_json(tmp_path / "out") + + +def test_cli_metrics_json_has_no_funnel_when_enrichment_degraded(tmp_path, monkeypatch): + payload = _funnel_in_cli_metrics( + tmp_path, monkeypatch, PullRequestFetch(prs=[], degraded=("enrichment",))) + + assert "origin_funnel" not in payload + + +def test_cli_metrics_json_keeps_the_funnel_on_a_clean_run(tmp_path, monkeypatch): + payload = _funnel_in_cli_metrics( + tmp_path, monkeypatch, PullRequestFetch(prs=[], degraded=())) + + assert "origin_funnel" in payload From e4de8dc0b1dc6cbf98720a11ae38caf6e1d88d68 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 20:44:32 -0300 Subject: [PATCH 09/22] fix(engine): skip the funnel whenever a degraded read lost acceptance The funnel returned nothing only when enrichment failed. When every state's list failed (basic) or the whole read raised (fetch), there are no PRs, acceptance is absent, and the "In PR" stage fell back to every commit being in a PR: 100% In PR in metrics.json and the report. Now the funnel is omitted whenever the read was degraded and acceptance is absent. PRs absent by design (no gh, no GitHub remote) mark nothing as degraded and keep the old default. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 --- docs/METRICS.md | 14 ++++++++----- iris/analysis/origin_funnel.py | 11 ++++++---- tests/test_pr_fetch_degradation.py | 33 ++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 9 deletions(-) diff --git a/docs/METRICS.md b/docs/METRICS.md index 5af5805..99b6412 100644 --- a/docs/METRICS.md +++ b/docs/METRICS.md @@ -256,11 +256,15 @@ dataclass is always `None`; the field only exists in the emitted JSON. | Field | Unit | Source | Nullable when | |---|---|---|---| -| `origin_funnel` | `Record` | `analysis/origin_funnel.py` (via `reports/writer.py`) | no `commit_origin_distribution`, or `enrichment` degraded | - -The whole funnel is omitted when `enrichment` is in `pr_enrichment_degraded` -(§15): its `In PR` stage reads `acceptance_by_origin`, which is omitted then, -and every later conversion chains from that stage. +| `origin_funnel` | `Record` | `analysis/origin_funnel.py` (via `reports/writer.py`) | no `commit_origin_distribution`, or `pr_enrichment_degraded` present and `acceptance_by_origin` absent | + +The whole funnel is omitted when `pr_enrichment_degraded` (§15) is present and +`acceptance_by_origin` is absent: its `In PR` stage reads acceptance, and every +later conversion chains from that stage. That covers a failed `enrichment`, +which omits acceptance (§9), and a read that lost the PRs — `basic` failed for +every state, or `fetch` — which leaves no PRs to match commits to. Only when +PRs are absent by design (no `gh`, no GitHub remote; nothing degraded) does +`In PR` fall back to counting every commit as in a PR. Stages: `Committed` → `In PR` → `Stabilized` → `Lines Surviving` (the last stage only when durability data is available). Each stage carries diff --git a/iris/analysis/origin_funnel.py b/iris/analysis/origin_funnel.py index 5ea0d12..b3dd246 100644 --- a/iris/analysis/origin_funnel.py +++ b/iris/analysis/origin_funnel.py @@ -19,7 +19,6 @@ from dataclasses import dataclass from iris.models.metrics import ReportMetrics -from iris.models.pull_request import DEGRADED_ENRICHMENT @dataclass(frozen=True) @@ -58,10 +57,14 @@ def calculate_origin_funnel(metrics: ReportMetrics) -> FunnelResult | None: Returns: FunnelResult with per-origin funnels, or None if no origin data, or - if the PR enrichment failed: the "In PR" stage reads acceptance, - which is omitted then, and every later conversion chains from it. + if the PR read was degraded and acceptance is absent: the "In PR" + stage reads acceptance, and every later conversion chains from it. """ - if DEGRADED_ENRICHMENT in (metrics.pr_enrichment_degraded or []): + # A failed enrichment omits acceptance, and a failed list or read leaves + # no PRs to match commits to; either way "In PR" would fall back to + # "every commit is in a PR". That default is kept only for PRs absent by + # design (no gh, no GitHub remote), where nothing is marked degraded. + if metrics.pr_enrichment_degraded and not metrics.acceptance_by_origin: return None origin_dist = metrics.commit_origin_distribution diff --git a/tests/test_pr_fetch_degradation.py b/tests/test_pr_fetch_degradation.py index 0f60500..ac01734 100644 --- a/tests/test_pr_fetch_degradation.py +++ b/tests/test_pr_fetch_degradation.py @@ -11,6 +11,8 @@ import sys from pathlib import Path +import pytest + sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) from iris.ingestion import github_reader, window_cache @@ -794,6 +796,27 @@ def test_origin_funnel_is_none_when_enrichment_degraded(): assert calculate_origin_funnel(metrics) is None +@pytest.mark.parametrize("degraded", [("basic",), ("fetch",)]) +def test_origin_funnel_is_none_when_the_read_lost_the_prs(degraded): + # Every state's list failed (`basic`) or the whole read did (`fetch`): + # there are no PRs, so acceptance is absent and "In PR" would fall back + # to every commit being in a PR. + metrics = aggregate( + _commits_with_ai(), churn_days=14, prs=[], pr_fetch_degraded=degraded, + ) + + assert metrics.commit_origin_distribution + assert metrics.acceptance_by_origin is None + assert calculate_origin_funnel(metrics) is None + + +def test_origin_funnel_keeps_the_default_when_prs_are_absent_by_design(): + # No gh or no GitHub remote: nothing failed, the field is absent. + metrics = aggregate(_commits_with_ai(), churn_days=14, prs=[]) + + assert calculate_origin_funnel(metrics) is not None + + def test_origin_funnel_is_kept_on_a_clean_run(): metrics = aggregate(_commits_with_ai(), churn_days=14, prs=_reviewed_prs()) @@ -828,6 +851,16 @@ def test_cli_metrics_json_has_no_funnel_when_enrichment_degraded(tmp_path, monke assert "origin_funnel" not in payload +@pytest.mark.parametrize("degraded", [("basic",), ("fetch",)]) +def test_cli_metrics_json_has_no_funnel_when_the_read_lost_the_prs( + tmp_path, monkeypatch, degraded +): + payload = _funnel_in_cli_metrics( + tmp_path, monkeypatch, PullRequestFetch(prs=[], degraded=degraded)) + + assert "origin_funnel" not in payload + + def test_cli_metrics_json_keeps_the_funnel_on_a_clean_run(tmp_path, monkeypatch): payload = _funnel_in_cli_metrics( tmp_path, monkeypatch, PullRequestFetch(prs=[], degraded=())) From daf119055d4203fd8eeac95313589ec26c0eb6ad Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 20:45:09 -0300 Subject: [PATCH 10/22] fix(engine): pass string GraphQL variables as raw fields The enrichment query sent owner and name with `-F`, which lets gh coerce the value: a repo named 2048 went out as the number 2048 (and `true`/`null` as a boolean/null), so the `String!` variable failed on every page and the enrichment always degraded. Every variable of the query is a string, so all of them now go with `-f`, as the cursor and states already did. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 --- iris/ingestion/github_reader.py | 8 ++++++-- tests/test_pr_fetch_degradation.py | 27 ++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/iris/ingestion/github_reader.py b/iris/ingestion/github_reader.py index f33eaf0..470a8a4 100644 --- a/iris/ingestion/github_reader.py +++ b/iris/ingestion/github_reader.py @@ -266,11 +266,15 @@ def _fetch_pr_enrichment_graphql( end_cursor: str | None = None while len(by_pr) < max_prs: + # Every variable is a string, so each goes as a raw field (`-f`): + # `-F` lets gh coerce the value, sending a repo named 2048 as a + # number (or `true`/`null` as a boolean/null) that `String!` rejects, + # and reading a file for a value starting with "@". args = [ "gh", "api", "graphql", "-f", "query=" + _COMMITS_GRAPHQL_QUERY, - "-F", f"owner={owner}", - "-F", f"name={name}", + "-f", f"owner={owner}", + "-f", f"name={name}", "-f", f"states[]={graphql_state}", ] if end_cursor: diff --git a/tests/test_pr_fetch_degradation.py b/tests/test_pr_fetch_degradation.py index ac01734..19ee1da 100644 --- a/tests/test_pr_fetch_degradation.py +++ b/tests/test_pr_fetch_degradation.py @@ -703,7 +703,7 @@ def test_enrichment_node_with_null_commits_has_no_commits(monkeypatch): assert complete is True -# --- the cursor is sent as a raw field -------------------------------------- +# --- string variables are sent as raw fields -------------------------------- def test_cursor_is_passed_as_a_raw_field(monkeypatch): @@ -726,6 +726,31 @@ def fake_run(cmd, **kwargs): assert seen[1][seen[1].index("cursor=@page2") - 1] == "-f" +@pytest.mark.parametrize("name", ["2048", "true", "null"]) +def test_string_variables_are_passed_as_raw_fields(monkeypatch, name): + # `-F` lets gh coerce a value: a repo named 2048 would be sent as the + # number 2048 (and `true`/`null` as a boolean/null), which the + # `String!` variable rejects on every page. + seen: list[list[str]] = [] + answers = [ + _graphql_page([1], has_next=True, cursor="page2"), + _graphql_page([2]), + ] + + def fake_run(cmd, **kwargs): + seen.append(cmd) + return _ok(answers.pop(0)) + + monkeypatch.setattr(github_reader.subprocess, "run", fake_run) + + github_reader._fetch_pr_enrichment_graphql(f"1234/{name}", "merged", 500) + + second = seen[1] + for field in ("owner=1234", f"name={name}", "states[]=MERGED", "cursor=page2"): + assert field in second, field + assert second[second.index(field) - 1] == "-f", field + + # --- the console says what the PR read missed ------------------------------- From 73534484df6c99ecb0b1dae04ae30f6720f0d5ea Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 20:45:42 -0300 Subject: [PATCH 11/22] fix(engine): skip null GraphQL nodes and commits A page with `"nodes": null` raised a TypeError, and a null PR node or a null commit entry raised an AttributeError; any of them crashed the whole PR read into `fetch`. The enrichment pass now reads a null list as empty and skips nodes and commit entries that are not objects, keeping the rest of the page. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 --- iris/ingestion/github_reader.py | 7 ++++++- tests/test_pr_fetch_degradation.py | 33 ++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/iris/ingestion/github_reader.py b/iris/ingestion/github_reader.py index 470a8a4..6f69a9d 100644 --- a/iris/ingestion/github_reader.py +++ b/iris/ingestion/github_reader.py @@ -297,7 +297,10 @@ def _fetch_pr_enrichment_graphql( if not page or not isinstance(page, dict): return by_pr, False - for node in page.get("nodes", []): + # A null list, PR node or commit entry is skipped the same way. + for node in page.get("nodes") or []: + if not isinstance(node, dict): + continue number = node.get("number") if number is None: continue @@ -305,6 +308,8 @@ def _fetch_pr_enrichment_graphql( commits_conn = node.get("commits") entries = commits_conn.get("nodes") if isinstance(commits_conn, dict) else None for entry in entries or []: + if not isinstance(entry, dict): + continue commit = entry.get("commit") or {} oid = commit.get("oid", "") if not oid: diff --git a/tests/test_pr_fetch_degradation.py b/tests/test_pr_fetch_degradation.py index 19ee1da..5f1b39e 100644 --- a/tests/test_pr_fetch_degradation.py +++ b/tests/test_pr_fetch_degradation.py @@ -703,6 +703,39 @@ def test_enrichment_node_with_null_commits_has_no_commits(monkeypatch): assert complete is True +def test_enrichment_page_with_null_nodes_has_no_prs(monkeypatch): + page = _graphql_page([]) + page["data"]["repository"]["pullRequests"]["nodes"] = None + _fake_gh(monkeypatch, graphql=[page]) + + by_pr, complete = github_reader._fetch_pr_enrichment_graphql("acme/widgets", "merged", 500) + + assert by_pr == {} + assert complete is True + + +def test_enrichment_skips_a_null_node(monkeypatch): + page = _graphql_page([1, 2]) + page["data"]["repository"]["pullRequests"]["nodes"].insert(1, None) + _fake_gh(monkeypatch, graphql=[page]) + + by_pr, complete = github_reader._fetch_pr_enrichment_graphql("acme/widgets", "merged", 500) + + assert sorted(by_pr) == [1, 2] + assert complete is True + + +def test_enrichment_skips_a_null_commit_entry(monkeypatch): + page = _graphql_page([1]) + page["data"]["repository"]["pullRequests"]["nodes"][0]["commits"]["nodes"].insert(0, None) + _fake_gh(monkeypatch, graphql=[page]) + + by_pr, complete = github_reader._fetch_pr_enrichment_graphql("acme/widgets", "merged", 500) + + assert [c["oid"] for c in by_pr[1]["commits"]] == ["commit1"] + assert complete is True + + # --- string variables are sent as raw fields -------------------------------- From 874a51bfea7fef00b3ef9998020da6b5e5fc4fb0 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 20:46:20 -0300 Subject: [PATCH 12/22] fix(engine): count all PR states in the console line The console printed "{count} merged PRs found", but the count is every PR read in the window: merged, closed and open. Both the clean and the degraded line now say "{count} PRs found" ("{count} PRs encontrados" in pt-BR). Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 --- iris/i18n.py | 8 ++++---- tests/test_pr_fetch_degradation.py | 21 ++++++++++++++++++++- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/iris/i18n.py b/iris/i18n.py index a26ba53..511c01a 100644 --- a/iris/i18n.py +++ b/iris/i18n.py @@ -338,10 +338,10 @@ # CLI "cli_reading_prs": "Fetching pull requests...", - "cli_prs_found": "{count} merged PRs found.", + "cli_prs_found": "{count} PRs found.", "cli_prs_skipped": "skipped (no GitHub remote or gh CLI).", "cli_prs_failed": "failed ({steps}) — continuing without PR data.", - "cli_prs_found_degraded": "{count} merged PRs found — PR read incomplete ({steps}).", + "cli_prs_found_degraded": "{count} PRs found — PR read incomplete ({steps}).", "cli_prs_none_degraded": "no PRs in the window — PR read incomplete ({steps}).", "cli_classifying": "Classifying commits...", "cli_classified": "{count} commits classified.", @@ -1233,10 +1233,10 @@ # CLI "cli_reading_prs": "Buscando pull requests...", - "cli_prs_found": "{count} PRs merged encontrados.", + "cli_prs_found": "{count} PRs encontrados.", "cli_prs_skipped": "pulado (sem remote GitHub ou gh CLI).", "cli_prs_failed": "falhou ({steps}) — seguindo sem dados de PR.", - "cli_prs_found_degraded": "{count} PRs merged encontrados — leitura de PR incompleta ({steps}).", + "cli_prs_found_degraded": "{count} PRs encontrados — leitura de PR incompleta ({steps}).", "cli_prs_none_degraded": "nenhum PR na janela — leitura de PR incompleta ({steps}).", "cli_classifying": "Classificando commits...", "cli_classified": "{count} commits classificados.", diff --git a/tests/test_pr_fetch_degradation.py b/tests/test_pr_fetch_degradation.py index 5f1b39e..6cb52a5 100644 --- a/tests/test_pr_fetch_degradation.py +++ b/tests/test_pr_fetch_degradation.py @@ -808,7 +808,26 @@ def test_console_found_prs_with_a_degraded_read_warns(tmp_path, monkeypatch, cap out = _console_for(tmp_path, monkeypatch, capsys, PullRequestFetch( prs=_merged_prs(), degraded=("reviews",))) - assert "6 merged PRs found — PR read incomplete (reviews)." in out + assert "6 PRs found — PR read incomplete (reviews)." in out + + +def test_console_count_covers_every_pr_state(tmp_path, monkeypatch, capsys): + out = _console_for(tmp_path, monkeypatch, capsys, PullRequestFetch( + prs=_merged_prs() + _open_prs(), degraded=())) + + assert "11 PRs found." in out + assert "merged PRs found" not in out + + +def test_console_count_does_not_say_merged_in_portuguese(): + from iris.i18n import get_strings + + pt = get_strings("pt-br") + + assert pt["cli_prs_found"].format(count=11) == "11 PRs encontrados." + assert pt["cli_prs_found_degraded"].format(count=11, steps="reviews") == ( + "11 PRs encontrados — leitura de PR incompleta (reviews)." + ) def test_console_no_prs_and_only_a_secondary_step_degraded(tmp_path, monkeypatch, capsys): From 0e0bf9dd0ff977c9cf8cc1b02142e0f41f87ff47 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 21:18:19 -0300 Subject: [PATCH 13/22] fix(engine): treat a page without a nodes list as incomplete A page whose `nodes` was null, or not a list at all, was read as an empty page: the enrichment pass ended "complete" with none of that page's PRs. It is now an incomplete page, like a null `pullRequests`: the pass keeps what earlier pages collected and reports enrichment as failed. Null PR nodes and commit entries inside the list are still skipped. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 --- iris/ingestion/github_reader.py | 8 ++++++-- tests/test_pr_fetch_degradation.py | 21 ++++++++++++++++++--- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/iris/ingestion/github_reader.py b/iris/ingestion/github_reader.py index 6f69a9d..2c9fb45 100644 --- a/iris/ingestion/github_reader.py +++ b/iris/ingestion/github_reader.py @@ -297,8 +297,12 @@ def _fetch_pr_enrichment_graphql( if not page or not isinstance(page, dict): return by_pr, False - # A null list, PR node or commit entry is skipped the same way. - for node in page.get("nodes") or []: + # Without a list of nodes the page's PRs are missing, as above. A null + # PR node or commit entry inside the list is skipped instead. + nodes = page.get("nodes") + if not isinstance(nodes, list): + return by_pr, False + for node in nodes: if not isinstance(node, dict): continue number = node.get("number") diff --git a/tests/test_pr_fetch_degradation.py b/tests/test_pr_fetch_degradation.py index 6cb52a5..2649fc3 100644 --- a/tests/test_pr_fetch_degradation.py +++ b/tests/test_pr_fetch_degradation.py @@ -703,15 +703,30 @@ def test_enrichment_node_with_null_commits_has_no_commits(monkeypatch): assert complete is True -def test_enrichment_page_with_null_nodes_has_no_prs(monkeypatch): +@pytest.mark.parametrize("nodes", [None, {}, "nodes"]) +def test_enrichment_page_without_a_nodes_list_is_incomplete(monkeypatch, nodes): + # Like a null `pullRequests`: the page's PRs are missing, not absent. page = _graphql_page([]) - page["data"]["repository"]["pullRequests"]["nodes"] = None + page["data"]["repository"]["pullRequests"]["nodes"] = nodes _fake_gh(monkeypatch, graphql=[page]) by_pr, complete = github_reader._fetch_pr_enrichment_graphql("acme/widgets", "merged", 500) assert by_pr == {} - assert complete is True + assert complete is False + + +def test_enrichment_later_page_without_a_nodes_list_keeps_what_it_has(monkeypatch): + later = _graphql_page([]) + later["data"]["repository"]["pullRequests"]["nodes"] = None + _fake_gh(monkeypatch, graphql=[ + _graphql_page([1, 2], has_next=True, cursor="page2"), later, + ]) + + by_pr, complete = github_reader._fetch_pr_enrichment_graphql("acme/widgets", "merged", 500) + + assert sorted(by_pr) == [1, 2] + assert complete is False def test_enrichment_skips_a_null_node(monkeypatch): From 0ec130bdd1c896fbb19e7beced68eac8a9c51403 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 20:11:22 -0300 Subject: [PATCH 14/22] fix(engine): retry only transient gh errors, and the reviews pass too The enrichment retry waited 2 s on any gh failure, deterministic ones included: a 401, a 404 or a GraphQL error fails the same way on the second try, so every run paid the pause per page and per PR state for nothing. The retry now reads gh's stderr and only tries again on a transient error (HTTP 5xx, a timeout, a dropped connection). The reviews pass, which the code documents as prone to 504s, had no retry at all, while one failure of it now drops review metrics, the flow block and staleness from the report. It gets the same single transient-only retry. The basic list stays as it was. The pause constant becomes _RETRY_DELAY_S, since both passes share it. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 --- docs/METRICS.md | 6 +- iris/ingestion/github_reader.py | 82 ++++++++++++++++++---------- tests/test_pr_fetch_degradation.py | 88 +++++++++++++++++++++++++++++- 3 files changed, 143 insertions(+), 33 deletions(-) diff --git a/docs/METRICS.md b/docs/METRICS.md index 99b6412..9a845e2 100644 --- a/docs/METRICS.md +++ b/docs/METRICS.md @@ -390,8 +390,10 @@ without persisting individual PR durations. `pr_enrichment_degraded` — the steps of the GitHub PR read that failed this run and fell back to partial data: `basic` (`gh pr list` for one state failed, so that state's PRs are missing), `enrichment` (the GraphQL pass for commit -refs and merge-commit parents failed, after one retry), `reviews` (the reviews -pass failed), `fetch` (the whole read raised). Sorted, without repeats. Unlike +refs and merge-commit parents failed), `reviews` (the reviews pass failed), +`fetch` (the whole read raised). The enrichment and reviews passes get one +retry when gh reports a transient error (HTTP 5xx, a timeout, a dropped +connection); a 4xx or GraphQL error is not retried. Sorted, without repeats. Unlike the fields above it does not require PR data: a read that lost every PR still reports why. Absent when every step succeeded, and when PRs are absent by design — no `gh`, no GitHub remote — because Iris works without PRs. When diff --git a/iris/ingestion/github_reader.py b/iris/ingestion/github_reader.py index 2c9fb45..a51e264 100644 --- a/iris/ingestion/github_reader.py +++ b/iris/ingestion/github_reader.py @@ -124,7 +124,8 @@ def _fetch_prs(nwo: str, limit: int, gh_state: str) -> tuple[list[dict], set[str 3. Reviews via `gh pr list --json number,reviews` in one shot, also capped at `_BATCH_SIZE`. - Both secondary passes are best-effort — if either fails the PRs come + Both secondary passes are best-effort, with one retry on a transient + failure (`_run_gh_with_one_retry`) — if either still fails the PRs come back with the respective field empty, but the rest of the metadata is still usable. @@ -158,7 +159,9 @@ def _fetch_prs(nwo: str, limit: int, gh_state: str) -> tuple[list[dict], set[str if merge_commit: pr["mergeCommit"] = merge_commit - reviews_prs = _gh_pr_list(nwo, "number,reviews", min(limit, _BATCH_SIZE), gh_state) + reviews_prs = _gh_pr_list( + nwo, "number,reviews", min(limit, _BATCH_SIZE), gh_state, retry=True, + ) if reviews_prs is None: degraded.add(DEGRADED_REVIEWS) elif reviews_prs: @@ -204,14 +207,27 @@ def _fetch_prs(nwo: str, limit: int, gh_state: str) -> tuple[list[dict], set[str } -# One retry for the enrichment query, after this pause. GitHub answers that -# query with intermittent 504s (see `_BATCH_SIZE`); a single retry makes a -# degraded run rarer without hiding the ones that still fail. -_ENRICHMENT_RETRY_DELAY_S = 2.0 +# One retry for the enrichment query and the reviews pass, after this pause. +# GitHub answers both with intermittent 504s (see `_BATCH_SIZE`); a single +# retry makes a degraded run rarer without hiding the ones that still fail. +_RETRY_DELAY_S = 2.0 + +# gh writes why it failed to stderr. A 5xx or a slow or dropped connection can +# pass on a second try; anything else — a 4xx, a GraphQL error — fails the same +# way again, so retrying it would only add the pause to every run. +_TRANSIENT_GH_ERROR = re.compile( + r"HTTP 5\d\d|timeout|timed out|deadline exceeded|connection reset" + r"|connection refused|EOF", + re.IGNORECASE, +) def _run_gh_with_one_retry(args: list[str]) -> subprocess.CompletedProcess | None: - """Run a gh command; on failure wait once and retry. None if both fail.""" + """Run a gh command; if it fails transiently, wait once and retry. + + None when the command fails twice, or fails once with an error that a + retry would not fix. + """ for attempt in (1, 2): try: return subprocess.run( @@ -220,9 +236,10 @@ def _run_gh_with_one_retry(args: list[str]) -> subprocess.CompletedProcess | Non ) except FileNotFoundError: return None - except subprocess.CalledProcessError: - if attempt == 1: - time.sleep(_ENRICHMENT_RETRY_DELAY_S) + except subprocess.CalledProcessError as error: + if attempt == 2 or not _TRANSIENT_GH_ERROR.search(error.stderr or ""): + return None + time.sleep(_RETRY_DELAY_S) return None @@ -342,26 +359,33 @@ def _fetch_pr_enrichment_graphql( return by_pr, True -def _gh_pr_list(nwo: str, fields: str, limit: int, gh_state: str) -> list[dict] | None: - """Run gh pr list and return parsed JSON, or None on failure.""" - try: - result = subprocess.run( - [ - "gh", "pr", "list", - "--repo", nwo, - "--state", gh_state, - "--json", fields, - "--limit", str(limit), - ], - capture_output=True, - text=True, - check=True, - env=git_env(), - ) - except (subprocess.CalledProcessError, FileNotFoundError): - return None +def _gh_pr_list( + nwo: str, fields: str, limit: int, gh_state: str, *, retry: bool = False, +) -> list[dict] | None: + """Run gh pr list and return parsed JSON, or None on failure. - if not result.stdout.strip(): + With ``retry``, a transient failure gets one more try + (`_run_gh_with_one_retry`). + """ + args = [ + "gh", "pr", "list", + "--repo", nwo, + "--state", gh_state, + "--json", fields, + "--limit", str(limit), + ] + if retry: + result = _run_gh_with_one_retry(args) + else: + try: + result = subprocess.run( + args, capture_output=True, text=True, check=True, + env=git_env(), + ) + except (subprocess.CalledProcessError, FileNotFoundError): + result = None + + if result is None or not result.stdout.strip(): return None try: diff --git a/tests/test_pr_fetch_degradation.py b/tests/test_pr_fetch_degradation.py index 2649fc3..40d902e 100644 --- a/tests/test_pr_fetch_degradation.py +++ b/tests/test_pr_fetch_degradation.py @@ -71,6 +71,8 @@ def broken_read(_repo_path, _days): import subprocess from types import SimpleNamespace +import pytest + def _ok(payload) -> SimpleNamespace: return SimpleNamespace(returncode=0, stdout=json.dumps(payload), stderr="") @@ -99,10 +101,12 @@ def _fake_gh(monkeypatch, *, full=None, basic=None, graphql=(), reviews=None): """Route each gh call to a scripted answer. `full`, `basic` and `reviews` are a payload, or an exception to raise. + `reviews` can also be a tuple of answers for successive reviews calls. `graphql` is the sequence of answers for successive GraphQL calls. Returns the list of pauses the retry slept, so a test can assert on them. """ graphql_answers = list(graphql) + reviews_answers = list(reviews) if isinstance(reviews, tuple) else None sleeps: list[float] = [] def answer(spec, cmd): @@ -117,6 +121,8 @@ def fake_run(cmd, **kwargs): return answer(graphql_answers.pop(0), cmd) fields = cmd[cmd.index("--json") + 1] if fields == "number,reviews": + if reviews_answers is not None: + return answer(reviews_answers.pop(0), cmd) return answer(reviews, cmd) if "commits" in fields: return answer(full, cmd) @@ -131,6 +137,14 @@ def _failure() -> subprocess.CalledProcessError: return subprocess.CalledProcessError(1, ["gh"], stderr="HTTP 504") +# Errors that fail the same way on a second try: retrying them only waits. +_DETERMINISTIC_STDERR = ( + "HTTP 401: Bad credentials (https://api.github.com/graphql)", + "gh: Not Found (HTTP 404)", + "GraphQL: Could not resolve to a Repository with the name 'acme/widgets'. (repository)", +) + + def test_enrichment_completes_on_a_single_page(monkeypatch): sleeps = _fake_gh(monkeypatch, graphql=[_graphql_page([1, 2])]) @@ -148,7 +162,7 @@ def test_enrichment_retries_once_and_recovers(monkeypatch): assert sorted(by_pr) == [1] assert complete is True - assert sleeps == [github_reader._ENRICHMENT_RETRY_DELAY_S] + assert sleeps == [github_reader._RETRY_DELAY_S] def test_enrichment_that_fails_twice_is_incomplete(monkeypatch): @@ -158,7 +172,77 @@ def test_enrichment_that_fails_twice_is_incomplete(monkeypatch): assert by_pr == {} assert complete is False - assert sleeps == [github_reader._ENRICHMENT_RETRY_DELAY_S] + assert sleeps == [github_reader._RETRY_DELAY_S] + + +@pytest.mark.parametrize("stderr", _DETERMINISTIC_STDERR) +def test_enrichment_does_not_retry_a_deterministic_error(monkeypatch, stderr): + # One scripted answer: a second call would find the script empty and raise. + sleeps = _fake_gh(monkeypatch, graphql=[ + subprocess.CalledProcessError(1, ["gh"], stderr=stderr), + ]) + + by_pr, complete = github_reader._fetch_pr_enrichment_graphql("acme/widgets", "merged", 500) + + assert by_pr == {} + assert complete is False + assert sleeps == [] + + +def test_reviews_pass_retries_a_transient_failure_and_recovers(monkeypatch): + review = {"author": {"login": "reviewer"}, "state": "APPROVED", + "submittedAt": "2026-09-01T00:00:00Z"} + sleeps = _fake_gh( + monkeypatch, + basic=[{"number": 1}], + graphql=[_graphql_page([1])], + reviews=(_failure(), [{"number": 1, "reviews": [review]}]), + ) + + prs, degraded = github_reader._fetch_prs("acme/widgets", 1000, "merged") + + assert prs[0]["reviews"] == [review] + assert degraded == set() + assert sleeps == [github_reader._RETRY_DELAY_S] + + +def test_reviews_pass_that_fails_twice_is_reported(monkeypatch): + sleeps = _fake_gh( + monkeypatch, + basic=[{"number": 1}], + graphql=[_graphql_page([1])], + reviews=(_failure(), _failure()), + ) + + _prs, degraded = github_reader._fetch_prs("acme/widgets", 1000, "merged") + + assert degraded == {"reviews"} + assert sleeps == [github_reader._RETRY_DELAY_S] + + +@pytest.mark.parametrize("stderr", _DETERMINISTIC_STDERR) +def test_reviews_pass_does_not_retry_a_deterministic_error(monkeypatch, stderr): + sleeps = _fake_gh( + monkeypatch, + basic=[{"number": 1}], + graphql=[_graphql_page([1])], + reviews=(subprocess.CalledProcessError(1, ["gh"], stderr=stderr),), + ) + + _prs, degraded = github_reader._fetch_prs("acme/widgets", 1000, "merged") + + assert degraded == {"reviews"} + assert sleeps == [] + + +def test_basic_list_is_not_retried(monkeypatch): + sleeps = _fake_gh(monkeypatch, basic=_failure()) + + prs, degraded = github_reader._fetch_prs("acme/widgets", 1000, "merged") + + assert prs == [] + assert degraded == {"basic"} + assert sleeps == [] def test_enrichment_failing_on_a_later_page_keeps_what_it_has_but_is_incomplete(monkeypatch): From 54c7a1d13f53ef287cc9bf32efe77842ba8f8d59 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 20:16:22 -0300 Subject: [PATCH 15/22] fix(engine): gate degraded metrics on the PR state that feeds them The PR read fetches merged, closed and open PRs separately, but the aggregator gated on the union of the three: a reviews or enrichment failure only for open PRs wiped review, flow, acceptance and merge strategy, which read merged PRs alone. That threw away good data. PullRequestFetch now keeps the failed steps per state (degraded_by_state), the window-cache slice inherits them, and every aggregate() call in the CLI and the org runner passes them. Review, flow, acceptance and merge strategy follow the merged state; open-PR staleness follows the open state; a closed-state failure omits nothing, since closed PRs only feed counts. Without the per-state map (a fetch that never reached the states, or no map at all) every state takes the union, as before. The origin funnel cannot tell the states apart from the union alone, so it now skips when enrichment failed and acceptance is absent. The aggregator always omits acceptance when the merged state's enrichment failed, so the funnel's "In PR" stage still never falls back to a made-up "every commit is in a PR" then, and the funnel survives a failure that hit only open or closed PRs. pr_enrichment_degraded keeps listing the union, so the JSON contract does not change, and a clean run's outputs stay identical. The report caveat and METRICS.md now say the omissions follow the PR state. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 --- docs/METRICS.md | 26 ++++--- iris/analysis/origin_funnel.py | 11 ++- iris/cli.py | 4 + iris/i18n.py | 16 ++-- iris/ingestion/github_reader.py | 5 ++ iris/ingestion/window_cache.py | 1 + iris/metrics/aggregator.py | 41 ++++++---- iris/models/pull_request.py | 6 ++ iris/org_runner.py | 4 + tests/test_pr_fetch_degradation.py | 115 +++++++++++++++++++++++++++++ tests/test_window_cache.py | 6 +- 11 files changed, 198 insertions(+), 37 deletions(-) diff --git a/docs/METRICS.md b/docs/METRICS.md index 9a845e2..e181bdc 100644 --- a/docs/METRICS.md +++ b/docs/METRICS.md @@ -241,9 +241,9 @@ its PR (if any) and aggregated by origin and AI tool. `AcceptanceMetrics`: `total_commits`, `commits_in_prs`, `pr_rate` (commits in PRs / total), `single_pass_rate` (PRs merged with -zero `CHANGES_REQUESTED` / PRs), `median_review_rounds`. The last two are omitted when `reviews` is in -`pr_enrichment_degraded` (§15). Both maps are omitted entirely when `enrichment` -is in `pr_enrichment_degraded`: commits are matched to PRs through the commit +zero `CHANGES_REQUESTED` / PRs), `median_review_rounds`. The last two are omitted when `reviews` failed +for merged PRs (§15). Both maps are omitted entirely when `enrichment` failed +for merged PRs: commits are matched to PRs through the commit refs that pass reads, so without them every group would read 0 commits in PRs. --- @@ -260,11 +260,11 @@ dataclass is always `None`; the field only exists in the emitted JSON. The whole funnel is omitted when `pr_enrichment_degraded` (§15) is present and `acceptance_by_origin` is absent: its `In PR` stage reads acceptance, and every -later conversion chains from that stage. That covers a failed `enrichment`, -which omits acceptance (§9), and a read that lost the PRs — `basic` failed for -every state, or `fetch` — which leaves no PRs to match commits to. Only when -PRs are absent by design (no `gh`, no GitHub remote; nothing degraded) does -`In PR` fall back to counting every commit as in a PR. +later conversion chains from that stage. That covers a failed `enrichment` for +merged PRs, which omits acceptance (§9), and a read that lost the PRs — `basic` +failed for every state, or `fetch` — which leaves no PRs to match commits to. +Only when PRs are absent by design (no `gh`, no GitHub remote; nothing +degraded) does `In PR` fall back to counting every commit as in a PR. Stages: `Committed` → `In PR` → `Stabilized` → `Lines Surviving` (the last stage only when durability data is available). Each stage carries @@ -419,6 +419,14 @@ refs. Open PR count and ages (`open_pr_count`, `median_open_pr_age_days`, depends only on the creation time. It is the first field to read when two runs on the same commit disagree. +The read fetches merged, closed and open PRs separately, and a step can fail +for one state only, so each omission above follows the state that feeds the +field. The review fields, Flow Efficiency, acceptance (§9) and merge strategy +(§28) read merged PRs: only a failure for merged PRs omits them. Staleness +reads open PRs: only a failure for open PRs omits it. Closed PRs feed only +counts (Flow Load, §24), so a failure there omits nothing. +`pr_enrichment_degraded` still lists the union over the three states. + --- ## 16. Duplicate block detection @@ -952,7 +960,7 @@ Platform: indexed columns `merge_strategy` + `commit_metrics_reliable` on carries `merge_strategy_dominant_share` for the repo-detail badge. **Degraded enrichment.** When the PR read's `enrichment` step failed -(`pr_enrichment_degraded` contains `enrichment`, from any PR state, §15), the repo is `unknown` +for merged PRs (`pr_enrichment_degraded` then contains `enrichment`, §15), the repo is `unknown` without classifying: `merge_strategy_dominant_share` is absent and `commit_metrics_reliable` is `true`. Without parent counts and commit refs, a merge PR falls through to `unknown` while a squash-stamped one still diff --git a/iris/analysis/origin_funnel.py b/iris/analysis/origin_funnel.py index b3dd246..d231d4a 100644 --- a/iris/analysis/origin_funnel.py +++ b/iris/analysis/origin_funnel.py @@ -60,10 +60,13 @@ def calculate_origin_funnel(metrics: ReportMetrics) -> FunnelResult | None: if the PR read was degraded and acceptance is absent: the "In PR" stage reads acceptance, and every later conversion chains from it. """ - # A failed enrichment omits acceptance, and a failed list or read leaves - # no PRs to match commits to; either way "In PR" would fall back to - # "every commit is in a PR". That default is kept only for PRs absent by - # design (no gh, no GitHub remote), where nothing is marked degraded. + # `pr_enrichment_degraded` is the union over PR states, so it alone cannot + # say whether the PRs acceptance reads were lost; acceptance itself can. + # The aggregator omits it when the merged PRs lost their enrichment, and a + # failed list or read leaves no PRs to match commits to: either way "In + # PR" would fall back to "every commit is in a PR". That default is kept + # only for PRs absent by design (no gh, no GitHub remote), where nothing + # is marked degraded. if metrics.pr_enrichment_degraded and not metrics.acceptance_by_origin: return None diff --git a/iris/cli.py b/iris/cli.py index 9014aad..601c73a 100644 --- a/iris/cli.py +++ b/iris/cli.py @@ -529,6 +529,7 @@ def _tick(label: str) -> None: prs=prs or None, external_data=external_data, pr_fetch_degraded=pr_fetch.degraded, + pr_fetch_degraded_by_state=pr_fetch.degraded_by_state, ) print(s["cli_classified"].format(count=len(commits))) _tick("aggregate analysis done") @@ -559,6 +560,7 @@ def _tick(label: str) -> None: recent_metrics = aggregate( recent_commits, churn_days=args.churn_days, prs=recent_prs, pr_fetch_degraded=pr_fetch.degraded, + pr_fetch_degraded_by_state=pr_fetch.degraded_by_state, ) trend = compute_trend_delta( baseline=metrics, @@ -692,10 +694,12 @@ def _tick(label: str) -> None: pre_metrics = aggregate( pre_commits, churn_days=args.churn_days, prs=pre_prs, pr_fetch_degraded=pr_fetch.degraded, + pr_fetch_degraded_by_state=pr_fetch.degraded_by_state, ) post_metrics = aggregate( post_commits, churn_days=args.churn_days, prs=post_prs, pr_fetch_degraded=pr_fetch.degraded, + pr_fetch_degraded_by_state=pr_fetch.degraded_by_state, ) # Compute day spans from commit date ranges diff --git a/iris/i18n.py b/iris/i18n.py index 511c01a..71cbfa6 100644 --- a/iris/i18n.py +++ b/iris/i18n.py @@ -35,10 +35,10 @@ ), "pr_enrichment_degraded_caveat": ( "PR data is incomplete for this run — failed steps: {steps}. Merge " - "strategy is reported as unknown when enrichment failed, and flow, " - "review and in-PR metrics may be missing or skewed. A transient " - "GitHub error usually clears on the next run; if every run fails, " - "check that gh is authenticated (gh auth status)." + "strategy is reported as unknown when enrichment failed for merged " + "PRs, and flow, review and in-PR metrics may be missing or skewed. A " + "transient GitHub error usually clears on the next run; if every run " + "fails, check that gh is authenticated (gh auth status)." ), "system_disclaimer": ( "Engineering outcomes emerge from system dynamics, domain complexity, " @@ -928,10 +928,10 @@ "pr_enrichment_degraded_caveat": ( "Os dados de PR estão incompletos nesta execução — passos com falha: " "{steps}. A estratégia de merge sai como desconhecida quando o " - "enriquecimento falha, e métricas de flow, de review e de commits em " - "PR podem faltar ou sair distorcidas. Um erro transitório do GitHub " - "costuma sumir na próxima execução; se toda execução falhar, " - "verifique se o gh está autenticado (gh auth status)." + "enriquecimento dos PRs mergeados falha, e métricas de flow, de review " + "e de commits em PR podem faltar ou sair distorcidas. Um erro " + "transitório do GitHub costuma sumir na próxima execução; se toda " + "execução falhar, verifique se o gh está autenticado (gh auth status)." ), "system_disclaimer": ( "Resultados de engenharia emergem de dinâmicas sistêmicas, complexidade do domínio " diff --git a/iris/ingestion/github_reader.py b/iris/ingestion/github_reader.py index a51e264..ab85714 100644 --- a/iris/ingestion/github_reader.py +++ b/iris/ingestion/github_reader.py @@ -465,6 +465,11 @@ def _read_pull_requests_uncached(repo_path: str, days: int) -> PullRequestFetch: return PullRequestFetch( prs=_parse_pull_requests(merged_raw + closed_raw + open_raw, since), degraded=tuple(sorted(merged_degraded | closed_degraded | open_degraded)), + degraded_by_state={ + "merged": tuple(sorted(merged_degraded)), + "closed": tuple(sorted(closed_degraded)), + "open": tuple(sorted(open_degraded)), + }, ) diff --git a/iris/ingestion/window_cache.py b/iris/ingestion/window_cache.py index 57617dc..8f2a8a8 100644 --- a/iris/ingestion/window_cache.py +++ b/iris/ingestion/window_cache.py @@ -72,6 +72,7 @@ def pull_requests( return PullRequestFetch( prs=[pr for pr in fetch.prs if keep(pr, since)], degraded=fetch.degraded, + degraded_by_state=fetch.degraded_by_state, ) fresh = load() diff --git a/iris/metrics/aggregator.py b/iris/metrics/aggregator.py index 7ade07c..caa1357 100644 --- a/iris/metrics/aggregator.py +++ b/iris/metrics/aggregator.py @@ -69,6 +69,7 @@ def aggregate( prs: list[PullRequest] | None = None, external_data: ExternalDORAData | None = None, pr_fetch_degraded: tuple[str, ...] = (), + pr_fetch_degraded_by_state: dict[str, tuple[str, ...]] | None = None, ) -> ReportMetrics: """Run all analyses on commits and return the combined ReportMetrics. @@ -87,18 +88,34 @@ def aggregate( merge strategy is not classified; when it includes ``reviews`` every review-derived field is omitted rather than reported as the 0% / 100% an empty review list would give. + pr_fetch_degraded_by_state: The same steps per PR state + (`PullRequestFetch.degraded_by_state`). Each omission then + follows the state that feeds the field: merged PRs for review, + flow, acceptance and merge strategy, open PRs for staleness; + closed PRs feed only counts, so they omit nothing. Without it + (None or empty) every state takes ``pr_fetch_degraded``. Returns: ReportMetrics with all fields populated. PR fields are None when prs is None or empty. """ - # With the reviews pass failed every PR has no reviews: review-derived - # fields would read as fabricated facts, so they are left out. - reviews_degraded = DEGRADED_REVIEWS in pr_fetch_degraded - # Acceptance and open-PR staleness read the commit pushes (commit_refs) - # that come from the enrichment pass; without them acceptance would read - # 0 commits in PRs and staleness would be inflated. - enrichment_degraded = DEGRADED_ENRICHMENT in pr_fetch_degraded + if pr_fetch_degraded_by_state: + merged_degraded = pr_fetch_degraded_by_state.get("merged", ()) + open_degraded = pr_fetch_degraded_by_state.get("open", ()) + else: + merged_degraded = open_degraded = pr_fetch_degraded + # With the reviews pass failed every merged PR has no reviews: + # review-derived fields would read as fabricated facts, so they are left out. + reviews_degraded = DEGRADED_REVIEWS in merged_degraded + # Acceptance and merge strategy read the commit pushes (commit_refs) that + # come from the enrichment pass; without them acceptance would read + # 0 commits in PRs. + enrichment_degraded = DEGRADED_ENRICHMENT in merged_degraded + # Open-PR staleness is measured from the last review or commit push; + # without either it would read older than it is. + staleness_degraded = ( + DEGRADED_REVIEWS in open_degraded or DEGRADED_ENRICHMENT in open_degraded + ) churn_result = calculate_churn(commits, churn_days) stab_result = calculate_stabilization(commits, churn_days) @@ -389,9 +406,7 @@ def aggregate( open_pr_aging_kwargs["median_open_pr_age_by_intent"] = ( aging_result.median_open_pr_age_by_intent ) - # Staleness is measured from the last review or commit; without - # the reviews or the commit pushes it would read older than it is. - if not reviews_degraded and not enrichment_degraded: + if not staleness_degraded: open_pr_aging_kwargs["stale_open_pr_pct"] = ( aging_result.stale_open_pr_pct ) @@ -401,11 +416,7 @@ def aggregate( open_pr_aging_kwargs["abandonment_risk_pct"] = ( aging_result.abandonment_risk_pct ) - if ( - not reviews_degraded - and not enrichment_degraded - and aging_result.stale_open_pr_pct_by_origin - ): + if not staleness_degraded and aging_result.stale_open_pr_pct_by_origin: open_pr_aging_kwargs["stale_open_pr_pct_by_origin"] = ( aging_result.stale_open_pr_pct_by_origin ) diff --git a/iris/models/pull_request.py b/iris/models/pull_request.py index 98b7a95..baa763f 100644 --- a/iris/models/pull_request.py +++ b/iris/models/pull_request.py @@ -93,7 +93,13 @@ class PullRequestFetch: sorted and without repeats. It is empty when every step succeeded, and also when PRs are absent *by design* (no ``gh`` on the machine, no GitHub remote): Iris works without PRs, and that is not a degradation. + + ``degraded_by_state`` maps each gh state read (``merged``, ``closed``, + ``open``) to the steps that failed for it; ``degraded`` is their union. + Empty when the read never got to the states — absent by design, or the + whole read raised (``fetch``). """ prs: list[PullRequest] degraded: tuple[str, ...] = () + degraded_by_state: dict[str, tuple[str, ...]] = field(default_factory=dict) diff --git a/iris/org_runner.py b/iris/org_runner.py index 5ae2434..a3d6b5c 100644 --- a/iris/org_runner.py +++ b/iris/org_runner.py @@ -90,6 +90,7 @@ def analyze_single_repo( churn_days=churn_days, prs=prs or None, pr_fetch_degraded=pr_fetch.degraded, + pr_fetch_degraded_by_state=pr_fetch.degraded_by_state, ) # Step 4a: Repository kind (keeps docs/board repos out of org comparisons) @@ -109,6 +110,7 @@ def analyze_single_repo( recent_metrics = aggregate( recent_commits, churn_days=churn_days, prs=recent_prs, pr_fetch_degraded=pr_fetch.degraded, + pr_fetch_degraded_by_state=pr_fetch.degraded_by_state, ) trend = compute_trend_delta( baseline=metrics, @@ -147,10 +149,12 @@ def analyze_single_repo( pre_metrics = aggregate( pre_commits, churn_days=churn_days, prs=pre_prs, pr_fetch_degraded=pr_fetch.degraded, + pr_fetch_degraded_by_state=pr_fetch.degraded_by_state, ) post_metrics = aggregate( post_commits, churn_days=churn_days, prs=post_prs, pr_fetch_degraded=pr_fetch.degraded, + pr_fetch_degraded_by_state=pr_fetch.degraded_by_state, ) pre_days = max(1, (pre_commits[-1].date - pre_commits[0].date).days) diff --git a/tests/test_pr_fetch_degradation.py b/tests/test_pr_fetch_degradation.py index 40d902e..b0204ed 100644 --- a/tests/test_pr_fetch_degradation.py +++ b/tests/test_pr_fetch_degradation.py @@ -343,6 +343,13 @@ def test_degraded_steps_from_every_state_are_merged_and_sorted(monkeypatch): fetch = github_reader.read_pull_requests("/any/repo", days=30) assert fetch.degraded == ("enrichment", "reviews") + # Each state keeps its own steps, so a metric can be gated on the state + # that feeds it. + assert fetch.degraded_by_state == { + "merged": ("enrichment", "reviews"), + "closed": (), + "open": ("enrichment",), + } # --- the signal in the metrics --------------------------------------------- @@ -1042,3 +1049,111 @@ def test_cli_metrics_json_keeps_the_funnel_on_a_clean_run(tmp_path, monkeypatch) tmp_path, monkeypatch, PullRequestFetch(prs=[], degraded=())) assert "origin_funnel" in payload + + +# --- the gate follows the PR state that feeds each metric ------------------- +# +# A step can fail for one PR state only. Review, flow, acceptance and merge +# strategy read merged PRs; staleness reads open PRs; closed PRs feed only the +# basic list. A failure in one state leaves the metrics of the others alone, +# while `pr_enrichment_degraded` still lists the union. + + +def _per_state(**steps) -> dict[str, tuple[str, ...]]: + return {"merged": (), "closed": (), "open": (), **steps} + + +def _payload(degraded, by_state=None) -> dict: + return aggregate( + _stamped_commits(), churn_days=14, prs=_reviewed_prs() + _open_prs(), + pr_fetch_degraded=degraded, pr_fetch_degraded_by_state=by_state, + ).to_dict() + + +def test_reviews_degraded_only_for_open_prs_omits_only_staleness(): + clean = _payload(()) + payload = _payload(("reviews",), _per_state(open=("reviews",))) + + assert payload.pop("pr_enrichment_degraded") == ["reviews"] + assert payload["pr_single_pass_rate"] == clean["pr_single_pass_rate"] + assert payload["flow_efficiency_median"] == clean["flow_efficiency_median"] + for key in _STALENESS_KEYS: + assert key not in payload, key + clean.pop(key, None) + assert payload == clean + + +def test_enrichment_degraded_only_for_closed_prs_omits_nothing(): + clean = _payload(()) + payload = _payload(("enrichment",), _per_state(closed=("enrichment",))) + + assert payload.pop("pr_enrichment_degraded") == ["enrichment"] + assert payload["merge_strategy"] != "unknown" + assert payload["flow_efficiency_median"] == clean["flow_efficiency_median"] + assert payload["acceptance_by_origin"] == clean["acceptance_by_origin"] + assert payload == clean + + +def test_enrichment_degraded_only_for_merged_prs_keeps_staleness(): + payload = _payload(("enrichment",), _per_state(merged=("enrichment",))) + + assert payload["merge_strategy"] == "unknown" + assert "acceptance_by_origin" not in payload + for key in _STALENESS_KEYS: + if key != "stale_open_pr_pct_by_origin": + assert key in payload, key + + +def test_origin_funnel_follows_the_merged_enrichment(): + def funnel(by_state): + return calculate_origin_funnel(aggregate( + _commits_with_ai(), churn_days=14, prs=_reviewed_prs(), + pr_fetch_degraded=("enrichment",), pr_fetch_degraded_by_state=by_state, + )) + + assert funnel(_per_state(open=("enrichment",))) is not None + assert funnel(_per_state(merged=("enrichment",))) is None + + +@pytest.mark.parametrize("entry", ["cli", "org_runner"]) +def test_every_aggregate_call_gets_the_per_state_degradation(tmp_path, monkeypatch, entry): + from iris import cli, org_runner + + repo = tmp_path / "repo" + repo.mkdir() + _build_repo(repo) + fetch = PullRequestFetch( + prs=_reviewed_prs(), degraded=("reviews",), + degraded_by_state=_per_state(open=("reviews",)), + ) + module = cli if entry == "cli" else org_runner + monkeypatch.setattr(module, "read_pull_requests_with_fallback", lambda *a, **k: fetch) + _adoption_after_first_commit(monkeypatch) + calls: list[dict] = [] + real_aggregate = module.aggregate + + def spy(*args, **kwargs): + calls.append(kwargs) + return real_aggregate(*args, **kwargs) + + monkeypatch.setattr(module, "aggregate", spy) + + if entry == "cli": + cli._run_single_repo(argparse.Namespace( + repo_path=str(repo), days=30, churn_days=14, lang="en", recent_days=30, + verbose=False, trend=True, out=str(tmp_path / "out"), no_push=True, + )) + else: + org_runner.analyze_single_repo( + str(repo), days=30, churn_days=14, out_dir=str(tmp_path / "out"), + trend_enabled=True, + ) + + # Main run, trend window, pre- and post-adoption. + assert len(calls) == 4 + for kwargs in calls: + assert kwargs["pr_fetch_degraded"] == ("reviews",) + assert kwargs["pr_fetch_degraded_by_state"] == fetch.degraded_by_state + payload = _metrics_json(tmp_path / "out") + assert payload["pr_enrichment_degraded"] == ["reviews"] + assert "pr_single_pass_rate" in payload diff --git a/tests/test_window_cache.py b/tests/test_window_cache.py index 1c9cef7..83d192f 100644 --- a/tests/test_window_cache.py +++ b/tests/test_window_cache.py @@ -111,7 +111,10 @@ def load(): def test_a_narrower_window_keeps_the_wide_fetch_degradation(): def load(): - return PullRequestFetch(prs=[_pr(80), _pr(3)], degraded=("enrichment",)) + return PullRequestFetch( + prs=[_pr(80), _pr(3)], degraded=("enrichment",), + degraded_by_state={"merged": ("enrichment",), "closed": (), "open": ()}, + ) window_cache.enable() window_cache.pull_requests("r", 90, load, _keep) @@ -120,6 +123,7 @@ def load(): narrow = window_cache.pull_requests("r", 7, load, _keep) assert len(narrow.prs) == 1 assert narrow.degraded == ("enrichment",) + assert narrow.degraded_by_state == {"merged": ("enrichment",), "closed": (), "open": ()} # --- the overlap predicate (single source of truth for the filter) --------- From bc19f2192f9834642ef4d2d19dca33e368a6f1a7 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 20:54:16 -0300 Subject: [PATCH 16/22] fix(engine): omit what a failed PR list feeds, per state When `gh pr list` failed for one state while the others were read, the PR list was not empty, so everything the missing state feeds read as a real zero: with the merged list gone, acceptance showed 0% for every group, the activity timeline 0 PRs merged a week, and merge strategy "unknown" with reliable commit metrics; with any list gone, Flow Load undercounted the WIP. A missing merged list now omits what merged PRs feed (PR lifecycle, review, Flow Efficiency, human coverage, acceptance, merge strategy) and leaves the timeline's prs_merged unknown; a missing open list omits every open-PR field; a missing list in any state omits Flow Load. Without the per-state map the union applies, and `fetch` counts as every list failed. Clean runs are unchanged. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 --- iris/metrics/aggregator.py | 55 +++++++++--- tests/test_pr_fetch_degradation.py | 137 +++++++++++++++++++++++++++++ 2 files changed, 179 insertions(+), 13 deletions(-) diff --git a/iris/metrics/aggregator.py b/iris/metrics/aggregator.py index caa1357..279cc38 100644 --- a/iris/metrics/aggregator.py +++ b/iris/metrics/aggregator.py @@ -42,7 +42,13 @@ from iris.models.commit import Commit from iris.models.external import ExternalDORAData from iris.models.metrics import ReportMetrics -from iris.models.pull_request import DEGRADED_ENRICHMENT, DEGRADED_REVIEWS, PullRequest +from iris.models.pull_request import ( + DEGRADED_BASIC, + DEGRADED_ENRICHMENT, + DEGRADED_FETCH, + DEGRADED_REVIEWS, + PullRequest, +) logger = logging.getLogger(__name__) @@ -92,8 +98,13 @@ def aggregate( (`PullRequestFetch.degraded_by_state`). Each omission then follows the state that feeds the field: merged PRs for review, flow, acceptance and merge strategy, open PRs for staleness; - closed PRs feed only counts, so they omit nothing. Without it - (None or empty) every state takes ``pr_fetch_degraded``. + closed PRs feed only Flow Load, which every state feeds. A + state whose list (``basic``) failed is missing for everything + it feeds: merged for the PR lifecycle, review, flow, coverage, + acceptance, merge strategy and the timeline's ``prs_merged``; + open for every open-PR field; any state for Flow Load. Without + it (None or empty) every state takes ``pr_fetch_degraded``, + where ``fetch`` (the whole read raised) means every list failed. Returns: ReportMetrics with all fields populated. PR fields are None @@ -102,8 +113,21 @@ def aggregate( if pr_fetch_degraded_by_state: merged_degraded = pr_fetch_degraded_by_state.get("merged", ()) open_degraded = pr_fetch_degraded_by_state.get("open", ()) + every_state_degraded = list(pr_fetch_degraded_by_state.values()) else: merged_degraded = open_degraded = pr_fetch_degraded + every_state_degraded = [pr_fetch_degraded] + # A state whose list failed (`basic`) has no PRs here while the others + # do; with `fetch` (the whole read raised, so only the union carries it) + # no state has any. Whatever a missing state feeds would read as a real + # zero — 0% acceptance, 0 PRs merged a week, a WIP short of its PRs — so + # it is left out, as with no PRs at all. + list_failed = {DEGRADED_BASIC, DEGRADED_FETCH} + merged_missing = not list_failed.isdisjoint(merged_degraded) + open_missing = not list_failed.isdisjoint(open_degraded) + any_list_missing = any( + not list_failed.isdisjoint(steps) for steps in every_state_degraded + ) # With the reviews pass failed every merged PR has no reviews: # review-derived fields would read as fabricated facts, so they are left out. reviews_degraded = DEGRADED_REVIEWS in merged_degraded @@ -288,9 +312,12 @@ def aggregate( for c in churn_detail.couplings ] - # Activity timeline + # Activity timeline. Without the merged list, `prs_merged` per week is + # unknown (None), not 0. timeline_kwargs: dict = {} - timeline_result = calculate_activity_timeline(commits, churn_days, prs=prs) + timeline_result = calculate_activity_timeline( + commits, churn_days, prs=None if merged_missing else prs, + ) if timeline_result: timeline_kwargs["activity_timeline"] = [ { @@ -319,7 +346,7 @@ def aggregate( # Acceptance rate (requires PR data with commit hashes) acceptance_kwargs: dict = {} - if prs and not enrichment_degraded: + if prs and not merged_missing and not enrichment_degraded: acceptance_result = calculate_acceptance_rate(commits, prs) if acceptance_result: if acceptance_result.by_origin: @@ -335,7 +362,7 @@ def aggregate( # Its phases anchor on the first review: with the reviews read failed, # every PR would count as never reviewed, so the block is left out. flow_efficiency_kwargs: dict = {} - if prs and not reviews_degraded: + if prs and not merged_missing and not reviews_degraded: flow_efficiency_result = analyze_flow_efficiency( prs, commit_origin_map=origin_map, @@ -365,7 +392,7 @@ def aggregate( # Human Review Coverage — fraction of merged PRs a human actually reviewed human_review_coverage_kwargs: dict = {} - if prs and not reviews_degraded: + if prs and not merged_missing and not reviews_degraded: coverage_result = analyze_human_review_coverage( prs, commit_origin_map=origin_map, @@ -388,7 +415,7 @@ def aggregate( # Open PR Aging — snapshot of stuck inventory (non-draft, non-bot) open_pr_aging_kwargs: dict = {} - if prs: + if prs and not open_missing: aging_result = analyze_open_pr_aging( prs, now=now_utc(), @@ -426,7 +453,7 @@ def aggregate( # PR ground truth (merge_commit_parent_count) and commit_refs with the # local main history. Strictly per-repository — no author axis. merge_strategy_kwargs: dict = {} - if prs: + if prs and not merged_missing: merge_strategy_result = detect_merge_strategy( prs, commits, @@ -447,9 +474,11 @@ def aggregate( merge_strategy_result.distribution, ) - # Flow Load — WIP per ISO week (PRs in flight + author concurrency) + # Flow Load — WIP per ISO week (PRs in flight + author concurrency). + # Every PR state is in flight at some point, so a missing list in any of + # them would undercount the WIP. flow_load_kwargs: dict = {} - flow_load_result = analyze_flow_load(prs or [], commits) + flow_load_result = None if any_list_missing else analyze_flow_load(prs or [], commits) if flow_load_result: flow_load_kwargs["flow_load"] = [ { @@ -465,7 +494,7 @@ def aggregate( # PR lifecycle (optional — only over merged PRs) pr_kwargs: dict = {} - if prs: + if prs and not merged_missing: pr_result = analyze_pr_lifecycle(prs) if pr_result is not None: pr_kwargs = { diff --git a/tests/test_pr_fetch_degradation.py b/tests/test_pr_fetch_degradation.py index b0204ed..a40fcff 100644 --- a/tests/test_pr_fetch_degradation.py +++ b/tests/test_pr_fetch_degradation.py @@ -1157,3 +1157,140 @@ def spy(*args, **kwargs): payload = _metrics_json(tmp_path / "out") assert payload["pr_enrichment_degraded"] == ["reviews"] assert "pr_single_pass_rate" in payload + + +# --- a failed list omits what its state feeds ------------------------------- +# +# When `gh pr list` fails for a state, that state's PRs are missing while the +# others are read: the PR list is not empty, so every metric the missing state +# feeds would read as a real zero (0% acceptance, 0 PRs merged a week, a WIP +# short of the PRs in flight). Those are left out instead. + +# Fed by merged PRs: lifecycle, review, flow, coverage, acceptance, strategy. +_MERGED_FED_KEYS = ( + "pr_merged_count", + "pr_median_time_to_merge_hours", + "pr_mean_time_to_merge_hours", + "pr_p90_time_to_merge_hours", + "pr_pct_merged_within_24h", + "pr_cycle_time_buckets", + "pr_median_size_files", + "pr_median_size_lines", + *_REVIEW_KEYS, + *_FLOW_KEYS, + "acceptance_by_origin", + "acceptance_by_tool", + "merge_strategy", + "merge_strategy_dominant_share", + "commit_metrics_reliable", +) +_OPEN_FED_KEYS = (*_AGE_KEYS, "median_open_pr_age_by_intent", *_STALENESS_KEYS) + + +def _closed_prs() -> list[PullRequest]: + return [ + PullRequest(number=200 + n, title=f"closed {n}", author="dev", + created_at=_NOW, additions=1, deletions=0, changed_files=1, + closed_at=_NOW + timedelta(hours=n), state="closed", + commit_refs=[CommitRef(hash=f"c{n}", committed_at=_NOW)]) + for n in range(1, 4) + ] + + +def _two_week_commits() -> list[Commit]: + # The activity timeline needs at least two ISO weeks of commits. + later = Commit(hash="c9", author="dev", date=_NOW + timedelta(days=8), + message="change 9 (#9)") + return _stamped_commits() + [later] + + +def _list_payload(prs, degraded, by_state) -> dict: + return aggregate( + _two_week_commits(), churn_days=14, prs=prs, + pr_fetch_degraded=degraded, pr_fetch_degraded_by_state=by_state, + ).to_dict() + + +def test_missing_merged_list_omits_what_merged_prs_feed(): + payload = _list_payload( + _closed_prs() + _open_prs(), ("basic",), _per_state(merged=("basic",))) + + for key in _MERGED_FED_KEYS: + assert key not in payload, key + assert "flow_load" not in payload + assert payload["activity_timeline"] + for week in payload["activity_timeline"]: + assert week["prs_merged"] is None + assert week["pr_median_ttm_hours"] is None + # The open list was read: its fields stay. + for key in _AGE_KEYS: + assert key in payload, key + + +def test_missing_open_list_omits_every_open_pr_field(): + clean = _list_payload(_reviewed_prs() + _closed_prs(), (), None) + payload = _list_payload( + _reviewed_prs() + _closed_prs(), ("basic",), _per_state(open=("basic",))) + + for key in _OPEN_FED_KEYS: + assert key not in payload, key + assert "flow_load" not in payload + # The merged list was read: its fields stay as on a clean run. + assert payload["pr_merged_count"] == 6 + assert payload["acceptance_by_origin"] == clean["acceptance_by_origin"] + assert payload["activity_timeline"] == clean["activity_timeline"] + + +def test_missing_closed_list_omits_only_flow_load(): + prs = _reviewed_prs() + _closed_prs() + _open_prs() + clean = _list_payload(prs, (), None) + payload = _list_payload(prs, ("basic",), _per_state(closed=("basic",))) + + assert clean["flow_load"] + assert payload.pop("pr_enrichment_degraded") == ["basic"] + clean.pop("flow_load") + assert payload == clean + + +@pytest.mark.parametrize("by_state", [None, {}]) +def test_missing_list_without_the_per_state_dict_omits_every_state(by_state): + payload = _list_payload( + _reviewed_prs() + _closed_prs() + _open_prs(), ("basic",), by_state) + + for key in (*_MERGED_FED_KEYS, *_OPEN_FED_KEYS, "flow_load"): + assert key not in payload, key + for week in payload["activity_timeline"]: + assert week["prs_merged"] is None + + +@pytest.mark.parametrize("degraded, by_state", [ + (("fetch",), {}), + (("basic",), _per_state(merged=("basic",), closed=("basic",), open=("basic",))), +]) +def test_read_that_lost_every_list_leaves_no_pr_zeros(degraded, by_state): + payload = _list_payload([], degraded, by_state) + + assert "flow_load" not in payload + for week in payload["activity_timeline"]: + assert week["prs_merged"] is None + + +@pytest.mark.parametrize("prs", [None, []]) +def test_prs_absent_by_design_keep_flow_load(prs): + # No gh or no GitHub remote: nothing failed, so nothing is left out. + payload = _list_payload(prs, (), None) + + assert "flow_load" in payload + + +# --- without the per-state dict, the union applies --------------------------- + + +@pytest.mark.parametrize("by_state", [None, {}]) +def test_reviews_without_the_per_state_dict_omit_merged_and_open_metrics(by_state): + payload = _payload(("reviews",), by_state) + + for key in (*_REVIEW_KEYS, *_FLOW_KEYS, *_STALENESS_KEYS): + assert key not in payload, key + for key in _AGE_KEYS: + assert key in payload, key From b8284dbc00e2d8cca40947a5d872243ee3412c4f Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 20:54:36 -0300 Subject: [PATCH 17/22] fix(engine): anchor transient gh error detection The transient-error check matched anywhere in gh's stderr: "EOF" matched "thereof", and "timeout" matched any error about a repo named like timeout-svc, so a deterministic failure there was retried with the pause on every page. The repo's owner/name, name and owner are now blanked out of stderr first (only where they stand alone, so a repo named "out" leaves "timeout" whole), and the patterns match whole words: HTTP 5xx, timeout/timed out, deadline exceeded, connection reset/refused, EOF. GitHub's GraphQL "may be the result of a timeout" error is still retried. Also refresh the enrichment docstring, which still said a page failed "even after one retry". Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 --- iris/ingestion/github_reader.py | 52 ++++++++++++++++++++------- tests/test_pr_fetch_degradation.py | 58 ++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 13 deletions(-) diff --git a/iris/ingestion/github_reader.py b/iris/ingestion/github_reader.py index ab85714..fe5d712 100644 --- a/iris/ingestion/github_reader.py +++ b/iris/ingestion/github_reader.py @@ -212,18 +212,41 @@ def _fetch_prs(nwo: str, limit: int, gh_state: str) -> tuple[list[dict], set[str # retry makes a degraded run rarer without hiding the ones that still fail. _RETRY_DELAY_S = 2.0 -# gh writes why it failed to stderr. A 5xx or a slow or dropped connection can -# pass on a second try; anything else — a 4xx, a GraphQL error — fails the same -# way again, so retrying it would only add the pause to every run. +# gh writes why it failed to stderr. A 5xx, a timeout (GitHub's GraphQL "may +# be the result of a timeout" error included) or a dropped connection can pass +# on a second try; anything else — a 4xx or other GraphQL error — fails the +# same way again, so retrying it would only add the pause to every run. Whole +# words only, so "thereof" is not an EOF. _TRANSIENT_GH_ERROR = re.compile( - r"HTTP 5\d\d|timeout|timed out|deadline exceeded|connection reset" - r"|connection refused|EOF", + r"\bHTTP 5\d\d\b|\btime(?:d)? ?out\b|\bdeadline exceeded\b" + r"|\bconnection (?:reset|refused)\b|\bEOF\b", re.IGNORECASE, ) -def _run_gh_with_one_retry(args: list[str]) -> subprocess.CompletedProcess | None: - """Run a gh command; if it fails transiently, wait once and retry. +def _is_transient_gh_error(stderr: str, nwo: str) -> bool: + """Whether gh's stderr reads as an error a second try could clear. + + The repo's ``owner/name``, name and owner are blanked out first: gh echoes + them in its errors, and a repo named ``timeout-svc`` would otherwise make + any error about it look like a timeout. + """ + owner, _, name = nwo.partition("/") + for part in (nwo, name, owner): + if part: + # Only where it stands alone, so a repo named "out" leaves + # "timeout" whole. + stderr = re.sub( + rf"(? subprocess.CompletedProcess | None: + """Run a gh command for ``nwo``; if it fails transiently, wait once and retry. None when the command fails twice, or fails once with an error that a retry would not fix. @@ -237,7 +260,7 @@ def _run_gh_with_one_retry(args: list[str]) -> subprocess.CompletedProcess | Non except FileNotFoundError: return None except subprocess.CalledProcessError as error: - if attempt == 2 or not _TRANSIENT_GH_ERROR.search(error.stderr or ""): + if attempt == 2 or not _is_transient_gh_error(error.stderr or "", nwo): return None time.sleep(_RETRY_DELAY_S) return None @@ -266,9 +289,12 @@ def _fetch_pr_enrichment_graphql( of. Returns ``(by_pr, complete)``. ``complete`` is False when the pass stopped - on an error — a page that failed even after one retry, unparseable - output, a response without data — so ``by_pr`` holds only what earlier - pages collected. Reaching ``max_prs`` or the last page is a normal end. + on an error — a page whose gh call failed (at once on an error a retry + would not fix, after the one retry on a transient one), unparseable + output, a response without data, more pages with no cursor to reach + them — so ``by_pr`` holds only what earlier pages collected, and the + pass counts as failed even when those pages came back. Reaching + ``max_prs`` or the last page is a normal end. """ try: owner, name = nwo.split("/", 1) @@ -297,7 +323,7 @@ def _fetch_pr_enrichment_graphql( if end_cursor: args.extend(["-f", f"cursor={end_cursor}"]) - result = _run_gh_with_one_retry(args) + result = _run_gh_with_one_retry(args, nwo) if result is None: return by_pr, False @@ -375,7 +401,7 @@ def _gh_pr_list( "--limit", str(limit), ] if retry: - result = _run_gh_with_one_retry(args) + result = _run_gh_with_one_retry(args, nwo) else: try: result = subprocess.run( diff --git a/tests/test_pr_fetch_degradation.py b/tests/test_pr_fetch_degradation.py index a40fcff..4d6b8ee 100644 --- a/tests/test_pr_fetch_degradation.py +++ b/tests/test_pr_fetch_degradation.py @@ -189,6 +189,64 @@ def test_enrichment_does_not_retry_a_deterministic_error(monkeypatch, stderr): assert sleeps == [] +# A repo whose name looks like a transient error must not make a +# deterministic error look transient, and must not hide a real one. +_TIMEOUT_REPO = "acme/timeout-svc" + + +@pytest.mark.parametrize("stderr", [ + "GraphQL: Could not resolve to a Repository with the name 'acme/timeout-svc'. (repository)", + "HTTP 404: Not Found (https://api.github.com/repos/acme/timeout-svc/pulls)", + "no pull requests match your search in timeout-svc", + "GraphQL: Field 'oid' is not defined on any part thereof (query)", +]) +def test_enrichment_does_not_read_the_repo_name_as_a_transient_error(monkeypatch, stderr): + sleeps = _fake_gh(monkeypatch, graphql=[ + subprocess.CalledProcessError(1, ["gh"], stderr=stderr), + ]) + + _by_pr, complete = github_reader._fetch_pr_enrichment_graphql(_TIMEOUT_REPO, "merged", 500) + + assert complete is False + assert sleeps == [] + + +@pytest.mark.parametrize("nwo, stderr", [ + (_TIMEOUT_REPO, "HTTP 504: Gateway Timeout (https://api.github.com/graphql)"), + (_TIMEOUT_REPO, "Post \"https://api.github.com/graphql\": unexpected EOF"), + ("acme/out", "dial tcp: i/o timeout"), + ("acme/out", "read: connection reset by peer"), + ("acme/widgets", "GraphQL: Something went wrong while executing your query. " + "This may be the result of a timeout, or it could be a GitHub bug."), +]) +def test_enrichment_still_retries_a_transient_error(monkeypatch, nwo, stderr): + sleeps = _fake_gh(monkeypatch, graphql=[ + subprocess.CalledProcessError(1, ["gh"], stderr=stderr), _graphql_page([1]), + ]) + + by_pr, complete = github_reader._fetch_pr_enrichment_graphql(nwo, "merged", 500) + + assert sorted(by_pr) == [1] + assert complete is True + assert sleeps == [github_reader._RETRY_DELAY_S] + + +def test_reviews_pass_does_not_read_the_repo_name_as_a_transient_error(monkeypatch): + sleeps = _fake_gh( + monkeypatch, + basic=[{"number": 1}], + graphql=[_graphql_page([1])], + reviews=(subprocess.CalledProcessError( + 1, ["gh"], stderr="GraphQL: Could not resolve to a Repository with " + "the name 'acme/timeout-svc'. (repository)"),), + ) + + _prs, degraded = github_reader._fetch_prs(_TIMEOUT_REPO, 1000, "merged") + + assert degraded == {"reviews"} + assert sleeps == [] + + def test_reviews_pass_retries_a_transient_failure_and_recovers(monkeypatch): review = {"author": {"login": "reviewer"}, "state": "APPROVED", "submittedAt": "2026-09-01T00:00:00Z"} From 2716615b27b6b323396a8b65efdbfc24bbdfe580 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 20:56:00 -0300 Subject: [PATCH 18/22] docs(engine): align METRICS.md with per-state degradation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The nullable-when cells said "`reviews` degraded" or "`enrichment` degraded" while the prose already gated each field on the PR state that feeds it; the cells now carry that state, and the ones a failed list omits say so (acceptance, funnel, timeline, PR lifecycle, Flow Load, Flow Efficiency, coverage, open PR aging, merge strategy). §15 documents the failed-list rule, defines a stopped enrichment pass as failed even when earlier pages came back, and says a 4xx or other GraphQL error is not retried (GitHub's "may be the result of a timeout" error is). Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 --- docs/METRICS.md | 110 +++++++++++++++++++++++++++++------------------- 1 file changed, 67 insertions(+), 43 deletions(-) diff --git a/docs/METRICS.md b/docs/METRICS.md index e181bdc..f2c0694 100644 --- a/docs/METRICS.md +++ b/docs/METRICS.md @@ -236,8 +236,8 @@ its PR (if any) and aggregated by origin and AI tool. | Field | Unit | Source | Nullable when | |---|---|---|---| -| `acceptance_by_origin` | `Record` | `analysis/acceptance_rate.py` | no PR data, or < 5 commits per origin, or `enrichment` degraded | -| `acceptance_by_tool` | `Record` | same | < 5 commits per tool, or `enrichment` degraded | +| `acceptance_by_origin` | `Record` | `analysis/acceptance_rate.py` | no PR data, or < 5 commits per origin, or `basic` or `enrichment` failed for merged PRs | +| `acceptance_by_tool` | `Record` | same | < 5 commits per tool, or `basic` or `enrichment` failed for merged PRs | `AcceptanceMetrics`: `total_commits`, `commits_in_prs`, `pr_rate` (commits in PRs / total), `single_pass_rate` (PRs merged with @@ -245,6 +245,8 @@ zero `CHANGES_REQUESTED` / PRs), `median_review_rounds`. The last two are omitte for merged PRs (§15). Both maps are omitted entirely when `enrichment` failed for merged PRs: commits are matched to PRs through the commit refs that pass reads, so without them every group would read 0 commits in PRs. +The same goes when the merged list itself failed (`basic`): with no merged PRs +to match, every group would read 0 too. --- @@ -256,13 +258,14 @@ dataclass is always `None`; the field only exists in the emitted JSON. | Field | Unit | Source | Nullable when | |---|---|---|---| -| `origin_funnel` | `Record` | `analysis/origin_funnel.py` (via `reports/writer.py`) | no `commit_origin_distribution`, or `pr_enrichment_degraded` present and `acceptance_by_origin` absent | +| `origin_funnel` | `Record` | `analysis/origin_funnel.py` (via `reports/writer.py`) | no `commit_origin_distribution`, or `pr_enrichment_degraded` present and `acceptance_by_origin` absent (`basic` or `enrichment` failed for merged PRs, or `fetch`) | The whole funnel is omitted when `pr_enrichment_degraded` (§15) is present and `acceptance_by_origin` is absent: its `In PR` stage reads acceptance, and every -later conversion chains from that stage. That covers a failed `enrichment` for -merged PRs, which omits acceptance (§9), and a read that lost the PRs — `basic` -failed for every state, or `fetch` — which leaves no PRs to match commits to. +later conversion chains from that stage. That covers a failed `basic` or +`enrichment` for merged PRs, which omits acceptance (§9), and `fetch`, which +leaves no PRs to match commits to. A failure confined to closed or open PRs +keeps acceptance, and the funnel with it. Only when PRs are absent by design (no `gh`, no GitHub remote; nothing degraded) does `In PR` fall back to counting every commit as in a PR. @@ -344,7 +347,9 @@ weeks. `ActivityWeek`: `week_start`/`week_end` (ISO date), `commits`, `lines_changed`, `intent` (distribution), `origin` (distribution), `stabilization_ratio` (or `None` for weeks < 3 commits), `churn_events`, -`prs_merged`, `pr_median_ttm_hours`. +`prs_merged`, `pr_median_ttm_hours` (both `None` with no PR data, or when the +merged list failed — `basic` for merged PRs, or `fetch` — rather than 0 PRs +merged; §15). `ActivityPattern`: `pattern` (one of `burst_then_fix`, `quiet_period`, `ai_ramp`, `intent_shift`), `week` (`MM/DD`), `description`. @@ -363,7 +368,7 @@ All fields require GitHub PR data. | Field | Unit | Source | Nullable when | |---|---|---|---| -| `pr_merged_count` | int ≥ 0 | `analysis/pr_lifecycle.py` | no PR data | +| `pr_merged_count` | int ≥ 0 | `analysis/pr_lifecycle.py` | no PR data, or `basic` failed for merged PRs | | `pr_median_time_to_merge_hours` | float ≥ 0 | same | same | | `pr_mean_time_to_merge_hours` | float ≥ 0 | same | same | | `pr_p90_time_to_merge_hours` | float ≥ 0 | same | same | @@ -371,8 +376,8 @@ All fields require GitHub PR data. | `pr_cycle_time_buckets` | `{same_day, one_day, two_to_three_days, four_to_seven_days, seven_plus_days}` ints | same | same | | `pr_median_size_files` | int ≥ 0 | same | same | | `pr_median_size_lines` | int ≥ 0 | same | same | -| `pr_review_rounds_median` | float ≥ 0 | same | same, or `reviews` degraded | -| `pr_single_pass_rate` | float `0.0–1.0` | same | same, or `reviews` degraded | +| `pr_review_rounds_median` | float ≥ 0 | same | same, or `reviews` failed for merged PRs | +| `pr_single_pass_rate` | float `0.0–1.0` | same | same, or `reviews` failed for merged PRs | | `pr_enrichment_degraded` | list of `basic\|enrichment\|reviews\|fetch` | `ingestion/github_reader.py` | every read step succeeded, or PRs are absent by design | `pr_review_rounds_median` — median count of `CHANGES_REQUESTED` reviews @@ -390,18 +395,21 @@ without persisting individual PR durations. `pr_enrichment_degraded` — the steps of the GitHub PR read that failed this run and fell back to partial data: `basic` (`gh pr list` for one state failed, so that state's PRs are missing), `enrichment` (the GraphQL pass for commit -refs and merge-commit parents failed), `reviews` (the reviews pass failed), -`fetch` (the whole read raised). The enrichment and reviews passes get one -retry when gh reports a transient error (HTTP 5xx, a timeout, a dropped -connection); a 4xx or GraphQL error is not retried. Sorted, without repeats. Unlike -the fields above it does not require PR data: a read that lost every PR still -reports why. Absent when every step succeeded, and when PRs are absent by -design — no `gh`, no GitHub remote — because Iris works without PRs. When -present, this run's PR-derived fields may be missing or skewed — flow and -`commits_in_prs` go missing — and `merge_strategy` is `unknown` if -`enrichment` failed (§28). With `reviews` failed every PR carries no reviews, -so the fields derived from them are omitted rather than reported as the 0% -coverage and 100% single-pass an empty review list would give: +refs and merge-commit parents failed; a pass that stops on any page counts as +failed, even when earlier pages came back), `reviews` (the reviews pass +failed), `fetch` (the whole read raised). The enrichment and reviews passes +get one retry when gh reports a transient error (HTTP 5xx, a timeout — +GitHub's GraphQL "may be the result of a timeout" error included — or a +dropped connection); a 4xx or other GraphQL error is not retried. Sorted, +without repeats. Unlike the fields above it does not require PR data: a read +that lost every PR still reports why. Absent when every step succeeded, and +when PRs are absent by design — no `gh`, no GitHub remote — because Iris works +without PRs. When present, this run's PR-derived fields may be missing or +skewed — flow and `commits_in_prs` go missing — and `merge_strategy` is +`unknown` if `enrichment` failed for merged PRs (§28). With `reviews` failed +every PR carries no reviews, so the fields derived from them are omitted +rather than reported as the 0% coverage and 100% single-pass an empty review +list would give: `pr_review_rounds_median`, `pr_single_pass_rate`, `median_time_to_first_review_hours`, `human_review_coverage_pct`, `human_approval_coverage_pct`, `human_review_coverage_by_intent`, @@ -416,16 +424,30 @@ fields are omitted then too, and `acceptance_by_origin` and `acceptance_by_tool` (§9) are omitted whole, since they match commits to PRs through those same refs. Open PR count and ages (`open_pr_count`, `median_open_pr_age_days`, `p90_open_pr_age_days`, `median_open_pr_age_by_intent`) are kept, since age -depends only on the creation time. It is the first field to read when two runs on the -same commit disagree. +depends only on the creation time — unless the open list itself failed (see +below). It is the first field to read when two runs on the same commit +disagree. The read fetches merged, closed and open PRs separately, and a step can fail for one state only, so each omission above follows the state that feeds the field. The review fields, Flow Efficiency, acceptance (§9) and merge strategy (§28) read merged PRs: only a failure for merged PRs omits them. Staleness reads open PRs: only a failure for open PRs omits it. Closed PRs feed only -counts (Flow Load, §24), so a failure there omits nothing. -`pr_enrichment_degraded` still lists the union over the three states. +Flow Load (§24), and only through their list, so a failed enrichment or +reviews pass there omits nothing. `pr_enrichment_degraded` still lists the +union over the three states. + +When a state's list failed (`basic`), its PRs are missing while the other +states' are read, so whatever it feeds would read as a real zero; those +fields are omitted instead, as on a run with no PR data. A failed merged list +omits the PR lifecycle fields above, the review fields, Flow Efficiency (§25), +Human Review Coverage (§26), acceptance (§9) and merge strategy (§28), and +leaves the activity timeline's `prs_merged` and `pr_median_ttm_hours` `None` +(§14). A failed open list omits every Open PR Aging field (§27), count and +ages included. Flow Load (§24) counts PRs of every state in flight, so a +failed list in any state omits it. `fetch` counts as every list failed. When +the per-state breakdown is unavailable, a step in the union applies to every +state. --- @@ -597,7 +619,7 @@ proxy. | Field | Unit | Source | Nullable when | |---|---|---|---| -| `flow_load` | `FlowLoadWeek[]` | `analysis/flow_load.py` | < 2 weeks of signal | +| `flow_load` | `FlowLoadWeek[]` | `analysis/flow_load.py` | < 2 weeks of signal, or `basic` failed for any PR state, or `fetch` (§15) | `FlowLoadWeek`: @@ -663,11 +685,11 @@ say nothing about whether work is flowing or queued in a different shape. | Field | Unit | Source | Nullable when | |---|---|---|---| -| `flow_efficiency_median` | float `0.0–1.0` | `analysis/flow_efficiency.py` | no merged PR survives the filters, or `reviews` degraded | -| `median_time_to_first_review_hours` | float ≥ 0 | same | no merged PR had a review, or `reviews` degraded | +| `flow_efficiency_median` | float `0.0–1.0` | `analysis/flow_efficiency.py` | no merged PR survives the filters, or `basic` or `reviews` failed for merged PRs | +| `median_time_to_first_review_hours` | float ≥ 0 | same | no merged PR had a review, or `basic` or `reviews` failed for merged PRs | | `time_in_phase_median_hours` | `Record` | same | same as `flow_efficiency_median` | -| `flow_efficiency_by_intent` | `Record` | same | < `min_sample` (default 10) PRs in the segment, or `reviews` degraded | -| `flow_efficiency_by_origin` | `Record` | same | no `commit_origin_map` provided, or < `min_sample` PRs in the segment, or `reviews` degraded | +| `flow_efficiency_by_intent` | `Record` | same | < `min_sample` (default 10) PRs in the segment, or `basic` or `reviews` failed for merged PRs | +| `flow_efficiency_by_origin` | `Record` | same | no `commit_origin_map` provided, or < `min_sample` PRs in the segment, or `basic` or `reviews` failed for merged PRs | Phase model (5 timestamps → 4 phases per merged PR): @@ -765,10 +787,10 @@ event at all. | Field | Unit | Source | Nullable when | |---|---|---|---| -| `human_review_coverage_pct` | float `0.0–1.0` | `analysis/human_review_coverage.py` | no merged PR in the window, or `reviews` degraded | +| `human_review_coverage_pct` | float `0.0–1.0` | `analysis/human_review_coverage.py` | no merged PR in the window, or `basic` or `reviews` failed for merged PRs | | `human_approval_coverage_pct` | float `0.0–1.0` | same | same | -| `human_review_coverage_by_intent` | `Record` | same | < `min_sample` (default 10) PRs in the segment, or `reviews` degraded | -| `human_review_coverage_by_origin_of_pr` | `Record` | same | no `commit_origin_map` provided, or < `min_sample` PRs in the segment, or `reviews` degraded | +| `human_review_coverage_by_intent` | `Record` | same | < `min_sample` (default 10) PRs in the segment, or `basic` or `reviews` failed for merged PRs | +| `human_review_coverage_by_origin_of_pr` | `Record` | same | no `commit_origin_map` provided, or < `min_sample` PRs in the segment, or `basic` or `reviews` failed for merged PRs | Definition (per merged PR, intermediate only): @@ -837,14 +859,14 @@ Aging = "do trabalho que ainda não saiu, está represado?". | Field | Unit | Source | Nullable when | |---|---|---|---| -| `open_pr_count` | int | `analysis/open_pr_aging.py` | no eligible open PR (all drafts/bots) | +| `open_pr_count` | int | `analysis/open_pr_aging.py` | no eligible open PR (all drafts/bots), or `basic` failed for open PRs | | `median_open_pr_age_days` | float ≥ 0 | same | same | | `p90_open_pr_age_days` | float ≥ 0 | same | same | -| `stale_open_pr_pct` | float `0.0–1.0` | same | same, or `reviews` or `enrichment` degraded | -| `very_stale_open_pr_pct` | float `0.0–1.0` | same | same, or `reviews` or `enrichment` degraded | -| `abandonment_risk_pct` | float `0.0–1.0` | same | same, or `reviews` or `enrichment` degraded | -| `median_open_pr_age_by_intent` | `Record` | same | < `min_sample` (default 5) PRs in segment | -| `stale_open_pr_pct_by_origin` | `Record` | same | no `commit_origin_map`, or < `min_sample` PRs in segment, or `reviews` or `enrichment` degraded | +| `stale_open_pr_pct` | float `0.0–1.0` | same | same, or `reviews` or `enrichment` failed for open PRs | +| `very_stale_open_pr_pct` | float `0.0–1.0` | same | same, or `reviews` or `enrichment` failed for open PRs | +| `abandonment_risk_pct` | float `0.0–1.0` | same | same, or `reviews` or `enrichment` failed for open PRs | +| `median_open_pr_age_by_intent` | `Record` | same | < `min_sample` (default 5) PRs in segment, or `basic` failed for open PRs | +| `stale_open_pr_pct_by_origin` | `Record` | same | no `commit_origin_map`, or < `min_sample` PRs in segment, or `basic`, `reviews` or `enrichment` failed for open PRs | Per-PR signals (intermediate, **never persisted**): @@ -915,9 +937,9 @@ under-report AI adoption in squash repos. | Field | Unit | Source | Nullable when | |---|---|---|---| -| `merge_strategy` | `merge\|squash\|rebase\|mixed\|unknown` | `analysis/merge_strategy_detector.py` | no PR data (`prs` empty/None) | -| `merge_strategy_dominant_share` | float `0.0–1.0` | same | strategy is `unknown` | -| `commit_metrics_reliable` | bool | same | no PR data | +| `merge_strategy` | `merge\|squash\|rebase\|mixed\|unknown` | `analysis/merge_strategy_detector.py` | no PR data (`prs` empty/None), or `basic` failed for merged PRs | +| `merge_strategy_dominant_share` | float `0.0–1.0` | same | strategy is `unknown` or absent | +| `commit_metrics_reliable` | bool | same | no PR data, or `basic` failed for merged PRs | Per-PR classification (over **merged** PRs only), in confidence order: @@ -966,6 +988,8 @@ without classifying: `merge_strategy_dominant_share` is absent and merge PR falls through to `unknown` while a squash-stamped one still classifies, so the fallback could only ever answer `squash` — a mixed repo would read as 100% squash, with its per-commit metrics flagged unreliable. +When the merged list itself failed (`basic` for merged PRs, §15), there is +nothing to classify and the three fields are absent, as with no PR data. --- From 8c3703d309ee411a34595d57f30c6ce369b4db31 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 20:56:40 -0300 Subject: [PATCH 19/22] fix(engine): name open-PR metrics in the PR-read caveat The report caveat listed flow, review and in-PR metrics as what a degraded read may leave missing, but a failed open list, or a failed enrichment or reviews pass for open PRs, omits open-PR fields too. The caveat now names them ("open-PR metrics" / "de PRs abertos"); its opening sentence is unchanged. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 --- iris/i18n.py | 15 ++++++++------- tests/test_pr_enrichment_caveat.py | 8 ++++++++ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/iris/i18n.py b/iris/i18n.py index 71cbfa6..623cdfa 100644 --- a/iris/i18n.py +++ b/iris/i18n.py @@ -36,9 +36,9 @@ "pr_enrichment_degraded_caveat": ( "PR data is incomplete for this run — failed steps: {steps}. Merge " "strategy is reported as unknown when enrichment failed for merged " - "PRs, and flow, review and in-PR metrics may be missing or skewed. A " - "transient GitHub error usually clears on the next run; if every run " - "fails, check that gh is authenticated (gh auth status)." + "PRs, and flow, review, in-PR and open-PR metrics may be missing or " + "skewed. A transient GitHub error usually clears on the next run; if " + "every run fails, check that gh is authenticated (gh auth status)." ), "system_disclaimer": ( "Engineering outcomes emerge from system dynamics, domain complexity, " @@ -928,10 +928,11 @@ "pr_enrichment_degraded_caveat": ( "Os dados de PR estão incompletos nesta execução — passos com falha: " "{steps}. A estratégia de merge sai como desconhecida quando o " - "enriquecimento dos PRs mergeados falha, e métricas de flow, de review " - "e de commits em PR podem faltar ou sair distorcidas. Um erro " - "transitório do GitHub costuma sumir na próxima execução; se toda " - "execução falhar, verifique se o gh está autenticado (gh auth status)." + "enriquecimento dos PRs mergeados falha, e métricas de flow, de " + "review, de commits em PR e de PRs abertos podem faltar ou sair " + "distorcidas. Um erro transitório do GitHub costuma sumir na próxima " + "execução; se toda execução falhar, verifique se o gh está " + "autenticado (gh auth status)." ), "system_disclaimer": ( "Resultados de engenharia emergem de dinâmicas sistêmicas, complexidade do domínio " diff --git a/tests/test_pr_enrichment_caveat.py b/tests/test_pr_enrichment_caveat.py index d35ffc2..7c26b81 100644 --- a/tests/test_pr_enrichment_caveat.py +++ b/tests/test_pr_enrichment_caveat.py @@ -68,3 +68,11 @@ def test_caveat_is_translated(tmp_path): def test_clean_run_has_no_caveat(tmp_path): assert _CAVEAT_EN not in _report_text(tmp_path, None) + + +def test_caveat_lists_open_pr_metrics_among_what_may_be_missing(tmp_path): + # A failed open list, or a failed enrichment or reviews pass for open + # PRs, omits open-PR fields too, so the caveat names them. + assert "flow, review, in-PR and open-PR metrics" in _report_text(tmp_path, ["basic"]) + assert "de commits em PR e de PRs abertos" in _report_text( + tmp_path, ["basic"], lang="pt-br") From 312f8200c9bb662a5b9ad8a537661001890d345d Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 21:08:15 -0300 Subject: [PATCH 20/22] docs(engine): note flow_load omission when a PR list failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §24 listed a failed list only in the nullable-when cell. It now says the whole series goes when `gh pr list` failed for any state (or `fetch`), author_concurrency included, and why: the missing list would undercount the WIP in every bucket it overlaps, and keeping only the commit-based author count would emit buckets in a shape the platform does not read. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 --- docs/METRICS.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/METRICS.md b/docs/METRICS.md index f2c0694..ab5e38b 100644 --- a/docs/METRICS.md +++ b/docs/METRICS.md @@ -634,6 +634,15 @@ proxy. falling back to author name) with at least one non-merge commit in the bucket +**Failed PR list.** When `gh pr list` failed for any PR state (`basic` for +merged, closed or open PRs, or `fetch`; §15), the whole `flow_load` series +is omitted, `author_concurrency` included. Every state is in flight at some +point, so a missing list would undercount `wip_total` and `wip_by_intent` in +every bucket it overlaps. `author_concurrency` comes from commits and would +still be right, but keeping it alone would mean emitting buckets with the +WIP fields missing, a shape the platform does not read; the series goes +whole instead. + Overlap rule for a bucket `[t_start, t_end)`: ``` From 581095b501c1f5b82a818fa0275ab457585962c3 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 21:19:46 -0300 Subject: [PATCH 21/22] docs(engine): correct what the PR read covers and what it skews MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several notes still described the older PR read. The aggregate() docstring called `prs` merged PRs, but they come in all three states. METRICS.md §10 and the funnel module said Iris only fetches merged PRs; it reads merged, closed and open PRs, and the limitation is that acceptance counts only commits in merged PRs. §10 now says a failure confined to closed or open PRs leaves acceptance and the funnel where a clean run would have them, without implying they are always present. §15 now says that with merged enrichment failed, Flow Efficiency is computed over the PRs whose commit refs came back, so it is skewed rather than missing. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 --- docs/METRICS.md | 22 ++++++++++++++-------- iris/analysis/origin_funnel.py | 6 ++++-- iris/metrics/aggregator.py | 3 ++- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/docs/METRICS.md b/docs/METRICS.md index ab5e38b..6131bd8 100644 --- a/docs/METRICS.md +++ b/docs/METRICS.md @@ -265,7 +265,9 @@ The whole funnel is omitted when `pr_enrichment_degraded` (§15) is present and later conversion chains from that stage. That covers a failed `basic` or `enrichment` for merged PRs, which omits acceptance (§9), and `fetch`, which leaves no PRs to match commits to. A failure confined to closed or open PRs -keeps acceptance, and the funnel with it. +does not remove acceptance or the funnel: they stay whenever a clean run +would have them — acceptance still needs an origin with at least 5 commits +(§9), and the funnel a `commit_origin_distribution`. Only when PRs are absent by design (no `gh`, no GitHub remote; nothing degraded) does `In PR` fall back to counting every commit as in a PR. @@ -274,8 +276,10 @@ last stage only when durability data is available). Each stage carries `count` and `conversion_from_previous`. `overall_conversion` is the product of conversions. -**Limitation:** `PR → Merge` is not measured — Iris only fetches merged -PRs. +**Limitation:** `PR → Merge` is not measured. Iris reads merged, closed and +open PRs, but `In PR` comes from acceptance (§9), which counts only commits in +merged PRs: a commit in a PR still open or closed without merging reads as not +in a PR, and no stage follows the PRs that never merge. --- @@ -405,11 +409,13 @@ without repeats. Unlike the fields above it does not require PR data: a read that lost every PR still reports why. Absent when every step succeeded, and when PRs are absent by design — no `gh`, no GitHub remote — because Iris works without PRs. When present, this run's PR-derived fields may be missing or -skewed — flow and `commits_in_prs` go missing — and `merge_strategy` is -`unknown` if `enrichment` failed for merged PRs (§28). With `reviews` failed -every PR carries no reviews, so the fields derived from them are omitted -rather than reported as the 0% coverage and 100% single-pass an empty review -list would give: +skewed. With `enrichment` failed for merged PRs, Flow Efficiency (§25) is +computed only over the merged PRs whose commit refs came back, so it is +skewed rather than missing (unless none came back); `commits_in_prs` goes +missing with acceptance (§9); and `merge_strategy` is `unknown` (§28). With +`reviews` failed every PR carries no reviews, so the fields derived from them +are omitted rather than reported as the 0% coverage and 100% single-pass an +empty review list would give: `pr_review_rounds_median`, `pr_single_pass_rate`, `median_time_to_first_review_hours`, `human_review_coverage_pct`, `human_approval_coverage_pct`, `human_review_coverage_by_intent`, diff --git a/iris/analysis/origin_funnel.py b/iris/analysis/origin_funnel.py index d231d4a..c02f05c 100644 --- a/iris/analysis/origin_funnel.py +++ b/iris/analysis/origin_funnel.py @@ -8,8 +8,10 @@ Each stage has a conversion rate. Comparing funnels across origins reveals where AI-assisted code drops off relative to human code. -Limitation: PR→Merge stage is not measured (Iris only fetches merged PRs). -Future work could add closed/rejected PR data to fill this gap. +Limitation: PR→Merge stage is not measured. Iris reads merged, closed and open +PRs, but the "In PR" stage reads acceptance, which counts only commits in +merged PRs, and no stage follows PRs that never merge. The closed PRs already +read could fill this gap. """ # AGGREGATOR_OPT_OUT: consumes a finalized ReportMetrics (post-aggregation composition). diff --git a/iris/metrics/aggregator.py b/iris/metrics/aggregator.py index 279cc38..8582342 100644 --- a/iris/metrics/aggregator.py +++ b/iris/metrics/aggregator.py @@ -82,7 +82,8 @@ def aggregate( Args: commits: Commits from git_reader (sorted by date ascending). churn_days: Churn/stabilization window in days. - prs: Optional list of merged PRs from github_reader. + prs: Optional PRs from github_reader, in all three states (merged, + closed and open); each analysis keeps the states it reads. external_data: Optional pre-fetched DORA events (deployments + incidents) from a connected provider — currently Datadog. When provided, populates the ``dora_*`` fields on ReportMetrics; From d0ae092a82783ca49b6d9f11533e4ec8af262108 Mon Sep 17 00:00:00 2001 From: Carlos Penaforte Date: Wed, 7 Oct 2026 21:20:16 -0300 Subject: [PATCH 22/22] fix(engine): name PR lifecycle metrics in the PR-read caveat A failed merged list omits the PR lifecycle fields (counts, time to merge, cycle time), and a partial read skews them, but the report caveat did not list them among what may be missing or skewed. It now does, in both languages; its opening sentence is unchanged. Refs #236 Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 --- iris/i18n.py | 10 ++++++---- tests/test_pr_enrichment_caveat.py | 9 +++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/iris/i18n.py b/iris/i18n.py index 623cdfa..4fba747 100644 --- a/iris/i18n.py +++ b/iris/i18n.py @@ -36,9 +36,10 @@ "pr_enrichment_degraded_caveat": ( "PR data is incomplete for this run — failed steps: {steps}. Merge " "strategy is reported as unknown when enrichment failed for merged " - "PRs, and flow, review, in-PR and open-PR metrics may be missing or " - "skewed. A transient GitHub error usually clears on the next run; if " - "every run fails, check that gh is authenticated (gh auth status)." + "PRs, and PR lifecycle (counts, time to merge, cycle time), flow, " + "review, in-PR and open-PR metrics may be missing or skewed. A " + "transient GitHub error usually clears on the next run; if every run " + "fails, check that gh is authenticated (gh auth status)." ), "system_disclaimer": ( "Engineering outcomes emerge from system dynamics, domain complexity, " @@ -928,7 +929,8 @@ "pr_enrichment_degraded_caveat": ( "Os dados de PR estão incompletos nesta execução — passos com falha: " "{steps}. A estratégia de merge sai como desconhecida quando o " - "enriquecimento dos PRs mergeados falha, e métricas de flow, de " + "enriquecimento dos PRs mergeados falha, e métricas de ciclo de vida " + "de PR (contagens, tempo até o merge, cycle time), de flow, de " "review, de commits em PR e de PRs abertos podem faltar ou sair " "distorcidas. Um erro transitório do GitHub costuma sumir na próxima " "execução; se toda execução falhar, verifique se o gh está " diff --git a/tests/test_pr_enrichment_caveat.py b/tests/test_pr_enrichment_caveat.py index 7c26b81..64bc28f 100644 --- a/tests/test_pr_enrichment_caveat.py +++ b/tests/test_pr_enrichment_caveat.py @@ -76,3 +76,12 @@ def test_caveat_lists_open_pr_metrics_among_what_may_be_missing(tmp_path): assert "flow, review, in-PR and open-PR metrics" in _report_text(tmp_path, ["basic"]) assert "de commits em PR e de PRs abertos" in _report_text( tmp_path, ["basic"], lang="pt-br") + + +def test_caveat_lists_pr_lifecycle_metrics_among_what_may_be_missing(tmp_path): + # A failed merged list omits the PR lifecycle fields, and a partial read + # skews them, so the caveat names them too. + assert "PR lifecycle (counts, time to merge, cycle time)" in _report_text( + tmp_path, ["basic"]) + assert "ciclo de vida de PR (contagens, tempo até o merge, cycle time)" in ( + _report_text(tmp_path, ["basic"], lang="pt-br"))