Skip to content

Commit 1af728b

Browse files
Fix reviewThreads and --format arg bugs
- Remove reviewThreads from gh pr view (not a valid field); fetch via GraphQL API instead - Move --format from parent parser to subcommands so `diagnose --format json <url>` works - Update parser tests for per-subcommand format flag
1 parent 2798bdb commit 1af728b

2 files changed

Lines changed: 77 additions & 46 deletions

File tree

pr-status/scripts/pr_status.py

Lines changed: 64 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,45 @@ def cmd_missing_checks(args: argparse.Namespace) -> int:
636636
return 0
637637

638638

639+
def _fetch_unresolved_thread_count(pr: str, repo: str | None) -> int:
640+
"""Fetch count of unresolved review threads via GraphQL."""
641+
effective_repo = repo or _detect_local_repo()
642+
if not effective_repo:
643+
return 0
644+
owner, name = effective_repo.split("/")
645+
query = """
646+
query($owner: String!, $name: String!, $pr: Int!) {
647+
repository(owner: $owner, name: $name) {
648+
pullRequest(number: $pr) {
649+
reviewThreads(first: 100) {
650+
nodes { isResolved }
651+
}
652+
}
653+
}
654+
}
655+
"""
656+
try:
657+
result = subprocess.run(
658+
["gh", "api", "graphql",
659+
"-F", f"owner={owner}", "-F", f"name={name}", "-F", f"pr={pr}",
660+
"-f", f"query={query}"],
661+
capture_output=True, text=True, timeout=30,
662+
)
663+
if result.returncode != 0:
664+
return 0
665+
data = json.loads(result.stdout)
666+
threads = (
667+
data.get("data", {})
668+
.get("repository", {})
669+
.get("pullRequest", {})
670+
.get("reviewThreads", {})
671+
.get("nodes", [])
672+
)
673+
return sum(1 for t in threads if not t.get("isResolved", True))
674+
except (subprocess.TimeoutExpired, json.JSONDecodeError, RuntimeError):
675+
return 0
676+
677+
639678
def cmd_diagnose(args: argparse.Namespace) -> int:
640679
"""Full blocker report — the primary command."""
641680
pr, repo = _resolve_pr(args)
@@ -646,7 +685,7 @@ def cmd_diagnose(args: argparse.Namespace) -> int:
646685
"pr", "view", pr, "--json",
647686
"number,title,state,isDraft,mergeable,mergeStateStatus,"
648687
"reviewDecision,reviewRequests,latestReviews,"
649-
"headRefName,baseRefName,statusCheckRollup,reviewThreads",
688+
"headRefName,baseRefName,statusCheckRollup",
650689
repo=repo,
651690
)
652691

@@ -713,14 +752,13 @@ def cmd_diagnose(args: argparse.Namespace) -> int:
713752
"fix": "Request review or wait for pending reviewers",
714753
})
715754

716-
# 4. Unresolved review threads
717-
threads = pr_data.get("reviewThreads") or []
718-
unresolved = [t for t in threads if not t.get("isResolved", True)]
719-
if unresolved:
755+
# 4. Unresolved review threads (via GraphQL — not available in gh pr view --json)
756+
unresolved_count = _fetch_unresolved_thread_count(pr, repo)
757+
if unresolved_count > 0:
720758
blockers.append({
721759
"type": "unresolved_threads",
722-
"count": len(unresolved),
723-
"message": f"{len(unresolved)} unresolved review thread(s)",
760+
"count": unresolved_count,
761+
"message": f"{unresolved_count} unresolved review thread(s)",
724762
"fix": "Resolve all review conversations",
725763
})
726764

@@ -869,50 +907,32 @@ def build_parser() -> argparse.ArgumentParser:
869907
description="PR Status Analyzer — diagnose why a PR can't merge",
870908
)
871909
parser.add_argument("--repo", help="Repository in owner/repo format")
872-
parser.add_argument("--format", choices=["text", "json"], default="text", help="Output format")
873910

874911
sub = parser.add_subparsers(dest="command", required=True)
875912

876-
# diagnose
877-
p = sub.add_parser("diagnose", help="Full blocker report (primary command)")
878-
p.add_argument("pr", nargs="?", help="PR number or URL")
879-
880-
# status
881-
p = sub.add_parser("status", help="PR metadata")
882-
p.add_argument("pr", nargs="?", help="PR number or URL")
883-
884-
# checks
885-
p = sub.add_parser("checks", help="Detailed check results")
886-
p.add_argument("pr", nargs="?", help="PR number or URL")
887-
888-
# comments
889-
p = sub.add_parser("comments", help="All PR comments and reviews")
890-
p.add_argument("pr", nargs="?", help="PR number or URL")
891-
892-
# bot-comments
893-
p = sub.add_parser("bot-comments", help="Bot comments with error keywords")
894-
p.add_argument("pr", nargs="?", help="PR number or URL")
895-
896-
# failed-runs
897-
p = sub.add_parser("failed-runs", help="Extract run IDs from failed checks")
898-
p.add_argument("pr", nargs="?", help="PR number or URL")
899-
900-
# analyze-logs
913+
def _add_pr_subcommand(name: str, help_text: str, *, has_pr: bool = True, has_format: bool = True) -> argparse.ArgumentParser:
914+
p = sub.add_parser(name, help=help_text)
915+
if has_format:
916+
p.add_argument("--format", choices=["text", "json"], default="text", help="Output format")
917+
if has_pr:
918+
p.add_argument("pr", nargs="?", help="PR number or URL")
919+
return p
920+
921+
_add_pr_subcommand("diagnose", "Full blocker report (primary command)")
922+
_add_pr_subcommand("status", "PR metadata")
923+
_add_pr_subcommand("checks", "Detailed check results")
924+
_add_pr_subcommand("comments", "All PR comments and reviews")
925+
_add_pr_subcommand("bot-comments", "Bot comments with error keywords")
926+
_add_pr_subcommand("failed-runs", "Extract run IDs from failed checks")
927+
_add_pr_subcommand("required-checks", "Branch protection required checks")
928+
_add_pr_subcommand("missing-checks", "Compare required vs actual checks")
929+
930+
# analyze-logs has run_id instead of pr
901931
p = sub.add_parser("analyze-logs", help="Download and analyze failed CI logs")
902932
p.add_argument("run_id", help="GitHub Actions run ID")
903933

904-
# required-checks
905-
p = sub.add_parser("required-checks", help="Branch protection required checks")
906-
p.add_argument("pr", nargs="?", help="PR number or URL")
907-
908-
# missing-checks
909-
p = sub.add_parser("missing-checks", help="Compare required vs actual checks")
910-
p.add_argument("pr", nargs="?", help="PR number or URL")
911-
912-
# check-cli
934+
# Utility commands (no format, no pr)
913935
sub.add_parser("check-cli", help="Verify gh auth and repo access")
914-
915-
# detect-repo
916936
sub.add_parser("detect-repo", help="Detect GitHub repo from git remote")
917937

918938
return parser

pr-status/scripts/test_pr_status.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,11 +360,22 @@ def test_global_repo_flag(self):
360360
assert args.repo == "org/repo"
361361
assert args.command == "status"
362362

363-
def test_global_format_flag(self):
363+
def test_subcommand_format_flag(self):
364364
parser = build_parser()
365-
args = parser.parse_args(["--format", "json", "checks"])
365+
args = parser.parse_args(["checks", "--format", "json"])
366366
assert args.format == "json"
367367

368+
def test_format_flag_with_pr_url(self):
369+
parser = build_parser()
370+
args = parser.parse_args(["diagnose", "--format", "json", "https://github.com/o/r/pull/1"])
371+
assert args.format == "json"
372+
assert args.pr == "https://github.com/o/r/pull/1"
373+
374+
def test_format_default_is_text(self):
375+
parser = build_parser()
376+
args = parser.parse_args(["diagnose"])
377+
assert args.format == "text"
378+
368379

369380
if __name__ == "__main__":
370381
pytest.main([__file__, "-v"])

0 commit comments

Comments
 (0)