You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "potiuk (via GitHub)" <gi...@apache.org> on 2026/06/11 00:11:57 UTC

[PR] feat(pr-management-stats): dashboard.py parity — render new panels + publish-to-gist [airflow-steward]

potiuk opened a new pull request, #487:
URL: https://github.com/apache/airflow-steward/pull/487

   Follow-up to #486 (the spec, now merged) — brings the deterministic CI-parity reference implementation (`tools/pr-management-stats/`) up to that spec, so a CI-scheduled `dashboard.py` render matches the agent-emitted one. Rebased onto `main` after #486 merged.
   
   ## What
   
   - **Ready-for-review queue split** — `compute_ready_split` / `render_ready_split`: the non-maintainer ready queue split into never-reviewed / discussed-no-decision / changes-requested / approved, as coloured cards + an age timeline (oldest bucket on the left).
   - **Drafts & closes attribution by person** — `compute_attribution` / `render_attribution`: draft-conversions (`ConvertToDraftEvent`) and closes (`ClosedEvent`) attributed **triage-action (actor ≠ author) vs author-self** with per-maintainer shares; **bots and backport PRs excluded**; maintainer set derived from OWNER/MEMBER/COLLABORATOR commenters (no hardcoded list).
   - **Always-publish-to-gist** — `publish_gist` + session-state helpers + `gh` gist-scope check; `--no-publish` / `--dry-run` flags; `main()` always publishes to a stable secret gist (in-place PATCH) and prints the `gistpreview.github.io` URL.
   
   `reference.py` query additions: `baseRefName`, `reviewDecision`, `__typename` on timeline items (open PRs); `baseRefName` + `CLOSED_EVENT` actor (closed PRs).
   
   ## Tests
   
   New `tests/test_ready_split.py`, `tests/test_attribution.py`, `tests/test_publish.py` (no network — mocked `subprocess`). **41 tests pass**; ruff / mypy / typos / pytest hooks green.
   
   ## Notes
   
   - Gitignores the per-repo `.apache-magpie.session-state.json` (stores the stats gist id; also used by triage session-history).
   - `__typename` added to the open-PR timeline query to robustly distinguish `ConvertToDraftEvent` from `LabeledEvent` / `ReadyForReviewEvent` (same actor+createdAt shape) rather than a fragile field-presence heuristic.
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes — Claude Code (Opus 4.8 1M context)
   
   Generated-by: Claude Code (Opus 4.8 1M context)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(pr-management-stats): dashboard.py parity — render new panels + publish-to-gist [airflow-steward]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #487:
URL: https://github.com/apache/airflow-steward/pull/487


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(pr-management-stats): dashboard.py parity — render new panels + publish-to-gist [airflow-steward]

Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #487:
URL: https://github.com/apache/airflow-steward/pull/487#discussion_r3392397729


##########
tools/pr-management-stats/tests/test_ready_split.py:
##########
@@ -0,0 +1,157 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Tests for compute_ready_split (render.md § Ready-for-review queue split)."""
+from __future__ import annotations
+
+import dashboard
+import reference
+from helpers import classify_all, comment, make_ctx, make_pr
+
+QC = reference.DEFAULT_TRIAGE_MARKER
+READY = reference.DEFAULT_READY_LABEL
+
+
+def _ready(number, **kw):
+    labels = kw.pop("labels", [])
+    return make_pr(number, labels=[READY, *labels], **kw)
+
+
+def test_ready_split_classifies_each_substate_once():
+    ctx = make_ctx()
+    prs = classify_all(
+        [
+            # never reviewed: ready, no decision, no maintainer discussion
+            _ready(1, author="alice", assoc="NONE", created_days_ago=5),
+            # changes requested
+            _ready(2, author="bob", assoc="NONE", created_days_ago=20),
+            # approved
+            _ready(3, author="carol", assoc="NONE", created_days_ago=40),
+            # discussed-no-decision: real (non-marker) maintainer comment
+            _ready(
+                4, author="dave", assoc="NONE", created_days_ago=70,
+                comments=[comment("can you rebase?", author="maint", assoc="MEMBER")],
+            ),
+        ],
+        ctx,
+    )
+    # Inject review decisions (classify reads pr["reviewDecision"]).
+    prs[1]["_review_decision"] = "CHANGES_REQUESTED"
+    prs[2]["_review_decision"] = "APPROVED"
+
+    split = dashboard.compute_ready_split(prs, ctx)
+    c = split["counts"]
+    assert c["never-reviewed"] == 1
+    assert c["changes-requested"] == 1
+    assert c["approved"] == 1
+    assert c["discussed-no-decision"] == 1
+    assert split["total"] == 4
+    assert split["excluded_maintainer"] == 0
+
+
+def test_triage_marker_comment_does_not_count_as_discussion():
+    """A maintainer comment that is ONLY the triage marker must leave the PR in
+    `never-reviewed`, not `discussed-no-decision`."""
+    ctx = make_ctx()
+    prs = classify_all(
+        [
+            _ready(
+                10, author="alice", assoc="NONE", created_days_ago=5,
+                comments=[comment(f"... {QC} ...", author="maint", assoc="MEMBER")],
+            ),
+        ],
+        ctx,
+    )
+    split = dashboard.compute_ready_split(prs, ctx)
+    assert split["counts"]["never-reviewed"] == 1
+    assert split["counts"]["discussed-no-decision"] == 0
+
+
+def test_commented_review_counts_as_discussion():
+    ctx = make_ctx()
+    prs = classify_all(
+        [
+            _ready(
+                11, author="alice", assoc="NONE", created_days_ago=5,
+                reviews=[{"author": {"login": "maint"}, "state": "COMMENTED",
+                          "submittedAt": None}],
+            ),
+        ],
+        ctx,
+    )
+    split = dashboard.compute_ready_split(prs, ctx)
+    assert split["counts"]["discussed-no-decision"] == 1
+    assert split["counts"]["never-reviewed"] == 0
+
+
+def test_maintainer_authored_ready_prs_are_excluded():
+    ctx = make_ctx()
+    prs = classify_all(
+        [
+            _ready(20, author="maint", assoc="MEMBER", created_days_ago=5),
+            _ready(21, author="alice", assoc="NONE", created_days_ago=5),
+        ],
+        ctx,
+    )
+    split = dashboard.compute_ready_split(prs, ctx)
+    assert split["total"] == 1
+    assert split["excluded_maintainer"] == 1
+
+
+def test_age_buckets_in_timeline():
+    ctx = make_ctx()
+    prs = classify_all(
+        [
+            _ready(30, author="a", assoc="NONE", created_days_ago=3),   # 0-2w
+            _ready(31, author="b", assoc="NONE", created_days_ago=20),  # 2-4w
+            _ready(32, author="c", assoc="NONE", created_days_ago=50),  # 4-8w
+            _ready(33, author="d", assoc="NONE", created_days_ago=70),  # 8-12w
+            _ready(34, author="e", assoc="NONE", created_days_ago=120),  # >12w
+        ],
+        ctx,
+    )
+    split = dashboard.compute_ready_split(prs, ctx)
+    tl = split["timeline"]
+    assert tl["0-2w"]["never-reviewed"] == 1
+    assert tl["2-4w"]["never-reviewed"] == 1
+    assert tl["4-8w"]["never-reviewed"] == 1
+    assert tl["8-12w"]["never-reviewed"] == 1
+    assert tl[">12w"]["never-reviewed"] == 1
+
+
+def test_render_ready_split_oldest_on_left_and_cards():
+    ctx = make_ctx()
+    prs = classify_all(
+        [_ready(40, author="a", assoc="NONE", created_days_ago=3)], ctx
+    )
+    split = dashboard.compute_ready_split(prs, ctx)
+    out = dashboard.render_ready_split(split)
+    assert "Ready-for-review queue split" in out
+    assert "Never reviewed" in out
+    # Oldest-on-left: the >12w x-axis label must appear before 0-2w in the SVG.
+    # esc() HTML-escapes the ">" in the bucket label to "&gt;".
+    assert out.index("&gt;12w") < out.index("0-2w")
+
+
+def test_render_ready_split_empty_state():
+    ctx = make_ctx()

Review Comment:
   ## CodeQL / Unused local variable
   
   Variable ctx is not used.
   
   [Show more details](https://github.com/apache/airflow-steward/security/code-scanning/16)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org