diff --git a/docs/METRICS.md b/docs/METRICS.md index cf8cda9..6131bd8 100644 --- a/docs/METRICS.md +++ b/docs/METRICS.md @@ -236,13 +236,17 @@ 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 `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 -zero `CHANGES_REQUESTED` / PRs), `median_review_rounds`. The last two are omitted when `reviews` is in -`pr_enrichment_degraded` (§15). +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. +The same goes when the merged list itself failed (`basic`): with no merged PRs +to match, every group would read 0 too. --- @@ -254,15 +258,28 @@ 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 `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 `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 +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. Stages: `Committed` → `In PR` → `Stabilized` → `Lines Surviving` (the 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. --- @@ -334,7 +351,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`. @@ -353,7 +372,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 | @@ -361,8 +380,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 @@ -380,16 +399,23 @@ 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 -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. 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`, @@ -398,10 +424,36 @@ 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. +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 +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. --- @@ -573,7 +625,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`: @@ -588,6 +640,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)`: ``` @@ -639,11 +700,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): @@ -741,10 +802,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): @@ -813,14 +874,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` 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 | -| `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` | 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**): @@ -891,9 +952,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: @@ -936,12 +997,14 @@ 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 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. --- diff --git a/iris/analysis/origin_funnel.py b/iris/analysis/origin_funnel.py index 5d56a38..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). @@ -56,8 +58,20 @@ 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 read was degraded and acceptance is absent: the "In PR" + stage reads acceptance, and every later conversion chains from it. """ + # `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 + origin_dist = metrics.commit_origin_distribution if not origin_dist: return None diff --git a/iris/cli.py b/iris/cli.py index 755eb41..601c73a 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") @@ -523,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") @@ -553,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, @@ -686,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 4522bb8..4fba747 100644 --- a/iris/i18n.py +++ b/iris/i18n.py @@ -35,10 +35,11 @@ ), "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 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, " @@ -338,9 +339,11 @@ # 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} 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": ( @@ -527,6 +530,10 @@ "project manifest, so their metrics describe documentation rather than " "delivery: {repos}." ), + "org_pr_read_degraded": ( + "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": ( "Of {total_repos} repositories, {ai_repos} contain AI-assisted commits. " @@ -922,10 +929,12 @@ "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 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á " + "autenticado (gh auth status)." ), "system_disclaimer": ( "Resultados de engenharia emergem de dinâmicas sistêmicas, complexidade do domínio " @@ -1227,9 +1236,11 @@ # 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 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": ( @@ -1417,6 +1428,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ó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": ( "De {total_repos} repositórios, {ai_repos} contêm commits assistidos por IA. " diff --git a/iris/ingestion/github_reader.py b/iris/ingestion/github_reader.py index e3d6df6..fe5d712 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,50 @@ 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, 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"\bHTTP 5\d\d\b|\btime(?:d)? ?out\b|\bdeadline exceeded\b" + r"|\bconnection (?:reset|refused)\b|\bEOF\b", + re.IGNORECASE, +) + +def _is_transient_gh_error(stderr: str, nwo: str) -> bool: + """Whether gh's stderr reads as an error a second try could clear. -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.""" + 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. + """ for attempt in (1, 2): try: return subprocess.run( @@ -220,9 +259,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 _is_transient_gh_error(error.stderr or "", nwo): + return None + time.sleep(_RETRY_DELAY_S) return None @@ -249,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) @@ -266,17 +309,21 @@ 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: - args.extend(["-F", f"cursor={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 @@ -285,21 +332,31 @@ 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", []): + # 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") 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 []: + if not isinstance(entry, dict): + continue commit = entry.get("commit") or {} oid = commit.get("oid", "") if not oid: @@ -328,26 +385,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, nwo) + 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: @@ -427,6 +491,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 6b615c5..8582342 100644 --- a/iris/metrics/aggregator.py +++ b/iris/metrics/aggregator.py @@ -42,24 +42,48 @@ 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__) +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, 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. 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; @@ -71,14 +95,52 @@ 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 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 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 + 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 + # 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) @@ -251,9 +313,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"] = [ { @@ -282,45 +347,23 @@ def aggregate( # Acceptance rate (requires PR data with commit hashes) acceptance_kwargs: dict = {} - if prs: + 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: - 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, # 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, @@ -350,7 +393,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, @@ -373,7 +416,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(), @@ -391,9 +434,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 it would read older than it is. - if not reviews_degraded: + if not staleness_degraded: open_pr_aging_kwargs["stale_open_pr_pct"] = ( aging_result.stale_open_pr_pct ) @@ -403,7 +444,7 @@ 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 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 ) @@ -413,11 +454,11 @@ 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, - 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"] = ( @@ -434,9 +475,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"] = [ { @@ -452,7 +495,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/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/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..a7e216a --- /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 repository(ies) (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") diff --git a/tests/test_pr_enrichment_caveat.py b/tests/test_pr_enrichment_caveat.py index d35ffc2..64bc28f 100644 --- a/tests/test_pr_enrichment_caveat.py +++ b/tests/test_pr_enrichment_caveat.py @@ -68,3 +68,20 @@ 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") + + +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")) diff --git a/tests/test_pr_fetch_degradation.py b/tests/test_pr_fetch_degradation.py index 23cccca..4d6b8ee 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 @@ -69,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="") @@ -97,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): @@ -115,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) @@ -129,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])]) @@ -146,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): @@ -156,7 +172,135 @@ 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 == [] + + +# 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"} + 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): @@ -257,6 +401,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 --------------------------------------------- @@ -643,3 +794,561 @@ 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 + + +# --- 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 + + +@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"] = 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 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): + 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 -------------------------------- + + +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" + + +@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 ------------------------------- + + +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 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): + 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 + + +# --- 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 + + +@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()) + + 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 + + +@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=())) + + 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 + + +# --- 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 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) ---------