diff --git a/agent/reviewer.py b/agent/reviewer.py index ccf1841e..1bc232f8 100644 --- a/agent/reviewer.py +++ b/agent/reviewer.py @@ -47,7 +47,7 @@ from .middleware import ( refresh_github_proxy_before_model, settle_review_check_on_exit, ) -from .reviewer_diff import compute_diff_line_set, fetch_pr_diff, fetch_pr_metadata +from .reviewer_diff import compute_diff_line_set, fetch_pr_diff, fetch_pr_metadata, truncate_diff from .reviewer_findings import ( list_findings as list_findings_async, ) @@ -847,9 +847,14 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel: and bool(github_api_token) ) - async def _fetch_diff_context() -> tuple[str, dict[str, dict[str, set[int]]] | None]: + async def _fetch_diff_context() -> tuple[str, str, dict[str, dict[str, set[int]]] | None]: + """Return (full_diff, truncated_diff, line_set). + + The line set is computed from the full diff so findings on any changed + line are accepted. Only the prompt-facing text is truncated. + """ if not can_fetch_pr or github_api_token is None or not isinstance(pr_number, int): - return "", None + return "", "", None fetched_diff = await fetch_pr_diff( owner=repo_owner, repo=repo_name, @@ -857,8 +862,8 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel: token=github_api_token, ) if fetched_diff is None: - return "", None - return fetched_diff, compute_diff_line_set(fetched_diff) + return "", "", None + return fetched_diff, truncate_diff(fetched_diff), compute_diff_line_set(fetched_diff) async def _fetch_pr_overview() -> tuple[str, str]: if not can_fetch_pr or github_api_token is None or not isinstance(pr_number, int): @@ -952,7 +957,7 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel: _fetch_org_guidelines(), fetch_api_standards_skill(), ) - pr_diff_text, pr_diff_line_set = diff_context + _, pr_diff_text, pr_diff_line_set = diff_context pr_title, pr_body = pr_overview config["configurable"]["diff_text"] = pr_diff_text config["configurable"]["diff_line_set"] = pr_diff_line_set diff --git a/agent/reviewer_diff.py b/agent/reviewer_diff.py index a8e1f0f9..ec6b68b8 100644 --- a/agent/reviewer_diff.py +++ b/agent/reviewer_diff.py @@ -197,6 +197,10 @@ def is_range_in_diff( return all(line in side_lines for line in range(start_line, end_line + 1)) +PR_DIFF_MAX_CHARS = 200_000 +PR_DIFF_TRUNCATION_MARKER = "\n... [PR diff truncated: {kept}/{total} chars]\n" + + async def fetch_pr_diff( *, owner: str, @@ -211,6 +215,9 @@ async def fetch_pr_diff( validates against when posting inline review comments, so it's the right source for ``add_finding``'s in-diff anchor validation and for ``publish_review``'s 422 retry filter. + + The full diff is returned — callers must use ``truncate_diff`` if they + need to cap the text before feeding it to the LLM. """ import httpx @@ -233,6 +240,19 @@ async def fetch_pr_diff( return response.text +def truncate_diff(diff_text: str) -> str: + """Truncate diff text to ``PR_DIFF_MAX_CHARS`` with a visible marker. + + Keeps the first half from the beginning and the second half from the end + so file headers and recent changes are both visible. + """ + if len(diff_text) <= PR_DIFF_MAX_CHARS: + return diff_text + half = PR_DIFF_MAX_CHARS // 2 + marker = PR_DIFF_TRUNCATION_MARKER.format(kept=PR_DIFF_MAX_CHARS, total=len(diff_text)) + return diff_text[:half] + marker + diff_text[-half:] + + async def fetch_pr_metadata( *, owner: str, diff --git a/agent/tools/fetch_url.py b/agent/tools/fetch_url.py index 978a74da..aebc9afc 100644 --- a/agent/tools/fetch_url.py +++ b/agent/tools/fetch_url.py @@ -5,6 +5,8 @@ from markdownify import markdownify from .http_request import _request_with_safe_redirects +FETCH_URL_MAX_CHARS = 100_000 + def fetch_url(url: str, timeout: int = 30) -> dict[str, Any]: """Fetch content from a URL and convert HTML to markdown format. @@ -50,6 +52,12 @@ def fetch_url(url: str, timeout: int = 30) -> dict[str, Any]: # Convert HTML content to markdown markdown_content = markdownify(response.text) + if len(markdown_content) > FETCH_URL_MAX_CHARS: + markdown_content = ( + markdown_content[:FETCH_URL_MAX_CHARS] + "\n... [content truncated: " + f"{FETCH_URL_MAX_CHARS}/{len(markdown_content)} chars]\n" + ) + return { "url": str(response.url), "markdown_content": markdown_content, diff --git a/agent/tools/slack_read_thread_messages.py b/agent/tools/slack_read_thread_messages.py index 25a16979..4b32eb86 100644 --- a/agent/tools/slack_read_thread_messages.py +++ b/agent/tools/slack_read_thread_messages.py @@ -2,6 +2,7 @@ import asyncio from typing import Any from ..utils.slack import ( + SLACK_THREAD_MAX_MESSAGES, fetch_slack_thread_messages, format_slack_messages_for_prompt, get_slack_user_names, @@ -19,8 +20,18 @@ async def _fetch_and_format(channel_id: str, message_ts: str) -> dict[str, Any]: ] user_names = await get_slack_user_names(user_ids) if user_ids else {} + truncated = len(messages) >= SLACK_THREAD_MAX_MESSAGES formatted = format_slack_messages_for_prompt(messages, user_names) - return {"success": True, "formatted": formatted, "count": len(messages)} + if truncated: + formatted = ( + f"[thread truncated — showing most recent {len(messages)} messages]\n{formatted}" + ) + return { + "success": True, + "formatted": formatted, + "count": len(messages), + "truncated": truncated, + } def slack_read_thread_messages(channel_id: str, message_ts: str) -> dict[str, Any]: diff --git a/agent/utils/github_comments.py b/agent/utils/github_comments.py index 7b771810..af85b996 100644 --- a/agent/utils/github_comments.py +++ b/agent/utils/github_comments.py @@ -44,6 +44,8 @@ _REACTION_ENDPOINTS: dict[str, str] = { "pull_request_review": "https://api.github.com/repos/{owner}/{repo}/pulls/{pull_number}/reviews/{comment_id}/reactions", } +PAGINATED_MAX_PAGES = 50 + def verify_github_signature(body: bytes, signature: str, *, secret: str) -> bool: """Verify the GitHub webhook signature (X-Hub-Signature-256). @@ -464,6 +466,9 @@ async def _fetch_paginated( ) -> list[dict[str, Any]]: """Fetch all pages from a GitHub paginated endpoint. + Caps at ``PAGINATED_MAX_PAGES`` pages to avoid unbounded fetching on + pathological PRs with thousands of comments. + Args: client: An active httpx async client. url: The GitHub API endpoint URL. @@ -475,7 +480,7 @@ async def _fetch_paginated( results: list[dict[str, Any]] = [] params: dict[str, Any] = {"per_page": 100, "page": 1} - while True: + while params["page"] <= PAGINATED_MAX_PAGES: try: response = await client.get(url, headers=headers, params=params) if response.status_code == 401: diff --git a/agent/utils/slack.py b/agent/utils/slack.py index e260b42b..0f013812 100644 --- a/agent/utils/slack.py +++ b/agent/utils/slack.py @@ -24,6 +24,7 @@ logger = logging.getLogger(__name__) SLACK_API_BASE_URL = "https://slack.com/api" SLACK_BOT_TOKEN = os.environ.get("SLACK_BOT_TOKEN", "") +SLACK_THREAD_MAX_MESSAGES = 500 DEFAULT_ASSISTANT_STATUS = "is thinking…" # Curated rotating loading strings shown by Slack while the indicator is active. @@ -504,12 +505,13 @@ async def get_slack_user_names(user_ids: list[str]) -> dict[str, str]: async def fetch_slack_thread_messages(channel_id: str, thread_ts: str) -> list[dict[str, Any]]: - """Fetch all messages for a Slack thread.""" + """Fetch messages for a Slack thread, keeping the most recent window.""" if not SLACK_BOT_TOKEN: return [] messages: list[dict[str, Any]] = [] cursor: str | None = None + truncated = False async with httpx.AsyncClient() as http_client: while True: @@ -537,6 +539,16 @@ async def fetch_slack_thread_messages(channel_id: str, thread_ts: str) -> list[d if isinstance(batch, list): messages.extend(item for item in batch if isinstance(item, dict)) + if len(messages) >= SLACK_THREAD_MAX_MESSAGES: + truncated = True + logger.warning( + "Slack thread %s/%s capped at %d messages", + channel_id, + thread_ts, + SLACK_THREAD_MAX_MESSAGES, + ) + break + response_metadata = payload.get("response_metadata", {}) cursor = ( response_metadata.get("next_cursor") if isinstance(response_metadata, dict) else "" @@ -544,6 +556,8 @@ async def fetch_slack_thread_messages(channel_id: str, thread_ts: str) -> list[d if not cursor: break + if truncated: + messages = messages[-SLACK_THREAD_MAX_MESSAGES:] messages.sort(key=lambda item: _parse_ts(item.get("ts"))) return messages diff --git a/agent/utils/thread_ops.py b/agent/utils/thread_ops.py index 447d7915..fb38d837 100644 --- a/agent/utils/thread_ops.py +++ b/agent/utils/thread_ops.py @@ -10,6 +10,8 @@ from langgraph_sdk import get_client logger = logging.getLogger(__name__) +MAX_QUEUED_MESSAGES = 100 + def langgraph_url() -> str: return os.environ.get("LANGGRAPH_URL") or os.environ.get( @@ -57,6 +59,13 @@ async def queue_message_for_thread( logger.debug("No existing queued messages for thread %s", thread_id) existing_messages.append(new_message) + if len(existing_messages) > MAX_QUEUED_MESSAGES: + existing_messages = existing_messages[-MAX_QUEUED_MESSAGES:] + logger.warning( + "Thread %s queue capped at %d messages (dropped oldest)", + thread_id, + MAX_QUEUED_MESSAGES, + ) await client.store.put_item(namespace, key, {"messages": existing_messages}) logger.info( "Queued message for thread %s (total queued: %d)",