Skip to content

Commit 4d717d2

Browse files
authored
⚡ perf: avoid timeline bootstrap for lightweight PR views (#30)
1 parent 02bbf68 commit 4d717d2

5 files changed

Lines changed: 181 additions & 55 deletions

File tree

src/gh_llm/commands/pr.py

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
from gh_llm.github_api import GitHubClient
1111
from gh_llm.invocation import display_command_with
1212
from gh_llm.models import PullRequestDiffPage
13-
from gh_llm.pager import DEFAULT_PAGE_SIZE, TimelinePager
13+
from gh_llm.pager import DEFAULT_PAGE_SIZE, TimelinePager, build_context_from_meta
1414
from gh_llm.render import (
1515
render_checks_section,
1616
render_comment_node_detail,
@@ -325,16 +325,22 @@ def cmd_pr_view(args: Any) -> int:
325325
pager = TimelinePager(client)
326326

327327
meta = client.resolve_pull_request(selector=args.pr, repo=args.repo)
328-
context, first_page, last_page = pager.build_initial(
329-
meta,
330-
page_size=page_size,
331-
show_resolved_details=expand.resolved,
332-
show_outdated_details=True,
333-
show_minimized_details=expand.minimized,
334-
show_details_blocks=expand.details,
335-
diff_hunk_lines=diff_hunk_lines,
336-
)
337-
shown_pages: set[int] = {1}
328+
context = build_context_from_meta(meta=meta, page_size=page_size)
329+
first_page: TimelinePage | None = None
330+
last_page: TimelinePage | None = None
331+
shown_pages: set[int] = set()
332+
333+
if show.timeline:
334+
context, first_page, last_page = pager.build_initial(
335+
meta,
336+
page_size=page_size,
337+
show_resolved_details=expand.resolved,
338+
show_outdated_details=True,
339+
show_minimized_details=expand.minimized,
340+
show_details_blocks=expand.details,
341+
diff_hunk_lines=diff_hunk_lines,
342+
)
343+
shown_pages.add(1)
338344

339345
wrote_output = False
340346

@@ -353,6 +359,7 @@ def print_block(lines: list[str]) -> None:
353359
if show.description:
354360
print_block(render_description(context))
355361
if show.timeline:
362+
assert first_page is not None
356363
print_block(["## Timeline"])
357364
print_block(render_page(1, context, first_page))
358365

@@ -565,9 +572,9 @@ def cmd_pr_thread_expand(args: Any) -> int:
565572

566573
def cmd_pr_checks(args: Any) -> int:
567574
client = GitHubClient()
568-
pager = TimelinePager(client)
569-
context, meta = _resolve_context_and_meta(client=client, pager=pager, args=args)
570-
checks = client.fetch_checks(meta.ref)
575+
meta = _resolve_pr_meta(client=client, args=args)
576+
context = build_context_from_meta(meta=meta, page_size=DEFAULT_PAGE_SIZE)
577+
checks = client.fetch_checks(meta.ref) if meta.state == "OPEN" else []
571578
for line in render_checks_section(
572579
context=context,
573580
checks=checks,

src/gh_llm/models.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ class TimelineContext:
155155
is_draft: bool
156156
body: str
157157
updated_at: str
158+
timeline_loaded: bool = True
158159
labels: tuple[str, ...] = ()
159160
kind: str = "pr"
160161
pr_reactions_summary: str | None = None
@@ -198,6 +199,7 @@ def to_dict(self) -> dict[str, object]:
198199
"is_draft": self.is_draft,
199200
"body": self.body,
200201
"updated_at": self.updated_at,
202+
"timeline_loaded": self.timeline_loaded,
201203
"labels": list(self.labels),
202204
"kind": self.kind,
203205
"pr_reactions_summary": self.pr_reactions_summary,
@@ -243,6 +245,11 @@ def from_dict(cls, value: dict[str, object]) -> TimelineContext:
243245
is_draft=bool(value.get("is_draft")),
244246
body=_as_str(value.get("body"), ""),
245247
updated_at=_as_str(value.get("updated_at"), ""),
248+
timeline_loaded=(
249+
_as_int(value.get("total_pages"), 0) > 0
250+
if value.get("timeline_loaded") is None
251+
else bool(value.get("timeline_loaded"))
252+
),
246253
labels=tuple(_as_str(item, "") for item in _as_list(value.get("labels")) if item),
247254
kind=_as_str(value.get("kind"), "pr"),
248255
pr_reactions_summary=_as_str_optional(value.get("pr_reactions_summary")),

src/gh_llm/pager.py

Lines changed: 60 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,64 @@
1212
DEFAULT_PAGE_SIZE = 8
1313

1414

15+
def build_context_from_meta(
16+
meta: PullRequestMeta,
17+
page_size: int,
18+
*,
19+
total_count: int | None = None,
20+
total_pages: int | None = None,
21+
) -> TimelineContext:
22+
_validate_page_size(page_size)
23+
24+
timeline_loaded = total_count is not None and total_pages is not None
25+
resolved_total_count = 0 if total_count is None else total_count
26+
resolved_total_pages = 0 if total_pages is None else total_pages
27+
28+
return TimelineContext(
29+
owner=meta.ref.owner,
30+
name=meta.ref.name,
31+
number=meta.ref.number,
32+
page_size=page_size,
33+
total_count=resolved_total_count,
34+
total_pages=resolved_total_pages,
35+
title=meta.title,
36+
url=meta.url,
37+
author=meta.author,
38+
state=meta.state,
39+
is_draft=meta.is_draft,
40+
body=meta.body,
41+
updated_at=meta.updated_at,
42+
timeline_loaded=timeline_loaded,
43+
labels=meta.labels,
44+
kind=meta.kind,
45+
pr_reactions_summary=meta.reactions_summary,
46+
can_edit_pr_body=meta.can_edit_body,
47+
is_merged=meta.is_merged,
48+
head_ref_name=meta.head_ref_name,
49+
head_ref_repo=meta.head_ref_repo,
50+
head_ref_oid=meta.head_ref_oid,
51+
head_ref_deleted=meta.head_ref_deleted,
52+
pr_node_id=meta.node_id,
53+
merge_state_status=meta.merge_state_status,
54+
mergeable=meta.mergeable,
55+
review_decision=meta.review_decision,
56+
requires_approving_reviews=meta.requires_approving_reviews,
57+
required_approving_review_count=meta.required_approving_review_count,
58+
requires_code_owner_reviews=meta.requires_code_owner_reviews,
59+
approved_review_count=meta.approved_review_count,
60+
requires_status_checks=meta.requires_status_checks,
61+
base_ref_name=meta.base_ref_name,
62+
base_ref_oid=meta.base_ref_oid,
63+
merge_commit_allowed=meta.merge_commit_allowed,
64+
squash_merge_allowed=meta.squash_merge_allowed,
65+
rebase_merge_allowed=meta.rebase_merge_allowed,
66+
co_author_trailers=meta.co_author_trailers,
67+
conflict_files=meta.conflict_files,
68+
forward_after_by_page=({1: None} if timeline_loaded else {}),
69+
backward_before_by_page=({resolved_total_pages: None} if timeline_loaded else {}),
70+
)
71+
72+
1573
class TimelinePager:
1674
def __init__(self, client: GitHubClient) -> None:
1775
self._client = client
@@ -45,47 +103,11 @@ def build_initial(
45103
total_count = first_page.total_count
46104
total_pages = _page_count(total_count, page_size)
47105

48-
context = TimelineContext(
49-
owner=meta.ref.owner,
50-
name=meta.ref.name,
51-
number=meta.ref.number,
106+
context = build_context_from_meta(
107+
meta=meta,
52108
page_size=page_size,
53109
total_count=total_count,
54110
total_pages=total_pages,
55-
title=meta.title,
56-
url=meta.url,
57-
author=meta.author,
58-
state=meta.state,
59-
is_draft=meta.is_draft,
60-
body=meta.body,
61-
updated_at=meta.updated_at,
62-
labels=meta.labels,
63-
kind=meta.kind,
64-
pr_reactions_summary=meta.reactions_summary,
65-
can_edit_pr_body=meta.can_edit_body,
66-
is_merged=meta.is_merged,
67-
head_ref_name=meta.head_ref_name,
68-
head_ref_repo=meta.head_ref_repo,
69-
head_ref_oid=meta.head_ref_oid,
70-
head_ref_deleted=meta.head_ref_deleted,
71-
pr_node_id=meta.node_id,
72-
merge_state_status=meta.merge_state_status,
73-
mergeable=meta.mergeable,
74-
review_decision=meta.review_decision,
75-
requires_approving_reviews=meta.requires_approving_reviews,
76-
required_approving_review_count=meta.required_approving_review_count,
77-
requires_code_owner_reviews=meta.requires_code_owner_reviews,
78-
approved_review_count=meta.approved_review_count,
79-
requires_status_checks=meta.requires_status_checks,
80-
base_ref_name=meta.base_ref_name,
81-
base_ref_oid=meta.base_ref_oid,
82-
merge_commit_allowed=meta.merge_commit_allowed,
83-
squash_merge_allowed=meta.squash_merge_allowed,
84-
rebase_merge_allowed=meta.rebase_merge_allowed,
85-
co_author_trailers=meta.co_author_trailers,
86-
conflict_files=meta.conflict_files,
87-
forward_after_by_page={1: None},
88-
backward_before_by_page={total_pages: None},
89111
)
90112
self._remember_forward(context, page=1, cursor_used=None, page_result=first_page)
91113

src/gh_llm/render.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,15 @@ def render_frontmatter(context: TimelineContext) -> list[str]:
3535
f"labels: {json.dumps(list(context.labels), ensure_ascii=False)}",
3636
f"draft: {str(context.is_draft).lower()}",
3737
f"updated_at: {context.updated_at}",
38-
f"timeline_events: {context.total_count}",
39-
f"page_size: {context.page_size}",
40-
f"total_pages: {context.total_pages}",
4138
]
39+
if context.timeline_loaded:
40+
lines.extend(
41+
[
42+
f"timeline_events: {context.total_count}",
43+
f"page_size: {context.page_size}",
44+
f"total_pages: {context.total_pages}",
45+
]
46+
)
4247
if context.kind == "pr":
4348
lines.append(f"is_merged: {str(context.is_merged).lower()}")
4449
if context.head_ref_repo:

tests/test_cli.py

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,91 @@ def test_view_and_expand_use_real_cursor_pagination(
609609
assert "END_MARKER" in out
610610

611611

612+
def test_pr_view_show_meta_skips_timeline_bootstrap(
613+
monkeypatch: pytest.MonkeyPatch,
614+
capsys: pytest.CaptureFixture[str],
615+
) -> None:
616+
responder = GhResponder()
617+
monkeypatch.setattr(github_api.subprocess, "run", responder.run)
618+
619+
code = cli.run(["pr", "view", "77928", "--repo", "PaddlePaddle/Paddle", "--show", "meta"])
620+
assert code == 0
621+
622+
out = capsys.readouterr().out
623+
assert "pr: 77928" in out
624+
assert "timeline_events:" not in out
625+
assert "## Timeline" not in out
626+
assert "## Checks" not in out
627+
628+
graphql_queries = [_extract_form(call, "query") for call in responder.calls if call[:3] == ["gh", "api", "graphql"]]
629+
assert any("headRefName" in query and "timelineItems" not in query for query in graphql_queries)
630+
assert not any("timelineItems(" in query for query in graphql_queries)
631+
assert not any("reviewThreads(first:100" in query for query in graphql_queries)
632+
assert not any("statusCheckRollup" in query for query in graphql_queries)
633+
634+
635+
def test_pr_view_show_checks_fetches_checks_without_timeline_bootstrap(
636+
monkeypatch: pytest.MonkeyPatch,
637+
capsys: pytest.CaptureFixture[str],
638+
) -> None:
639+
responder = GhResponder()
640+
monkeypatch.setattr(github_api.subprocess, "run", responder.run)
641+
642+
code = cli.run(["pr", "view", "77928", "--repo", "PaddlePaddle/Paddle", "--show", "checks"])
643+
assert code == 0
644+
645+
out = capsys.readouterr().out
646+
assert "## Checks" in out
647+
assert "[IN_PROGRESS/NONE] unit-tests (check-run)" in out
648+
assert "## Timeline" not in out
649+
650+
graphql_queries = [_extract_form(call, "query") for call in responder.calls if call[:3] == ["gh", "api", "graphql"]]
651+
assert any("statusCheckRollup" in query for query in graphql_queries)
652+
assert not any("timelineItems(" in query for query in graphql_queries)
653+
assert not any("reviewThreads(first:100" in query for query in graphql_queries)
654+
655+
656+
def test_pr_checks_command_skips_timeline_bootstrap(
657+
monkeypatch: pytest.MonkeyPatch,
658+
capsys: pytest.CaptureFixture[str],
659+
) -> None:
660+
responder = GhResponder()
661+
monkeypatch.setattr(github_api.subprocess, "run", responder.run)
662+
663+
code = cli.run(["pr", "checks", "--pr", "77928", "--repo", "PaddlePaddle/Paddle", "--all"])
664+
assert code == 0
665+
666+
out = capsys.readouterr().out
667+
assert "## Checks" in out
668+
assert "[COMPLETED/SUCCESS] lint (check-run)" in out
669+
670+
graphql_queries = [_extract_form(call, "query") for call in responder.calls if call[:3] == ["gh", "api", "graphql"]]
671+
assert any("statusCheckRollup" in query for query in graphql_queries)
672+
assert not any("timelineItems(" in query for query in graphql_queries)
673+
assert not any("reviewThreads(first:100" in query for query in graphql_queries)
674+
675+
676+
def test_pr_view_show_mergeability_fetches_status_without_timeline_bootstrap(
677+
monkeypatch: pytest.MonkeyPatch,
678+
capsys: pytest.CaptureFixture[str],
679+
) -> None:
680+
responder = GhResponder()
681+
monkeypatch.setattr(github_api.subprocess, "run", responder.run)
682+
683+
code = cli.run(["pr", "view", "77971", "--repo", "PaddlePaddle/Paddle", "--show", "mergeability"])
684+
assert code == 0
685+
686+
out = capsys.readouterr().out
687+
assert "## Mergeability" in out
688+
assert "Status: Merging is blocked" in out
689+
assert "## Timeline" not in out
690+
691+
graphql_queries = [_extract_form(call, "query") for call in responder.calls if call[:3] == ["gh", "api", "graphql"]]
692+
assert any("statusCheckRollup" in query for query in graphql_queries)
693+
assert not any("timelineItems(" in query for query in graphql_queries)
694+
assert not any("reviewThreads(first:100" in query for query in graphql_queries)
695+
696+
612697
def test_web_like_extra_timeline_events_are_rendered(
613698
monkeypatch: pytest.MonkeyPatch,
614699
tmp_path: Path,

0 commit comments

Comments
 (0)