-
Notifications
You must be signed in to change notification settings - Fork 0
PR-API channel: optional collect-pr command for review comments (#5) #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| """Optional PR-API channel — the only network-touching module in the package. | ||
|
|
||
| Records review comments and suggestion fences per document as a separate | ||
| human-signal channel (design: issue #5). Squash merges and closed-unmerged | ||
| PRs hide this signal from git entirely; the channel recovers it without ever | ||
| blending it into blame. The scan neither reads nor writes these artefacts, | ||
| so its determinism contract is untouched. | ||
|
|
||
| pr_channel.jsonl one record per review comment anchored to a configured document | ||
| pr_channel.json collection summary (volumes by kind, role and PR state) | ||
| """ | ||
| from __future__ import annotations | ||
|
|
||
| import fnmatch | ||
| import json | ||
| import re | ||
| import subprocess | ||
| import sys | ||
| from collections import Counter | ||
| from datetime import UTC, datetime | ||
| from pathlib import Path | ||
|
|
||
| from .config import Config, ConfigError | ||
| from .roster import Roster | ||
|
|
||
| SUGGESTION = re.compile(r"```suggestion[^\n]*\n(.*?)```", re.DOTALL) | ||
|
|
||
|
|
||
| class GhError(RuntimeError): | ||
| pass | ||
|
|
||
|
|
||
| def _concat(text: str): | ||
| """`gh api --paginate` concatenates JSON documents; merge arrays, pass dicts through.""" | ||
| dec = json.JSONDecoder() | ||
| idx, out, text = 0, None, text.strip() | ||
| while idx < len(text): | ||
| obj, idx = dec.raw_decode(text, idx) | ||
| if isinstance(obj, list): | ||
| out = out if isinstance(out, list) else [] | ||
| out.extend(obj) | ||
| else: | ||
| out = obj | ||
| while idx < len(text) and text[idx] in " \n\r\t": | ||
| idx += 1 | ||
| return [] if out is None else out | ||
|
|
||
|
|
||
| def gh_api(path: str): | ||
| try: | ||
| r = subprocess.run(["gh", "api", "--paginate", path], capture_output=True, check=False) | ||
| except FileNotFoundError as e: | ||
| raise GhError("gh is not installed — collect-pr requires an authenticated GitHub CLI") from e | ||
| if r.returncode != 0: | ||
| raise GhError(f"gh api {path}: {r.stderr.decode(errors='replace')[:400]}") | ||
| return _concat(r.stdout.decode("utf-8", errors="replace")) | ||
|
|
||
|
|
||
| def _hunk_tail(diff_hunk: str) -> str: | ||
| """The commented line: the last content line of the comment's diff hunk.""" | ||
| lines = [ln for ln in (diff_hunk or "").splitlines() if not ln.startswith("@@")] | ||
| return lines[-1][1:] if lines else "" | ||
|
|
||
|
|
||
| def collect(cfg: Config, out_dir: Path, log=sys.stderr, api=gh_api) -> dict: | ||
| pc = cfg.pr_channel | ||
| if not pc.repo: | ||
| raise ConfigError("collect-pr needs pr_channel.repo (an owner/name slug) in the config") | ||
| roster = Roster.load(cfg.resolve(cfg.people.roster)) | ||
| bots = [re.compile(p, re.IGNORECASE) for p in cfg.machine.bots + cfg.disclosure.authors] | ||
| out_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| comments = api(f"repos/{pc.repo}/pulls/comments?per_page=100&sort=created&direction=asc") | ||
| pr_state: dict[int, str] = {} | ||
| records: list[dict] = [] | ||
| for c in comments: | ||
| path = c.get("path") or "" | ||
| if not fnmatch.fnmatch(path, cfg.files): | ||
| continue | ||
| if pc.since and (c.get("created_at") or "") < pc.since: | ||
| continue | ||
| pr = int(c["pull_request_url"].rstrip("/").rsplit("/", 1)[-1]) | ||
| if pr not in pr_state: | ||
| p = api(f"repos/{pc.repo}/pulls/{pr}") | ||
| # self-describing states: "closed" without a merge is the channel's key case | ||
| pr_state[pr] = ("merged" if p.get("merged_at") | ||
| else "closed-unmerged" if p.get("state") == "closed" | ||
| else p.get("state", "unknown")) | ||
| login = (c.get("user") or {}).get("login") or "" | ||
| person = roster.by_handle.get(login.lower()) | ||
| # the comments endpoint reports Copilot's reviewer as a bare "Copilot" login | ||
| bot = login.lower() == "copilot" or any(p.search(login) for p in bots) | ||
| body = c.get("body") or "" | ||
| m = SUGGESTION.search(body) | ||
| records.append({"pr": pr, "pr_state": pr_state[pr], "comment_id": c["id"], | ||
| "document": path, "line": c.get("line") or c.get("original_line"), | ||
| "author_login": login, "roster_id": person.id if person else None, | ||
| "role": person.role if person else None, "bot": bot, | ||
| "kind": "suggestion" if m else "comment", | ||
| "created_at": c.get("created_at"), "body_chars": len(body), | ||
| "in_reply_to": c.get("in_reply_to_id"), | ||
| "before": _hunk_tail(c.get("diff_hunk", "")) if m else None, | ||
| "after": m.group(1).rstrip("\n") if m else None}) | ||
| records.sort(key=lambda r: (r["pr"], r["comment_id"])) | ||
|
|
||
| with (out_dir / "pr_channel.jsonl").open("w", encoding="utf-8") as fh: | ||
| for r in records: | ||
| fh.write(json.dumps(r, ensure_ascii=False) + "\n") | ||
| summary = { | ||
| "repo": pc.repo, | ||
| "collected_at": datetime.now(UTC).isoformat(timespec="seconds"), | ||
| "comments": len(records), | ||
| "suggestions": sum(1 for r in records if r["kind"] == "suggestion"), | ||
| "human": sum(1 for r in records if not r["bot"]), | ||
| "bot": sum(1 for r in records if r["bot"]), | ||
| "documents": len({r["document"] for r in records}), | ||
| "by_role": dict(Counter(r["role"] or ("bot" if r["bot"] else "unresolved") for r in records)), | ||
| "pr_states": dict(Counter(r["pr_state"] for r in records)), | ||
| } | ||
| (out_dir / "pr_channel.json").write_text(json.dumps(summary, ensure_ascii=False, indent=1), | ||
| encoding="utf-8") | ||
| print(f"{pc.repo}: {summary['comments']} review comments on {summary['documents']} documents " | ||
| f"({summary['human']} human, {summary['bot']} bot; {summary['suggestions']} suggestions)", | ||
| file=log) | ||
| return summary | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| """The PR-API channel collector against a recorded, stubbed API — no network.""" | ||
| import json | ||
| import re | ||
| import sys | ||
|
|
||
| import pytest | ||
|
|
||
| from textstrata.config import Config, ConfigError | ||
| from textstrata.pr_channel import _concat, _hunk_tail, collect | ||
|
|
||
| COMMENTS = [ | ||
| {"id": 11, "path": "lectures/a.md", "line": 12, "user": {"login": "HumphreyYang"}, | ||
| "body": "This is reverting code comments back to English", | ||
| "pull_request_url": "https://api.github.com/repos/o/r/pulls/7", | ||
| "created_at": "2026-04-14T01:00:00Z", | ||
| "diff_hunk": "@@ -10,2 +10,2 @@\n 上下文行\n+# Update wealth"}, | ||
| {"id": 12, "path": "lectures/a.md", "line": 13, "user": {"login": "HumphreyYang"}, | ||
| "body": "请修正:\n```suggestion\n# 更新财富\n```\n如上。", | ||
| "pull_request_url": "https://api.github.com/repos/o/r/pulls/7", | ||
| "created_at": "2026-04-14T01:05:00Z", | ||
| "diff_hunk": "@@ -13 +13 @@\n+# Update wealth"}, | ||
| {"id": 13, "path": "lectures/b.md", "line": 2, "user": {"login": "Copilot"}, | ||
| "body": "Consider handling the empty case.", | ||
| "pull_request_url": "https://api.github.com/repos/o/r/pulls/9", | ||
| "created_at": "2026-05-01T00:00:00Z", "diff_hunk": ""}, | ||
| # outside the files glob: never collected | ||
| {"id": 14, "path": "README.md", "line": 1, "user": {"login": "HumphreyYang"}, | ||
| "body": "readme note", "pull_request_url": "https://api.github.com/repos/o/r/pulls/7", | ||
| "created_at": "2026-04-14T02:00:00Z", "diff_hunk": ""}, | ||
| # before `since`: skipped | ||
| {"id": 15, "path": "lectures/a.md", "line": 3, "user": {"login": "someoneelse"}, | ||
| "body": "early note", "pull_request_url": "https://api.github.com/repos/o/r/pulls/7", | ||
| "created_at": "2026-01-01T00:00:00Z", "diff_hunk": ""}, | ||
| ] | ||
| PRS = {7: {"merged_at": None, "state": "closed"}, | ||
| 9: {"merged_at": "2026-05-02T00:00:00Z", "state": "closed"}} | ||
|
|
||
|
|
||
| def fake_api(path): | ||
| if "pulls/comments" in path: | ||
| return COMMENTS | ||
| m = re.search(r"/pulls/(\d+)$", path) | ||
| return PRS[int(m.group(1))] | ||
|
|
||
|
|
||
| def make_cfg(tmp_path): | ||
| (tmp_path / "roster.yml").write_text( | ||
| "people:\n - id: HumphreyYang\n role: editor\n emails: [x@example.org]\n", | ||
| encoding="utf-8") | ||
| cfg = Config(name="t", repo=tmp_path, base_dir=tmp_path) | ||
| cfg.pr_channel.repo = "o/r" | ||
| cfg.pr_channel.since = "2026-02-01" | ||
| cfg.people.roster = "roster.yml" | ||
| return cfg | ||
|
|
||
|
|
||
| def test_collect(tmp_path, capsys): | ||
| cfg = make_cfg(tmp_path) | ||
| summary = collect(cfg, tmp_path / "out", log=sys.stderr, api=fake_api) | ||
| recs = [json.loads(ln) for ln in (tmp_path / "out" / "pr_channel.jsonl").open(encoding="utf-8")] | ||
| assert [r["comment_id"] for r in recs] == [11, 12, 13] # glob and since filters applied, sorted | ||
| by_id = {r["comment_id"]: r for r in recs} | ||
| # roster resolution by login, and the closed-unmerged PR is the point of the channel | ||
| assert by_id[11]["roster_id"] == "HumphreyYang" and by_id[11]["role"] == "editor" | ||
| assert by_id[11]["pr_state"] == "closed-unmerged" and by_id[13]["pr_state"] == "merged" | ||
| # suggestion fences carry the before (hunk tail) / after (fence body) pair | ||
| assert by_id[12]["kind"] == "suggestion" | ||
| assert by_id[12]["before"] == "# Update wealth" and by_id[12]["after"] == "# 更新财富" | ||
| assert by_id[11]["kind"] == "comment" and by_id[11]["before"] is None | ||
| # the Copilot reviewer is collected as a bot, not dropped | ||
| assert by_id[13]["bot"] is True and by_id[11]["bot"] is False | ||
| assert summary["comments"] == 3 and summary["suggestions"] == 1 | ||
| assert summary["human"] == 2 and summary["bot"] == 1 | ||
| assert summary["by_role"] == {"editor": 2, "bot": 1} | ||
| assert summary["pr_states"] == {"closed-unmerged": 2, "merged": 1} | ||
| assert "3 review comments" in capsys.readouterr().err | ||
|
|
||
|
|
||
| def test_collect_needs_repo(tmp_path): | ||
| cfg = make_cfg(tmp_path) | ||
| cfg.pr_channel.repo = None | ||
| with pytest.raises(ConfigError, match="pr_channel.repo"): | ||
| collect(cfg, tmp_path / "out", api=fake_api) | ||
|
|
||
|
|
||
| def test_concat_and_hunk_tail(): | ||
| # gh --paginate concatenates array pages; single documents pass through | ||
| assert _concat('[{"a": 1}]\n[{"b": 2}]') == [{"a": 1}, {"b": 2}] | ||
| assert _concat('{"state": "closed"}') == {"state": "closed"} | ||
| assert _concat("") == [] | ||
| assert _hunk_tail("@@ -1,2 +1,2 @@\n context\n+new line") == "new line" | ||
| assert _hunk_tail("") == "" |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.