Fix git-ref --since including the tagged merge PR

github_activity/github_activity.py

@@ -97,6 +97,16 @@
 )
 
 
+def _activity_window_masks(data, since_dt_str, until_dt_str, since_is_git_ref):
+    """Return closed/opened masks; a git-ref ``since`` is exclusive, a date inclusive."""
+    lower = ">" if since_is_git_ref else ">="
+    closed = data.eval(f"closedAt {lower} @since_dt_str and closedAt <= @until_dt_str")
+    opened = data.eval(
+        f"createdAt {lower} @since_dt_str and createdAt <= @until_dt_str"
+    )
+    return closed, opened
+
+
 def get_activity(
     target, since, until=None, repo=None, kind=None, auth=None, cache=None
 ):
@@ -226,6 +236,15 @@ def get_activity(
     query_data = (
         pd.concat(query_data).drop_duplicates(subset=["id"]).reset_index(drop=True)
     )
+
+    # GitHub's search range is inclusive. Apply our public window semantics here
+    # so callers of get_activity() and generate_activity_md() see the same data.
+    if not query_data.empty:
+        closed, opened = _activity_window_masks(
+            query_data, since_dt_str, until_dt_str, since_is_git_ref
+        )
+        query_data = query_data[closed | opened].reset_index(drop=True)
+
     query_data.since_dt = since_dt
     query_data.until_dt = until_dt
     query_data.since_dt_str = since_dt_str
@@ -600,13 +619,10 @@ def filter_ignored(userlist):
     ].index.tolist()
     all_contributors |= set(c for c in comment_contributors if isinstance(c, str))
 
-    # Extract datetime strings from data attributes for pandas query
-    since_dt_str = data.since_dt_str  # noqa: F841
-    until_dt_str = data.until_dt_str  # noqa: F841
-
-    # Separate into closed and opened
-    closed = data.query("closedAt >= @since_dt_str and closedAt <= @until_dt_str")
-    opened = data.query("createdAt >= @since_dt_str and createdAt <= @until_dt_str")
+    closed_mask, opened_mask = _activity_window_masks(
+        data, data.since_dt_str, data.until_dt_str, data.since_is_git_ref
+    )
+    closed, opened = data[closed_mask], data[opened_mask]
 
     # Separate into PRs and issues
     closed_prs = closed.query("kind == 'pr'")

tests/test_activity_window.py

@@ -0,0 +1,131 @@
+"""Tests for changelog activity window filtering."""
+
+import datetime
+
+import pandas as pd
+import pytest
+
+from github_activity.github_activity import generate_activity_md, get_activity
+
+
+def _pr_row(number, title, closed_at):
+    return {
+        "id": f"PR_{number}",
+        "title": title,
+        "url": f"https://github.com/jupyterhub/action-k3s-helm/pull/{number}",
+        "number": number,
+        "state": "MERGED",
+        "kind": "pr",
+        "org": "jupyterhub",
+        "repo": "action-k3s-helm",
+        "author": "manics",
+        "mergedBy": "manics",
+        "committers": ["manics"],
+        "reviewers": [],
+        "comments": {"edges": []},
+        "labels": [],
+        "closedAt": closed_at,
+        "createdAt": "2022-08-20T10:00:00Z",
+        "mergeCommit": {"oid": f"sha{number}"},
+        "baseRefName": "master",
+    }
+
+
+def _mock_get_activity_dependencies(
+    monkeypatch,
+    rows,
+    since_dt,
+    until_dt,
+    since_is_git_ref,
+):
+    datetime_results = iter(
+        [
+            (since_dt, since_is_git_ref),
+            (until_dt, False),
+        ]
+    )
+    monkeypatch.setattr(
+        "github_activity.github_activity._get_datetime_and_type",
+        lambda *args, **kwargs: next(datetime_results),
+    )
+    monkeypatch.setattr(
+        "github_activity.github_activity._validate_repository_exists",
+        lambda *args, **kwargs: None,
+    )
+
+    class FakeGitHubGraphQlQuery:
+        def __init__(self, *args, **kwargs):
+            self.data = pd.DataFrame(rows)
+            self.data.attrs["bot_users"] = set()
+
+        request = lambda self: None  # noqa: E731
+
+    monkeypatch.setattr(
+        "github_activity.github_activity.GitHubGraphQlQuery",
+        FakeGitHubGraphQlQuery,
+    )
+
+
+@pytest.mark.parametrize("api", [get_activity, generate_activity_md])
+@pytest.mark.parametrize(
+    "since,since_dt,since_is_git_ref,expected",
+    [
+        ("v3.0.5", "2022-08-21T13:29:31+00:00", True, [74]),
+        ("2022-08-21", "2022-08-21T00:00:00+00:00", False, [73, 74]),
+    ],
+    ids=["git-ref-exclusive", "date-inclusive"],
+)
+def test_activity_window(monkeypatch, api, since, since_dt, since_is_git_ref, expected):
+    """Both APIs exclude a git-ref boundary but include a date boundary (#79)."""
+    since_dt = datetime.datetime.fromisoformat(since_dt)
+    until_dt = datetime.datetime(2022, 11, 15, 19, 11, 20, tzinfo=datetime.timezone.utc)
+    rows = [
+        _pr_row(
+            number,
+            title,
+            f"{since_dt + datetime.timedelta(hours=offset):%Y-%m-%dT%H:%M:%SZ}",
+        )
+        for number, title, offset in [
+            (72, "Closed before boundary", -1),
+            (73, "Closed at boundary", 0),
+            (74, "Closed after boundary", 1),
+        ]
+    ]
+    _mock_get_activity_dependencies(
+        monkeypatch, rows, since_dt, until_dt, since_is_git_ref
+    )
+
+    result = api(
+        target="jupyterhub/action-k3s-helm",
+        since=since,
+        until="2022-11-15T19:11:20Z",
+        auth="test-token",
+    )
+
+    if api is get_activity:
+        assert result["number"].tolist() == expected
+    else:
+        for number in [72, 73, 74]:
+            assert (f"[#{number}]" in result) == (number in expected)
+
+
+@pytest.mark.parametrize("api", [get_activity, generate_activity_md])
+def test_empty_activity(monkeypatch, api):
+    """Empty searches return an empty frame or the intended Markdown error."""
+    since_dt = datetime.datetime(2022, 8, 21, tzinfo=datetime.timezone.utc)
+    until_dt = datetime.datetime(2022, 8, 22, tzinfo=datetime.timezone.utc)
+    _mock_get_activity_dependencies(
+        monkeypatch, [], since_dt, until_dt, since_is_git_ref=False
+    )
+    kwargs = dict(
+        target="jupyterhub/action-k3s-helm",
+        since="2022-08-21",
+        until="2022-08-22",
+        auth="test-token",
+    )
+
+    if api is get_activity:
+        assert api(**kwargs).empty
+    else:
+        with pytest.raises(ValueError, match="No activity found"):
+            api(**kwargs)