diff --git a/agent/reviewer.py b/agent/reviewer.py index 9771dfa8..b4d65561 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, truncate_diff +from .reviewer_diff import compute_diff_line_set, fetch_pr_diff, fetch_pr_metadata from .reviewer_findings import ( list_findings as list_findings_async, ) @@ -871,14 +871,15 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel: and bool(github_api_token) ) - async def _fetch_diff_context() -> tuple[str, str, dict[str, dict[str, set[int]]] | None]: - """Return (full_diff, truncated_diff, line_set). + async def _fetch_diff_context() -> tuple[str, dict[str, dict[str, set[int]]] | None]: + """Return (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. + The reviewer model has a large context window and reviews the full diff; + it is not truncated. The line set is computed from the same diff so + findings on any changed line are accepted. """ 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, @@ -886,8 +887,8 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel: token=github_api_token, ) if fetched_diff is None: - return "", "", None - return fetched_diff, truncate_diff(fetched_diff), compute_diff_line_set(fetched_diff) + return "", None + return 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): @@ -981,7 +982,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 ec6b68b8..d15fd1c5 100644 --- a/agent/reviewer_diff.py +++ b/agent/reviewer_diff.py @@ -197,10 +197,6 @@ 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, @@ -216,8 +212,8 @@ async def fetch_pr_diff( 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. + The full diff is returned uncapped: the reviewer model has a large context + window and reviews the complete diff. """ import httpx @@ -240,19 +236,6 @@ 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/ui/src/components/agents/ReviewChat.tsx b/ui/src/components/agents/ReviewChat.tsx index 55150fc2..2be23f2c 100644 --- a/ui/src/components/agents/ReviewChat.tsx +++ b/ui/src/components/agents/ReviewChat.tsx @@ -579,7 +579,7 @@ function ChatBody({ }} placeholder="Ask anything about this PR…" rows={1} - className="max-h-40 min-h-7 flex-1 resize-none rounded-none border-0 bg-transparent px-0 py-1 shadow-none focus-visible:border-transparent focus-visible:ring-0" + className="max-h-40 min-h-7 flex-1 resize-none rounded-none border-0 bg-transparent px-0 py-1 shadow-none focus-visible:border-transparent focus-visible:ring-0 dark:bg-transparent" /> { + if (frames++ > 240) return + const top = scroller.scrollTop + if (top === lastTop) stableFrames++ + else { + stableFrames = 0 + lastTop = top + } + // Wait for the smooth scroll + height reconciliation to settle. + if (stableFrames < 3) { + requestAnimationFrame(align) + return + } + const marginTop = parseFloat(getComputedStyle(el).scrollMarginTop) || 0 + const delta = + el.getBoundingClientRect().top - + scroller.getBoundingClientRect().top - + marginTop + if (Math.abs(delta) > 1 && corrections++ < 5) { + el.scrollIntoView({ block: "start", behavior: "smooth" }) + stableFrames = 0 + lastTop = Number.NaN + requestAnimationFrame(align) + } + } + requestAnimationFrame(align) +} + interface ResolvedGroup { index: number title: string @@ -325,6 +367,7 @@ function ReviewBodyInner({ const [anchorEl, setAnchorEl] = useState(null) const [userSelection, setUserSelection] = useState(null) const [diffScrollEl, setDiffScrollEl] = useState(null) + const diffScrollElRef = useRef(null) const groupRefs = useRef>({}) const [diffStyle, setDiffStyleState] = useState(() => readStoredDiffStyle() @@ -516,19 +559,15 @@ function ReviewBodyInner({ setSelectedFile(path) setExpandedFiles((prev) => ({ ...prev, [path]: true })) requestAnimationFrame(() => { - fileRefs.current[path]?.scrollIntoView({ - block: "start", - behavior: "smooth", - }) + const el = fileRefs.current[path] + if (el) scrollCardToTop(el, diffScrollElRef.current) }) }, []) const scrollToGroup = useCallback((index: number) => { requestAnimationFrame(() => { - groupRefs.current[index]?.scrollIntoView({ - block: "start", - behavior: "smooth", - }) + const el = groupRefs.current[index] + if (el) scrollCardToTop(el, diffScrollElRef.current) }) }, []) @@ -544,7 +583,9 @@ function ReviewBodyInner({ // anchored finding card can position against it. const scrollerProbe = useCallback((node: HTMLDivElement | null) => { const scroller = node?.parentElement?.parentElement - setDiffScrollEl(scroller instanceof HTMLDivElement ? scroller : null) + const el = scroller instanceof HTMLDivElement ? scroller : null + diffScrollElRef.current = el + setDiffScrollEl(el) }, []) const registerSection = useCallback( @@ -1264,15 +1305,20 @@ function FindingRailMarker({ const FINDING_CARD_WIDTH = 412 const FINDING_CARD_GAP = 12 +// If the room beside the annotation is tighter than this, hold this width and +// overlay the diff rather than shrinking into an unreadable sliver (e.g. a very +// narrow side panel, or no panel at all below `xl`). +const FINDING_CARD_MIN_WIDTH = 320 const FINDING_CARD_CLASS = - "flex max-h-[70vh] w-[412px] flex-col overflow-hidden rounded-lg border border-border bg-background shadow-2xl" + "flex max-h-[70vh] flex-col overflow-hidden rounded-lg border border-border bg-background shadow-2xl" -// Pinned to the right of the diff column (toward the side panel), vertically -// aligned with the finding's annotation. The diff scroller is only as wide as -//
, so the card is viewport-fixed (clamped to the window width) to sit in -// the right gutter rather than overlapping the diff, and tracks the anchor as -// the diff scrolls (rAF-throttled). Hidden while the anchor is out of view. +// Anchored just to the right of the finding's annotation, close to the hunk, and +// extending right over the side panel. Its width fits the room available to the +// right (capped at the preferred size), so it narrows as the side panel shrinks; +// when that room gets too tight to read it holds a minimum width and overlays +// the diff (e.g. below `xl`, with no side panel). Tracks the anchor as the diff +// scrolls (rAF-throttled); hidden while the anchor is out of view. function AnchoredFindingCard({ detail, finding, @@ -1307,13 +1353,19 @@ function AnchoredFindingCard({ card.style.visibility = "hidden" return } - const left = Math.max( - FINDING_CARD_GAP, - Math.min( - anchorRect.right + FINDING_CARD_GAP, - window.innerWidth - FINDING_CARD_WIDTH - FINDING_CARD_GAP - ) - ) + // Sit just right of the finding's annotation (close to the hunk). Width + // fits the room to its right so the card narrows as the side panel shrinks + // instead of overflowing. When that room is too tight to read, hold a + // minimum width and shift left over the diff. + const rightBound = window.innerWidth - FINDING_CARD_GAP + const minLeft = scrollerRect.left + FINDING_CARD_GAP + let left = anchorRect.right + FINDING_CARD_GAP + let width = Math.min(FINDING_CARD_WIDTH, rightBound - left) + if (width < FINDING_CARD_MIN_WIDTH) { + width = Math.min(FINDING_CARD_WIDTH, rightBound - minLeft) + left = Math.max(minLeft, rightBound - width) + } + card.style.width = `${width}px` const top = Math.max( scrollerRect.top + FINDING_CARD_GAP, Math.min(