diff --git a/CHANGELOG.md b/CHANGELOG.md index 30f250a..9cb3f2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,36 @@ All notable changes to Iris are documented here. The format is based on [Keep a --- +## v1.2.0 — Merge Strategy detection + mergeCommit ingestion (2026-06-03) + +### Added + +- **Merge Strategy metric** (#76). New engine module + `analysis/merge_strategy_detector.py` classifies each repository's + dominant merge strategy (`merge` / `squash` / `rebase` / `mixed` / + `unknown`) from its merged PRs, and emits `merge_strategy`, + `merge_strategy_dominant_share`, and a `commit_metrics_reliable` flag + (False for squash/mixed, where collapsing commits makes per-commit + metrics approximate). Classification combines merge-commit ground truth + (parent count), commit-ref presence in `main`, and the GitHub squash + `(#N)` subject stamp. Strictly per-repository — no author axis. +- Wired through the full chain: schema, aggregator, report writer (Merge + Strategy section), narrative finding, i18n (en + pt-br), TypeScript + types, platform UI (repo-detail reliability badge + compare-table + column), ingest route, migration `019`, and `docs/METRICS.md`. + +### Changed + +- **PR ingestion** (#75) now captures `merge_commit_sha` + + `merge_commit_parent_count` on `PullRequest` and `subject` on + `CommitRef`. `github_reader` adds `mergeCommit` to the gh field lists + and fetches `mergeCommit{oid parents{totalCount}}` plus per-commit + `messageHeadline` via the light GraphQL enrichment pass. The data is + the ground-truth enabler for Merge Strategy detection; backward + compatible (fields default to `None` / `""`). + +--- + ## v1.1.0 — Human Review Coverage + sortable compare table (2026-05-29) ### Added diff --git a/docs/METRICS.md b/docs/METRICS.md index d2c5b5f..17b96f7 100644 --- a/docs/METRICS.md +++ b/docs/METRICS.md @@ -836,7 +836,66 @@ Findings emitted by `narrative.py` (see `iris/i18n.py:finding_open_pr_aging_*`): --- -## 28. Adoption timeline (post-report, not on `ReportMetrics`) +## 28. Merge Strategy + +Per-**repository** classification of how PRs land on the default branch, +plus a per-commit reliability flag. A repo's merge strategy decides how +much per-commit signal survives in history: **squash** collapses N commits +into 1 — discarding commit counts, temporal distribution, bursts, cascades, +and (depending on GitHub config) the `Co-Authored-By` trailers AI +attribution relies on. Comparing per-commit metrics across repos with +different strategies compares things that aren't comparable, and can +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 | + +Per-PR classification (over **merged** PRs only), in confidence order: + +1. **Ground truth** — `merge_commit_parent_count == 2` → `merge` (true + merge commit). Parent count comes from PR ingestion + (`merge_commit_sha` + parent count, issue #75); available on the + GraphQL enrichment path, `None` on the gh one-shot path → falls through + to the heuristic. +2. **Commit-ref presence in local `main` history** (the `commits` window): + - all of a PR's `commit_refs` present → `merge` (commits landed verbatim). + - none present → collapsed/rewritten: GitHub's squash default stamps the + landed subject `(#)` (matched against `main` subjects) → + `squash`; else single original commit → `squash`; else N commits, none + preserved → `rebase`. +3. Ambiguous (partial presence, no `commit_refs`, no signal) → that PR is + `unknown` and excluded from the dominant computation. + +Aggregation: the dominant strategy over **classified** merged PRs. +`dominant_share ≥ 0.8` → that strategy; below → `mixed`; fewer than +`MIN_CLASSIFIED_PRS` (5) classified → `unknown` (reason logged, never +invented). `commit_metrics_reliable` is `False` only for `squash`/`mixed`; +`merge`/`rebase`/`unknown` stay `True` (we never flag what we can't +determine). + +Privacy / ranking risk (Principle #2): **none by construction.** Strictly +per-repository — a property of repo *configuration* (the merge button), +never of people. No author axis anywhere in the output. The report/UI +framing is "this config affects metric reliability", never "this team/dev +does X". + +Finding emitted by `narrative.py` (`iris/i18n.py:finding_merge_strategy_*`): + +- `finding_merge_strategy_unreliable` — when `commit_metrics_reliable is + False` (squash/mixed): per-commit metrics are approximate for this repo. +- `finding_merge_strategy_descriptive` — otherwise (merge/rebase): history + preserved. Silent when `merge_strategy == "unknown"`. + +Platform: indexed columns `merge_strategy` + `commit_metrics_reliable` on +`metrics` (migration `019`) feed the compare table; the full payload +carries `merge_strategy_dominant_share` for the repo-detail badge. + +--- + +## 29. Adoption timeline (post-report, not on `ReportMetrics`) When AI-assisted commits started appearing, and how the pre-adoption vs post-adoption metrics compare. @@ -936,6 +995,7 @@ By-origin attribution: | `analysis/pr_lifecycle.py` | `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`, `pr_review_rounds_median`, `pr_single_pass_rate` | | `analysis/flow_load.py` | `flow_load` | | `analysis/flow_efficiency.py` | `flow_efficiency_median`, `median_time_to_first_review_hours`, `time_in_phase_median_hours`, `flow_efficiency_by_intent`, `flow_efficiency_by_origin` | +| `analysis/merge_strategy_detector.py` | `merge_strategy`, `merge_strategy_dominant_share`, `commit_metrics_reliable` | | `analysis/dora_real.py` | `dora_source`, `dora_deployments_total`, `dora_deployments_failed`, `dora_deployments_pending_evaluation`, `dora_incidents_total`, `dora_cfr`, `dora_mttr_per_deploy_seconds_median`, `dora_mttr_per_deploy_seconds_p90`, `dora_mttr_per_incident_seconds_median`, `dora_mttr_per_incident_seconds_p90`, `dora_rollback_rate`, `dora_rollbacks_total`, `dora_lead_time_seconds_median`, `dora_deploy_frequency_per_day`, `dora_remediation_distribution`, `dora_cfr_by_origin`, `dora_rollback_rate_by_origin`, `dora_cfr_by_origin_coverage_pct` | | `analysis/duplicate_detector.py` | `duplicate_block_rate`, `duplicate_block_count`, `duplicate_median_block_size`, `duplicate_by_origin`, `duplicate_by_tool` | | `analysis/move_detector.py` | `moved_code_pct`, `refactoring_ratio`, `move_by_origin` | diff --git a/iris/analysis/merge_strategy_detector.py b/iris/analysis/merge_strategy_detector.py new file mode 100644 index 0000000..ced7a97 --- /dev/null +++ b/iris/analysis/merge_strategy_detector.py @@ -0,0 +1,215 @@ +"""Merge Strategy detection — per-repository classification of how PRs land. + +A repository's merge strategy determines how much per-commit signal +survives in history. Squash collapses N commits into 1, discarding commit +counts, temporal distribution, bursts/cascades, and — depending on the +GitHub config — the ``Co-Authored-By`` trailers that AI attribution relies +on. Comparing per-commit metrics across repos that use *different* +strategies compares things that are not comparable, and can under-report AI +adoption in squash repos as if it were low real usage. + +This module classifies a repo into one of ``{merge, squash, rebase, mixed, +unknown}`` from its merged PRs, and emits ``commit_metrics_reliable=False`` +when the strategy (squash/mixed) erodes per-commit signal. + +Privacy / ranking risk +---------------------- +Strictly per-repository, by construction. The classification is a property +of repo *configuration* (the merge button), never of people. There is no +author axis anywhere in the output (Principle #2 / Non-Goal: no individual +ranking). The report/UI framing is always "this config affects metric +reliability", never "this team/dev does X". + +Signals, in order of confidence +-------------------------------- +1. **Ground truth** (when ``merge_commit_parent_count`` is available): + - parent count ``== 2`` → the PR landed as a true merge commit → MERGE. +2. **Commit-ref presence in the local main history:** + - *all* of the PR's ``commit_refs`` appear in main → the original + commits landed verbatim (merge or fast-forward) → MERGE. + - *none* of the PR's ``commit_refs`` appear in main → the commits were + collapsed or rewritten: + * GitHub's squash default stamps the landed commit subject with + ``(#)`` — a main-branch subject carrying this PR's + number corroborates SQUASH. + * else a single original commit → SQUASH (1→1 collapse/rewrite). + * else multiple original commits, none preserved → REBASE (replayed). +3. Anything ambiguous (partial presence, no commit_refs, no signal) → + ``unknown`` for that PR; excluded from the dominant computation. + +Aggregation +----------- +The dominant strategy over the *classified* merged PRs. +``dominant_share >= DOMINANT_SHARE_THRESHOLD`` → that strategy; below that +→ ``mixed``; fewer than ``MIN_CLASSIFIED_PRS`` classified → ``unknown`` +(the reason is carried on the result and logged by the caller, never +invented). + +Window / edge cases +------------------- +Uses the same ``commits`` window the rest of the pipeline runs on. A PR +merged near the window's start whose commits predate the window will read +as "refs absent" — the same window limitation Acceptance Rate lives with. +Repo with no merged PRs → ``unknown``. Open/closed-without-merge PRs are +ignored (they never landed). +""" + +import re +from collections import defaultdict +from dataclasses import dataclass, field + +from iris.models.commit import Commit +from iris.models.pull_request import PullRequest + +# Minimum classifiable merged PRs before we trust a dominant strategy. +MIN_CLASSIFIED_PRS = 5 + +# Share of classified PRs the leading strategy must reach to "own" the repo; +# below it (with a real split) the repo is ``mixed``. Hypothesis pending +# calibration — see Principle #4 (metrics are hypotheses). +DOMINANT_SHARE_THRESHOLD = 0.8 + +# Strategies that erode per-commit signal — these flip commit_metrics_reliable. +_UNRELIABLE_STRATEGIES = frozenset({"squash", "mixed"}) + +# GitHub's squash default titles the landed commit " (#)". +_SQUASH_SUBJECT_RE = re.compile(r"\(#(\d+)\)\s*$") + + +@dataclass(frozen=True) +class MergeStrategyResult: + """Per-repository merge-strategy classification. + + ``distribution`` and ``reason`` are diagnostics (not persisted to the + metrics schema) — they let the aggregator log *why* a repo came back + ``unknown`` instead of inventing a strategy. + """ + + merge_strategy: str # merge | squash | rebase | mixed | unknown + dominant_share: float | None # None when unknown + commit_metrics_reliable: bool + classified_pr_count: int + distribution: dict[str, int] = field(default_factory=dict) + reason: str | None = None + + +def detect_merge_strategy( + prs: list[PullRequest], + commits: list[Commit], + *, + min_classified: int = MIN_CLASSIFIED_PRS, +) -> MergeStrategyResult: + """Classify a repo's dominant merge strategy from its merged PRs. + + Args: + prs: PRs from github_reader (any state — only merged ones count). + commits: local main-branch commits from git_reader (the window the + rest of the pipeline analyses). Used to test whether a PR's + commit_refs landed verbatim and to read squash subject stamps. + min_classified: minimum classifiable merged PRs before a dominant + strategy is trusted; below it the repo is ``unknown``. + + Returns: + ``MergeStrategyResult`` — always non-None (``unknown`` when there + isn't enough signal). ``commit_metrics_reliable`` is False only for + squash/mixed; merge/rebase/unknown stay True (we never flag what we + can't determine). + """ + merged = [pr for pr in prs if pr.state == "merged"] + if not merged: + return MergeStrategyResult( + merge_strategy="unknown", + dominant_share=None, + commit_metrics_reliable=True, + classified_pr_count=0, + distribution={}, + reason="no merged PRs in window", + ) + + main_hashes = {c.hash for c in commits} + squash_pr_numbers = _squash_pr_numbers(commits) + + distribution: dict[str, int] = defaultdict(int) + for pr in merged: + distribution[_classify_pr(pr, main_hashes, squash_pr_numbers)] += 1 + + classified = {k: v for k, v in distribution.items() if k != "unknown"} + classified_count = sum(classified.values()) + + if classified_count < min_classified: + return MergeStrategyResult( + merge_strategy="unknown", + dominant_share=None, + commit_metrics_reliable=True, + classified_pr_count=classified_count, + distribution=dict(distribution), + reason=( + f"only {classified_count} classifiable merged PRs " + f"(< {min_classified} required)" + ), + ) + + dominant = max(classified, key=classified.get) + dominant_share = round(classified[dominant] / classified_count, 3) + strategy = dominant if dominant_share >= DOMINANT_SHARE_THRESHOLD else "mixed" + + return MergeStrategyResult( + merge_strategy=strategy, + dominant_share=dominant_share, + commit_metrics_reliable=strategy not in _UNRELIABLE_STRATEGIES, + classified_pr_count=classified_count, + distribution=dict(distribution), + reason=None, + ) + + +def _classify_pr( + pr: PullRequest, + main_hashes: set[str], + squash_pr_numbers: set[int], +) -> str: + """Classify a single merged PR. Returns merge/squash/rebase/unknown.""" + # 1. Ground truth: a true merge commit has two parents. + if pr.merge_commit_parent_count == 2: + return "merge" + + refs = [r.hash for r in pr.commit_refs] + present = sum(1 for h in refs if h in main_hashes) + + # 2a. Every commit landed verbatim → merge / fast-forward (signal intact). + if refs and present == len(refs): + return "merge" + + squash_corroborated = pr.number in squash_pr_numbers + + # 2b. No original commit survived → collapsed or rewritten. + if refs and present == 0: + if squash_corroborated: + return "squash" + if len(refs) == 1: + return "squash" + return "rebase" + + # 3. No usable commit_refs — lean only on the squash subject stamp. + if not refs: + return "squash" if squash_corroborated else "unknown" + + # Partial presence (force-push, shared base, dropped commits) — ambiguous. + return "unknown" + + +def _squash_pr_numbers(commits: list[Commit]) -> set[int]: + """PR numbers stamped on main-branch commit subjects via ``(#N)``. + + GitHub's squash default writes the landed commit subject as + `` (#<pr-number>)``. Collecting these lets ``_classify_pr`` + corroborate squash even for multi-commit PRs. Subjects without the + trailing stamp contribute nothing (no false positives from mid-message + issue references). + """ + numbers: set[int] = set() + for commit in commits: + match = _SQUASH_SUBJECT_RE.search(commit.message) + if match: + numbers.add(int(match.group(1))) + return numbers diff --git a/iris/cli.py b/iris/cli.py index 74e607f..d4dfe16 100644 --- a/iris/cli.py +++ b/iris/cli.py @@ -15,7 +15,7 @@ from iris.reports.narrative import generate_narrative from iris.reports.writer import write_output -VERSION = "v1.1.0" +VERSION = "v1.2.0" def _merge_durability(metrics, durability): diff --git a/iris/i18n.py b/iris/i18n.py index 741e1d5..4cf7ccd 100644 --- a/iris/i18n.py +++ b/iris/i18n.py @@ -210,6 +210,32 @@ "({ai_pct} vs {human_pct}) — generation outpaces review." ), + # Merge Strategy — per-repo classification + commit-reliability flag + "section_merge_strategy": "Merge Strategy", + "merge_strategy_intro": ( + "How PRs land on the default branch, classified from merged PRs. " + "Squash and mixed repos collapse commits, so per-commit metrics " + "(commit counts, bursts, cascades, AI attribution) are approximate." + ), + "metric_merge_strategy": "Dominant strategy", + "metric_merge_strategy_dominant_share": "Dominant share", + "metric_commit_metrics_reliable": "Per-commit metrics reliable", + "value_reliable": "Yes", + "value_not_reliable": "No", + "merge_strategy_unreliable_caveat": ( + "This repo collapses commits on merge — treat per-commit metrics " + "(commit shape, cascades, AI detection coverage) as approximate." + ), + "finding_merge_strategy_descriptive": ( + "Merge strategy: PRs land via {strategy} ({share} of merged PRs) — " + "per-commit history is preserved." + ), + "finding_merge_strategy_unreliable": ( + "Merge strategy: this repo merges via {strategy} ({share} of merged " + "PRs), collapsing commits — per-commit metrics (counts, bursts, AI " + "attribution) are approximate for this repo." + ), + # DORA (real) findings — populated only when external integration data is present "finding_dora_cfr_descriptive": ( "Change Failure Rate (Datadog): {cfr_pct} " @@ -1046,6 +1072,34 @@ "({ai_pct} vs {human_pct}) — geração supera a velocidade de review." ), + # Merge Strategy — classificação por repo + flag de confiabilidade por-commit + "section_merge_strategy": "Estratégia de Merge", + "merge_strategy_intro": ( + "Como os PRs aterrissam na branch padrão, classificado a partir dos " + "PRs mergeados. Repos squash e mixed colapsam commits, então métricas " + "por-commit (contagem de commits, bursts, cascades, atribuição de IA) " + "são aproximadas." + ), + "metric_merge_strategy": "Estratégia dominante", + "metric_merge_strategy_dominant_share": "Parcela dominante", + "metric_commit_metrics_reliable": "Métricas por-commit confiáveis", + "value_reliable": "Sim", + "value_not_reliable": "Não", + "merge_strategy_unreliable_caveat": ( + "Este repo colapsa commits no merge — trate métricas por-commit " + "(formato de commit, cascades, cobertura de detecção de IA) como " + "aproximadas." + ), + "finding_merge_strategy_descriptive": ( + "Estratégia de merge: PRs aterrissam via {strategy} ({share} dos PRs " + "mergeados) — o histórico por-commit é preservado." + ), + "finding_merge_strategy_unreliable": ( + "Estratégia de merge: este repo faz merge via {strategy} ({share} dos " + "PRs mergeados), colapsando commits — métricas por-commit (contagem, " + "bursts, atribuição de IA) são aproximadas para este repo." + ), + # Descobertas DORA (real) — populadas apenas quando há integração externa ativa "finding_dora_cfr_descriptive": ( "Change Failure Rate (Datadog): {cfr_pct} " diff --git a/iris/ingestion/github_reader.py b/iris/ingestion/github_reader.py index a0aebc0..4c10e37 100644 --- a/iris/ingestion/github_reader.py +++ b/iris/ingestion/github_reader.py @@ -71,13 +71,18 @@ def is_gh_available() -> bool: # commits secondary pass uses a custom light GraphQL query # (`oid + committedDate + authoredDate` only) via # `_fetch_commit_refs_by_pr_graphql`. +# +# `mergeCommit` IS in both lists: gh exposes it cheaply as a single +# `{oid}` node (no heavy author connections), so it never threatens the +# GraphQL budget. It supplies `merge_commit_sha`; the merge commit's +# parent count is only available via the light GraphQL pass below. _PR_FIELDS_BASIC = ( "number,title,createdAt,mergedAt,closedAt,state,isDraft," - "additions,deletions,changedFiles,author" + "additions,deletions,changedFiles,author,mergeCommit" ) _PR_FIELDS_FULL = ( "number,title,createdAt,mergedAt,closedAt,state,isDraft," - "additions,deletions,changedFiles,author,reviews,commits" + "additions,deletions,changedFiles,author,mergeCommit,reviews,commits" ) # Maximum PRs to fetch in a single gh call. Larger requests with the reviews @@ -117,11 +122,19 @@ def _fetch_prs(nwo: str, limit: int, gh_state: str) -> list[dict]: if prs is None: return [] - commits_by_pr = _fetch_commit_refs_by_pr_graphql( + enrichment_by_pr = _fetch_pr_enrichment_graphql( nwo, gh_state, min(limit, _BATCH_SIZE), ) for pr in prs: - pr["commits"] = commits_by_pr.get(pr["number"], []) + enrich = enrichment_by_pr.get(pr["number"], {}) + pr["commits"] = enrich.get("commits", []) + # GraphQL carries the merge commit's parent count, which gh's + # `mergeCommit` JSON field omits — prefer it when present so + # `merge_commit_parent_count` is populated on the common (busy-repo) + # path. The gh-supplied `{oid}` stays as the fallback. + merge_commit = enrich.get("mergeCommit") + if merge_commit: + pr["mergeCommit"] = merge_commit reviews_prs = _gh_pr_list(nwo, "number,reviews", min(limit, _BATCH_SIZE), gh_state) if reviews_prs: @@ -149,8 +162,9 @@ def _fetch_prs(nwo: str, limit: int, gh_state: str) -> list[dict]: pageInfo{endCursor hasNextPage} nodes{ number + mergeCommit{oid parents{totalCount}} commits(first:100){ - nodes{commit{oid committedDate authoredDate}} + nodes{commit{oid messageHeadline committedDate authoredDate}} } } } @@ -166,23 +180,27 @@ def _fetch_prs(nwo: str, limit: int, gh_state: str) -> list[dict]: } -def _fetch_commit_refs_by_pr_graphql( +def _fetch_pr_enrichment_graphql( nwo: str, gh_state: str, max_prs: int, -) -> dict[int, list[dict]]: - """Map PR number → light commit dicts via paginated GraphQL. +) -> dict[int, dict]: + """Map PR number → enrichment dict via paginated GraphQL. Why this exists: `gh pr list --json commits` is unusable on busy repos — gh's default commit subtree pulls `commit.authors.user. {id,login,email,name}`, which explodes the 500K-nodes GraphQL budget even at small page sizes. This helper asks only for the fields the - rest of the pipeline actually needs (oid + the two datetimes). + rest of the pipeline actually needs: per-commit oid + subject + the + two datetimes, plus the PR's merge commit (oid + parent count, the + ground-truth signal for Merge Strategy detection). - Returns ``{pr_number → [{"oid": ..., "committedDate": ..., - "authoredDate": ...}, ...]}``. Per-PR commit cap is 100 (the gh API - `first:` ceiling); PRs with more commits than that lose the older - entries — documented limitation that the caller should be aware of. + Returns ``{pr_number → {"commits": [{"oid", "messageHeadline", + "committedDate", "authoredDate"}, ...], "mergeCommit": {"oid", + "parents": {"totalCount"}} | None}}``. Per-PR commit cap is 100 (the + gh API `first:` ceiling); PRs with more commits than that lose the + older entries — documented limitation that the caller should be aware + of. Best-effort: returns whatever it has collected so far on any error. """ @@ -195,10 +213,10 @@ def _fetch_commit_refs_by_pr_graphql( if graphql_state is None: return {} - refs_by_pr: dict[int, list[dict]] = {} + by_pr: dict[int, dict] = {} end_cursor: str | None = None - while len(refs_by_pr) < max_prs: + while len(by_pr) < max_prs: args = [ "gh", "api", "graphql", "-f", "query=" + _COMMITS_GRAPHQL_QUERY, @@ -214,12 +232,12 @@ def _fetch_commit_refs_by_pr_graphql( args, capture_output=True, text=True, check=True, ) except (subprocess.CalledProcessError, FileNotFoundError): - return refs_by_pr + return by_pr try: data = json.loads(result.stdout) except json.JSONDecodeError: - return refs_by_pr + return by_pr page = ( data.get("data", {}) @@ -228,7 +246,7 @@ def _fetch_commit_refs_by_pr_graphql( if data.get("data") else None ) if not page: - return refs_by_pr + return by_pr for node in page.get("nodes", []): number = node.get("number") @@ -242,21 +260,25 @@ def _fetch_commit_refs_by_pr_graphql( continue commits.append({ "oid": oid, + "messageHeadline": commit.get("messageHeadline"), "committedDate": commit.get("committedDate"), "authoredDate": commit.get("authoredDate"), }) - refs_by_pr[number] = commits - if len(refs_by_pr) >= max_prs: - return refs_by_pr + by_pr[number] = { + "commits": commits, + "mergeCommit": node.get("mergeCommit"), + } + if len(by_pr) >= max_prs: + return by_pr info = page.get("pageInfo", {}) or {} if not info.get("hasNextPage"): - return refs_by_pr + return by_pr end_cursor = info.get("endCursor") if not end_cursor: - return refs_by_pr + return by_pr - return refs_by_pr + return by_pr def _gh_pr_list(nwo: str, fields: str, limit: int, gh_state: str) -> list[dict] | None: @@ -381,6 +403,9 @@ def read_single_pr(repo_path: str, pr_number: int) -> PullRequest | None: reviews = _parse_reviews(raw.get("reviews", [])) commit_refs = _parse_commit_refs(raw.get("commits", [])) + merge_commit_sha, merge_commit_parent_count = _parse_merge_commit( + raw.get("mergeCommit") + ) author = raw.get("author", {}) author_login = author.get("login", "") if isinstance(author, dict) else "" @@ -399,6 +424,8 @@ def read_single_pr(repo_path: str, pr_number: int) -> PullRequest | None: changed_files=raw.get("changedFiles", 0), reviews=reviews, commit_refs=commit_refs, + merge_commit_sha=merge_commit_sha, + merge_commit_parent_count=merge_commit_parent_count, ) @@ -432,6 +459,9 @@ def _parse_pull_requests( reviews = _parse_reviews(raw.get("reviews", [])) commit_refs = _parse_commit_refs(raw.get("commits", [])) + merge_commit_sha, merge_commit_parent_count = _parse_merge_commit( + raw.get("mergeCommit") + ) author = raw.get("author", {}) author_login = author.get("login", "") if isinstance(author, dict) else "" @@ -450,6 +480,8 @@ def _parse_pull_requests( changed_files=raw.get("changedFiles", 0), reviews=reviews, commit_refs=commit_refs, + merge_commit_sha=merge_commit_sha, + merge_commit_parent_count=merge_commit_parent_count, )) seen.add(number) @@ -502,11 +534,11 @@ def _parse_reviews(raw_reviews: list[dict]) -> list[PRReview]: def _parse_commit_refs(raw_commits: list[dict]) -> list[CommitRef]: - """Parse commit refs (oid + timestamps) from gh JSON commits field. + """Parse commit refs (oid + subject + timestamps) from gh JSON commits. - gh exposes ``committedDate`` and ``authoredDate`` per commit in the - PR's commit list. Both are optional in the JSON; ``hash`` is the - only required field. + gh exposes ``messageHeadline``, ``committedDate`` and ``authoredDate`` + per commit in the PR's commit list. All three are optional in the + JSON; ``hash`` is the only required field. """ refs: list[CommitRef] = [] for raw in raw_commits: @@ -519,10 +551,31 @@ def _parse_commit_refs(raw_commits: list[dict]) -> list[CommitRef]: hash=oid, committed_at=_parse_datetime(committed) if committed else None, authored_at=_parse_datetime(authored) if authored else None, + subject=raw.get("messageHeadline") or "", )) return refs +def _parse_merge_commit(raw_merge_commit: object) -> tuple[str | None, int | None]: + """Extract (sha, parent_count) from a gh/GraphQL ``mergeCommit`` node. + + Returns ``(None, None)`` when the PR was not merged or the field is + absent. ``parent_count`` is only present on the GraphQL path + (``parents.totalCount``); gh's JSON ``mergeCommit`` supplies just the + oid, so it comes back None there. + """ + if not isinstance(raw_merge_commit, dict): + return None, None + sha = raw_merge_commit.get("oid") or None + parents = raw_merge_commit.get("parents") + parent_count = None + if isinstance(parents, dict): + total = parents.get("totalCount") + if isinstance(total, int): + parent_count = total + return sha, parent_count + + def _parse_datetime(date_str: str) -> datetime: """Parse ISO-8601 datetime string from GitHub API. diff --git a/iris/metrics/aggregator.py b/iris/metrics/aggregator.py index a3183a3..7a7658c 100644 --- a/iris/metrics/aggregator.py +++ b/iris/metrics/aggregator.py @@ -5,6 +5,7 @@ schema (see `iris.models.metrics.ReportMetrics`). """ +import logging from dataclasses import asdict from iris.analysis.acceptance_rate import calculate_acceptance_rate @@ -18,6 +19,7 @@ from iris.analysis.fix_latency import calculate_fix_latency from iris.analysis.flow_efficiency import analyze_flow_efficiency from iris.analysis.human_review_coverage import analyze_human_review_coverage +from iris.analysis.merge_strategy_detector import detect_merge_strategy from iris.analysis.open_pr_aging import analyze_open_pr_aging, now_utc from iris.analysis.flow_load import analyze_flow_load from iris.analysis.stability_map import calculate_stability_map @@ -42,6 +44,8 @@ from iris.models.metrics import ReportMetrics from iris.models.pull_request import PullRequest +logger = logging.getLogger(__name__) + def aggregate( commits: list[Commit], @@ -377,6 +381,28 @@ def aggregate( aging_result.stale_open_pr_pct_by_origin ) + # Merge Strategy — per-repo classification of how PRs land + the + # commit_metrics_reliable flag (False for squash/mixed). Crosses merged + # 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: + merge_strategy_result = detect_merge_strategy(prs, commits) + merge_strategy_kwargs["merge_strategy"] = merge_strategy_result.merge_strategy + merge_strategy_kwargs["commit_metrics_reliable"] = ( + merge_strategy_result.commit_metrics_reliable + ) + if merge_strategy_result.dominant_share is not None: + merge_strategy_kwargs["merge_strategy_dominant_share"] = ( + merge_strategy_result.dominant_share + ) + if merge_strategy_result.merge_strategy == "unknown" and merge_strategy_result.reason: + logger.info( + "Merge strategy unknown: %s (distribution: %s)", + merge_strategy_result.reason, + merge_strategy_result.distribution, + ) + # Flow Load — WIP per ISO week (PRs in flight + author concurrency) flow_load_kwargs: dict = {} flow_load_result = analyze_flow_load(prs or [], commits) @@ -486,6 +512,7 @@ def aggregate( **flow_efficiency_kwargs, **human_review_coverage_kwargs, **open_pr_aging_kwargs, + **merge_strategy_kwargs, **_dora_real_kwargs(external_data, origin_map), ) diff --git a/iris/models/metrics.py b/iris/models/metrics.py index 5dd8e83..bbf743e 100644 --- a/iris/models/metrics.py +++ b/iris/models/metrics.py @@ -152,6 +152,15 @@ class ReportMetrics: median_open_pr_age_by_intent: dict[str, float] | None = None stale_open_pr_pct_by_origin: dict[str, float] | None = None + # Merge Strategy — per-repo classification of how PRs land + # (optional — None when no merged PR data is available). When the + # strategy is squash/mixed, ``commit_metrics_reliable`` is False to + # flag that per-commit metrics for this repo are approximate (squash + # collapses N commits into 1). Strictly per-repository — no author axis. + merge_strategy: str | None = None # merge|squash|rebase|mixed|unknown + merge_strategy_dominant_share: float | None = None # 0.0–1.0 + commit_metrics_reliable: bool | None = None + # DORA (real) — populated only when external Datadog events were fetched # for this run. None across the board when the org has no active Datadog # integration. ``dora_source`` is "datadog" when populated, None otherwise. diff --git a/iris/models/pull_request.py b/iris/models/pull_request.py index dc57f95..e9003d3 100644 --- a/iris/models/pull_request.py +++ b/iris/models/pull_request.py @@ -27,11 +27,17 @@ class CommitRef: ``committed_at`` is the authoritative ordering field. ``authored_at`` is kept for completeness (when the original author timestamp differs, e.g. rebased/cherry-picked work). + + ``subject`` is the commit's message headline (``messageHeadline`` from + the GitHub API). It enables matching the GitHub squash default subject + pattern ``(#<number>)`` used by Merge Strategy detection. Empty string + when the field was not fetched (backward-compatible). """ hash: str committed_at: datetime | None = None authored_at: datetime | None = None + subject: str = "" @dataclass(frozen=True) @@ -60,3 +66,12 @@ class PullRequest: is_draft: bool = False reviews: list[PRReview] = field(default_factory=list) commit_refs: list[CommitRef] = field(default_factory=list) + + # Merge-commit ground truth (optional — populated for merged PRs when the + # GitHub API supplies it). ``merge_commit_sha`` is the SHA of the commit + # that actually landed on the base branch; ``merge_commit_parent_count`` + # is that commit's parent count (2 → a true merge commit, 1 → squash or + # rebase landing). Both are None for non-merged PRs or when unavailable. + # Consumed by Merge Strategy detection (see merge_strategy_detector.py). + merge_commit_sha: str | None = None + merge_commit_parent_count: int | None = None diff --git a/iris/reports/narrative.py b/iris/reports/narrative.py index bb1b2c4..8140c6b 100644 --- a/iris/reports/narrative.py +++ b/iris/reports/narrative.py @@ -183,6 +183,9 @@ def generate_key_findings(metrics: ReportMetrics, lang: str = "en") -> str: # Open PR Aging — stuck-inventory pressure + abandonment + origin gap findings.extend(_open_pr_aging_findings(metrics, s)) + # Merge Strategy — reliability caveat when squash/mixed erodes commit signal + findings.extend(_merge_strategy_findings(metrics, s)) + # DORA (real) — descriptive bullets when external integration delivers data findings.extend(_dora_real_findings(metrics, s)) @@ -384,6 +387,33 @@ def _open_pr_aging_findings(metrics: ReportMetrics, s: dict) -> list[str]: return findings +def _merge_strategy_findings(metrics: ReportMetrics, s: dict) -> list[str]: + """Build Merge Strategy findings (0-1 bullet). + + Stays silent when the strategy is ``unknown`` (insufficient PRs to + classify — we never invent one). Emits a reliability caveat when + squash/mixed erodes per-commit signal, else a one-line descriptive + note that history is preserved. + """ + if metrics.merge_strategy is None or metrics.merge_strategy == "unknown": + return [] + + share = metrics.merge_strategy_dominant_share + share_str = f"{share:.0%}" if share is not None else "—" + + if metrics.commit_metrics_reliable is False: + return [ + s["finding_merge_strategy_unreliable"].format( + strategy=metrics.merge_strategy, share=share_str, + ) + ] + return [ + s["finding_merge_strategy_descriptive"].format( + strategy=metrics.merge_strategy, share=share_str, + ) + ] + + def _dora_real_findings(metrics: ReportMetrics, s: dict) -> list[str]: """Build DORA (real) findings (0-3 bullets) when external integration data is present. diff --git a/iris/reports/writer.py b/iris/reports/writer.py index c2abf3d..9bbb1e0 100644 --- a/iris/reports/writer.py +++ b/iris/reports/writer.py @@ -885,6 +885,33 @@ def write_report_md( ]) lines.extend(pr_rows) + # Merge Strategy section (conditional — only when PR data classified it) + if metrics.merge_strategy is not None: + lines.extend([ + f"## {s['section_merge_strategy']}", + f"", + f"> {s['merge_strategy_intro']}", + f"", + f"| {s['table_metric']} | {s['table_value']} |", + f"|---|---|", + f"| {s['metric_merge_strategy']} | {metrics.merge_strategy} |", + ]) + if metrics.merge_strategy_dominant_share is not None: + lines.append( + f"| {s['metric_merge_strategy_dominant_share']} | " + f"{metrics.merge_strategy_dominant_share:.0%} |" + ) + reliable_label = ( + s["value_not_reliable"] + if metrics.commit_metrics_reliable is False + else s["value_reliable"] + ) + lines.append(f"| {s['metric_commit_metrics_reliable']} | {reliable_label} |") + lines.append("") + if metrics.commit_metrics_reliable is False: + lines.append(f"> ⚠️ {s['merge_strategy_unreliable_caveat']}") + lines.append("") + # Delivery Velocity section (conditional — only when enough commits) if velocity is not None: lines.extend([ diff --git a/platform/lib/queries/temporal.ts b/platform/lib/queries/temporal.ts index b91ee11..82d0611 100644 --- a/platform/lib/queries/temporal.ts +++ b/platform/lib/queries/temporal.ts @@ -132,7 +132,7 @@ export async function getOrgReposSummary( const { data: allMetrics } = await supabase .from("metrics") .select( - "repository_id, created_at, stabilization_ratio, revert_rate, churn_events, commits_total, ai_detection_coverage_pct, pr_merged_count, pr_single_pass_rate, fix_latency_median_hours, cascade_rate", + "repository_id, created_at, stabilization_ratio, revert_rate, churn_events, commits_total, ai_detection_coverage_pct, pr_merged_count, pr_single_pass_rate, fix_latency_median_hours, cascade_rate, merge_strategy, commit_metrics_reliable", ) .eq("organization_id", organizationId) .order("created_at", { ascending: false }) @@ -181,6 +181,8 @@ export async function getOrgReposSummary( pr_single_pass_rate: latest?.pr_single_pass_rate ?? null, fix_latency_median_hours: latest?.fix_latency_median_hours ?? null, cascade_rate: latest?.cascade_rate ?? null, + merge_strategy: latest?.merge_strategy ?? null, + commit_metrics_reliable: latest?.commit_metrics_reliable ?? null, stabilization_delta: delta, health: classifyHealth(stabilization), sparkline, diff --git a/platform/lib/translations.ts b/platform/lib/translations.ts index 35f5c16..be62a68 100644 --- a/platform/lib/translations.ts +++ b/platform/lib/translations.ts @@ -551,6 +551,13 @@ export const translations = { churnEvents: "Churn Events", commits: "Commits", }, + mergeStrategy: { + label: "Merge", + reliable: "Per-commit metrics reliable", + unreliable: "Per-commit metrics approximate", + unreliableTooltip: + "This repo collapses commits on merge (squash/mixed), so commit-level metrics (commit shape, cascades, AI detection coverage) are approximate.", + }, dora: { title: "DORA", subtitle: @@ -1276,6 +1283,7 @@ export const translations = { churn: "Churn", commits: "Commits", ai: "AI%", + merge: "Merge", trend: "Trend", health: "Health", }, @@ -1902,6 +1910,13 @@ export const translations = { churnEvents: "Eventos de churn", commits: "Commits", }, + mergeStrategy: { + label: "Merge", + reliable: "Métricas por-commit confiáveis", + unreliable: "Métricas por-commit aproximadas", + unreliableTooltip: + "Este repo colapsa commits no merge (squash/mixed), então métricas por-commit (formato de commit, cascades, cobertura de detecção de IA) são aproximadas.", + }, dora: { title: "DORA", subtitle: @@ -2641,6 +2656,7 @@ export const translations = { churn: "Churn", commits: "Commits", ai: "IA%", + merge: "Merge", trend: "Tendência", health: "Saúde", }, diff --git a/platform/src/app/[tenant]/compare/compare-view.tsx b/platform/src/app/[tenant]/compare/compare-view.tsx index 1c2ff02..266f918 100644 --- a/platform/src/app/[tenant]/compare/compare-view.tsx +++ b/platform/src/app/[tenant]/compare/compare-view.tsx @@ -176,6 +176,35 @@ function MetricStat({ ); } +// Categorical merge-strategy cell. Amber + a ⚠ marker when squash/mixed makes +// per-commit metrics approximate; muted otherwise. Hidden when unclassified. +function MergeStrategyCell({ + strategy, + reliable, + unreliableTitle, +}: { + strategy: string | null; + reliable: boolean | null; + unreliableTitle: string; +}) { + if (!strategy || strategy === "unknown") + return <td className="px-3 py-2 text-muted-foreground">{"—"}</td>; + + const unreliable = reliable === false; + return ( + <td + className={cn( + "px-3 py-2 font-mono text-sm", + unreliable ? "text-signal-yellow" : "text-muted-foreground", + )} + title={unreliable ? unreliableTitle : undefined} + > + {strategy} + {unreliable ? " ⚠" : ""} + </td> + ); +} + export function CompareView({ repos }: CompareViewProps) { const { t } = useTranslation(); const [sort, setSort] = useState<SortState>({ @@ -311,6 +340,7 @@ export function CompareView({ repos }: CompareViewProps) { onSort={handleSort} /> )} + <th className="pb-2 px-3">{t("compare.columns.merge")}</th> <th className="pb-2 px-3">{t("compare.columns.trend")}</th> <SortableHeader label={t("compare.columns.health")} @@ -360,6 +390,13 @@ export function CompareView({ repos }: CompareViewProps) { : "—"} </td> )} + <MergeStrategyCell + strategy={repo.merge_strategy} + reliable={repo.commit_metrics_reliable} + unreliableTitle={t( + "repos.detail.mergeStrategy.unreliableTooltip", + )} + /> <td className="px-3 py-2"> <Sparkline data={repo.sparkline} /> </td> @@ -475,6 +512,24 @@ export function CompareView({ repos }: CompareViewProps) { format="rate" /> )} + {repo.merge_strategy && + repo.merge_strategy !== "unknown" && ( + <div className="flex flex-col gap-0.5"> + <span className="text-xs text-muted-foreground"> + {t("compare.columns.merge")} + </span> + <span + className={cn( + "font-mono text-sm", + repo.commit_metrics_reliable === false && + "text-signal-yellow", + )} + > + {repo.merge_strategy} + {repo.commit_metrics_reliable === false ? " ⚠" : ""} + </span> + </div> + )} </div> <div className="border-t border-border/60 pt-2"> diff --git a/platform/src/app/[tenant]/repos/[repoName]/page.tsx b/platform/src/app/[tenant]/repos/[repoName]/page.tsx index 20e6c1e..0de13cb 100644 --- a/platform/src/app/[tenant]/repos/[repoName]/page.tsx +++ b/platform/src/app/[tenant]/repos/[repoName]/page.tsx @@ -10,6 +10,7 @@ import { InvestmentHotspots } from "./investment-hotspots"; import { AdoptionTimelineCard } from "@/components/charts/AdoptionTimelineCard"; import { ChangeAlert } from "@/components/charts/ChangeAlert"; import { MetricCard } from "@/components/charts/MetricCard"; +import { Badge } from "@/components/ui/badge"; import { authOptions } from "@/lib/auth"; import { extractAdoptionSummary } from "@/lib/queries/adoption-timeline"; import { computeRepoDORA } from "@/lib/queries/dora"; @@ -98,6 +99,13 @@ function extractInsights(payload: Record<string, unknown> | null) { payload.human_review_coverage_by_origin_of_pr as | Partial<Record<string, number>> | undefined, + mergeStrategy: payload.merge_strategy as string | undefined, + mergeStrategyDominantShare: payload.merge_strategy_dominant_share as + | number + | undefined, + commitMetricsReliable: payload.commit_metrics_reliable as + | boolean + | undefined, }; } @@ -212,7 +220,27 @@ export default async function RepoDetailPage({ return ( <div className="space-y-6"> <div> - <h1 className="font-mono text-2xl font-bold">{repo.name}</h1> + <div className="flex flex-wrap items-center gap-2"> + <h1 className="font-mono text-2xl font-bold">{repo.name}</h1> + {insights.mergeStrategy && + insights.mergeStrategy !== "unknown" && + (insights.commitMetricsReliable === false ? ( + <Badge + variant="outline" + className="border-amber-500/40 bg-amber-500/10 text-amber-700 dark:text-amber-400" + title={t("repos.detail.mergeStrategy.unreliableTooltip")} + > + {t("repos.detail.mergeStrategy.label")}:{" "} + {insights.mergeStrategy} ·{" "} + {t("repos.detail.mergeStrategy.unreliable")} + </Badge> + ) : ( + <Badge variant="secondary"> + {t("repos.detail.mergeStrategy.label")}:{" "} + {insights.mergeStrategy} + </Badge> + ))} + </div> {repo.remote_url && ( <p className="text-sm text-muted-foreground">{repo.remote_url}</p> )} diff --git a/platform/src/app/api/ingest/route.ts b/platform/src/app/api/ingest/route.ts index 7930e6d..890273f 100644 --- a/platform/src/app/api/ingest/route.ts +++ b/platform/src/app/api/ingest/route.ts @@ -198,6 +198,14 @@ export async function POST(request: Request) { ((metrics as Record<string, unknown>).cascade_rate as | number | undefined) ?? null, + merge_strategy: + ((metrics as Record<string, unknown>).merge_strategy as + | string + | undefined) ?? null, + commit_metrics_reliable: + ((metrics as Record<string, unknown>).commit_metrics_reliable as + | boolean + | undefined) ?? null, }); if (metricsError) { diff --git a/platform/src/types/metrics.ts b/platform/src/types/metrics.ts index 696918a..2c9ce81 100644 --- a/platform/src/types/metrics.ts +++ b/platform/src/types/metrics.ts @@ -269,6 +269,13 @@ export interface ReportMetrics { human_review_coverage_by_intent?: Partial<Record<ChangeIntent, number>>; human_review_coverage_by_origin_of_pr?: Partial<Record<CommitOrigin, number>>; + // Merge Strategy — per-repo classification of how PRs land on the default + // branch. When `commit_metrics_reliable` is false (squash/mixed), the UI + // flags this repo's per-commit metrics as approximate. Strictly per-repo. + merge_strategy?: "merge" | "squash" | "rebase" | "mixed" | "unknown"; + merge_strategy_dominant_share?: number; // 0.0–1.0 + commit_metrics_reliable?: boolean; + // DORA (real) — populated when the org has an active Datadog integration // and the engine fetched events for the analysis window. Every field // here is optional; `dora_source` is the canonical "is there data?" flag. diff --git a/platform/src/types/temporal.ts b/platform/src/types/temporal.ts index 3dc8728..24d510b 100644 --- a/platform/src/types/temporal.ts +++ b/platform/src/types/temporal.ts @@ -33,6 +33,11 @@ export interface RepoSummary { pr_single_pass_rate: number | null; fix_latency_median_hours: number | null; cascade_rate: number | null; + // Merge strategy + per-commit reliability flag (null when not classified). + // When commit_metrics_reliable is false (squash/mixed), per-commit columns + // for this repo are approximate. + merge_strategy: string | null; + commit_metrics_reliable: boolean | null; // Delta vs previous run stabilization_delta: number | null; // Health classification @@ -72,7 +77,9 @@ export interface ChangeDetection { } /** Health classification thresholds. */ -export function classifyHealth(stabilization: number | null): RepoSummary["health"] { +export function classifyHealth( + stabilization: number | null, +): RepoSummary["health"] { if (stabilization === null) return "unknown"; if (stabilization >= 0.6) return "healthy"; if (stabilization >= 0.4) return "warning"; @@ -82,9 +89,13 @@ export function classifyHealth(stabilization: number | null): RepoSummary["healt /** Health indicator emoji. */ export function healthIndicator(health: RepoSummary["health"]): string { switch (health) { - case "healthy": return "green"; - case "warning": return "yellow"; - case "critical": return "red"; - default: return "gray"; + case "healthy": + return "green"; + case "warning": + return "yellow"; + case "critical": + return "red"; + default: + return "gray"; } } diff --git a/platform/supabase/migrations/019_merge_strategy.sql b/platform/supabase/migrations/019_merge_strategy.sql new file mode 100644 index 0000000..22cdbe3 --- /dev/null +++ b/platform/supabase/migrations/019_merge_strategy.sql @@ -0,0 +1,9 @@ +-- Merge Strategy detection (issue #76): per-repository classification of how +-- PRs land on the default branch, plus a per-commit reliability flag. These +-- mirror the engine's `merge_strategy` / `commit_metrics_reliable` fields and +-- are indexed on `metrics` (alongside cascade_rate, ai_detection_coverage_pct, +-- etc.) so the compare view can surface them without reading the full JSONB +-- payload. The full payload still carries `merge_strategy_dominant_share`. +ALTER TABLE metrics + ADD COLUMN merge_strategy TEXT, + ADD COLUMN commit_metrics_reliable BOOLEAN; diff --git a/pyproject.toml b/pyproject.toml index 4f7d283..181b340 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "iris" -version = "1.1.0" +version = "1.2.0" description = "Engineering intelligence for the AI era — measure signal vs noise in software delivery" requires-python = ">=3.11" license = "Apache-2.0" diff --git a/tests/test_github_reader_state.py b/tests/test_github_reader_state.py index d06f83b..3e5f796 100644 --- a/tests/test_github_reader_state.py +++ b/tests/test_github_reader_state.py @@ -18,6 +18,8 @@ _PR_FIELDS_BASIC, _PR_FIELDS_FULL, _infer_state, + _parse_commit_refs, + _parse_merge_commit, _parse_pull_requests, ) @@ -147,3 +149,57 @@ def test_pr_fields_basic_excludes_commits(): def test_pr_fields_full_includes_commits(): """FULL is only used for small windows where the explosion doesn't trigger.""" assert "commits" in _PR_FIELDS_FULL.split(",") + + +def test_pr_fields_include_merge_commit(): + """mergeCommit is cheap (single {oid}) — safe in both field lists.""" + assert "mergeCommit" in _PR_FIELDS_BASIC.split(",") + assert "mergeCommit" in _PR_FIELDS_FULL.split(",") + + +# --------------------------------------------------------------------------- +# Merge-commit + commit-subject parsing (issue #75) +# --------------------------------------------------------------------------- + + +def test_parse_merge_commit_gh_shape_has_no_parent_count(): + # gh's JSON `mergeCommit` field supplies only the oid. + assert _parse_merge_commit({"oid": "abc123"}) == ("abc123", None) + + +def test_parse_merge_commit_graphql_shape_has_parent_count(): + raw = {"oid": "abc123", "parents": {"totalCount": 2}} + assert _parse_merge_commit(raw) == ("abc123", 2) + + +def test_parse_merge_commit_absent_or_null(): + assert _parse_merge_commit(None) == (None, None) + assert _parse_merge_commit({}) == (None, None) + + +def test_parse_commit_refs_captures_subject(): + refs = _parse_commit_refs( + [{"oid": "deadbeef", "messageHeadline": "feat: ship it (#10)"}] + ) + assert refs[0].hash == "deadbeef" + assert refs[0].subject == "feat: ship it (#10)" + + +def test_parse_commit_refs_subject_defaults_empty(): + refs = _parse_commit_refs([{"oid": "deadbeef"}]) + assert refs[0].subject == "" + + +def test_parse_pull_requests_populates_merge_commit_fields(): + raw = _raw(5, created_offset_days=-10, merged_offset_days=-2, state="MERGED") + raw["mergeCommit"] = {"oid": "landed123", "parents": {"totalCount": 1}} + prs = _parse_pull_requests([raw], _SINCE) + assert prs[0].merge_commit_sha == "landed123" + assert prs[0].merge_commit_parent_count == 1 + + +def test_parse_pull_requests_merge_commit_absent_is_none(): + raw = _raw(6, created_offset_days=-10, state="OPEN") + prs = _parse_pull_requests([raw], _SINCE) + assert prs[0].merge_commit_sha is None + assert prs[0].merge_commit_parent_count is None diff --git a/tests/test_merge_strategy_detector.py b/tests/test_merge_strategy_detector.py new file mode 100644 index 0000000..a3c62c1 --- /dev/null +++ b/tests/test_merge_strategy_detector.py @@ -0,0 +1,167 @@ +"""Tests for the Merge Strategy detection module. + +Runnable as: `python -m pytest tests/test_merge_strategy_detector.py -v` +""" + +import sys +from datetime import datetime, timezone +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from iris.analysis.merge_strategy_detector import ( + MIN_CLASSIFIED_PRS, + detect_merge_strategy, + _squash_pr_numbers, +) +from iris.models.commit import Commit +from iris.models.pull_request import CommitRef, PullRequest + +_NOW = datetime(2026, 5, 1, tzinfo=timezone.utc) + + +def _merged_pr( + number: int, + *, + refs: tuple[str, ...] = (), + parent_count: int | None = None, + state: str = "merged", +) -> PullRequest: + return PullRequest( + number=number, + title=f"PR {number}", + author="alice", + created_at=_NOW, + additions=0, + deletions=0, + changed_files=0, + merged_at=_NOW if state == "merged" else None, + closed_at=_NOW if state in ("merged", "closed") else None, + state=state, + commit_refs=[CommitRef(hash=h) for h in refs], + merge_commit_sha=f"merge{number}", + merge_commit_parent_count=parent_count, + ) + + +def _commit(h: str, message: str = "") -> Commit: + return Commit(hash=h, author="alice", message=message) + + +# --------------------------------------------------------------------------- +# Ground truth: 2-parent merge commit +# --------------------------------------------------------------------------- + + +def test_two_parent_merge_commit_classifies_as_merge(): + prs = [_merged_pr(i, parent_count=2) for i in range(1, 7)] + result = detect_merge_strategy(prs, commits=[]) + assert result.merge_strategy == "merge" + assert result.dominant_share == 1.0 + assert result.commit_metrics_reliable is True + + +# --------------------------------------------------------------------------- +# Commit-ref presence heuristic +# --------------------------------------------------------------------------- + + +def test_all_refs_present_in_main_is_merge(): + # Each PR's single commit landed verbatim on main. + prs = [_merged_pr(i, refs=(f"c{i}",), parent_count=1) for i in range(1, 7)] + commits = [_commit(f"c{i}") for i in range(1, 7)] + result = detect_merge_strategy(prs, commits) + assert result.merge_strategy == "merge" + assert result.commit_metrics_reliable is True + + +def test_single_absent_ref_is_squash(): + # One original commit, absent from main (collapsed/rewritten) → squash. + prs = [_merged_pr(i, refs=(f"orig{i}",), parent_count=1) for i in range(1, 7)] + result = detect_merge_strategy(prs, commits=[]) + assert result.merge_strategy == "squash" + assert result.dominant_share == 1.0 + assert result.commit_metrics_reliable is False + + +def test_squash_subject_stamp_corroborates_multi_commit_squash(): + # Multi-commit PRs whose refs are absent but whose number is stamped on a + # main subject "(#N)" — the GitHub squash default → squash. + prs = [ + _merged_pr(i, refs=(f"a{i}", f"b{i}", f"c{i}"), parent_count=1) + for i in range(1, 7) + ] + commits = [_commit(f"landed{i}", f"feat: thing (#{i})") for i in range(1, 7)] + result = detect_merge_strategy(prs, commits) + assert result.merge_strategy == "squash" + assert result.commit_metrics_reliable is False + + +def test_multiple_absent_refs_without_stamp_is_rebase(): + # N original commits, none present, no squash stamp → replayed → rebase. + prs = [ + _merged_pr(i, refs=(f"a{i}", f"b{i}", f"c{i}"), parent_count=1) + for i in range(1, 7) + ] + result = detect_merge_strategy(prs, commits=[]) + assert result.merge_strategy == "rebase" + # Rebase preserves commit count — not flagged unreliable. + assert result.commit_metrics_reliable is True + + +# --------------------------------------------------------------------------- +# Aggregation: mixed / unknown thresholds +# --------------------------------------------------------------------------- + + +def test_split_strategies_is_mixed_and_unreliable(): + # 3 merge (2-parent) + 3 squash (single absent ref) → 50/50 → mixed. + prs = [_merged_pr(i, parent_count=2) for i in range(1, 4)] + prs += [_merged_pr(i, refs=(f"orig{i}",), parent_count=1) for i in range(4, 7)] + result = detect_merge_strategy(prs, commits=[]) + assert result.merge_strategy == "mixed" + assert result.dominant_share == 0.5 + assert result.commit_metrics_reliable is False + + +def test_below_min_classified_is_unknown(): + prs = [_merged_pr(i, parent_count=2) for i in range(1, MIN_CLASSIFIED_PRS)] + result = detect_merge_strategy(prs, commits=[]) + assert result.merge_strategy == "unknown" + assert result.dominant_share is None + # Unknown never flags reliability — we don't claim what we can't determine. + assert result.commit_metrics_reliable is True + assert result.reason is not None + + +def test_no_merged_prs_is_unknown(): + prs = [_merged_pr(1, state="open"), _merged_pr(2, state="closed")] + result = detect_merge_strategy(prs, commits=[]) + assert result.merge_strategy == "unknown" + assert result.classified_pr_count == 0 + assert "no merged PRs" in (result.reason or "") + + +def test_open_and_closed_prs_are_ignored(): + # 5 merged squash + noise from non-merged PRs that must not be counted. + prs = [_merged_pr(i, refs=(f"orig{i}",), parent_count=1) for i in range(1, 6)] + prs += [_merged_pr(99, parent_count=2, state="open")] + prs += [_merged_pr(98, parent_count=2, state="closed")] + result = detect_merge_strategy(prs, commits=[]) + assert result.merge_strategy == "squash" + assert result.classified_pr_count == 5 + + +# --------------------------------------------------------------------------- +# Subject-stamp parsing +# --------------------------------------------------------------------------- + + +def test_squash_pr_numbers_extracts_trailing_stamp_only(): + commits = [ + _commit("a", "fix(navbar): something (#71)"), + _commit("b", "feat: add metric (#56)"), + _commit("c", "chore: no stamp here"), + _commit("d", "refs #999 mid-message, not trailing"), + ] + assert _squash_pr_numbers(commits) == {71, 56}