From d8d3794649fa242a1119421c28f1097c9e823377 Mon Sep 17 00:00:00 2001 From: "open-swe[bot]" <215916821+open-swe[bot]@users.noreply.github.com> Date: Thu, 28 May 2026 13:48:14 -0700 Subject: [PATCH] fix: include reviewer trace links (#1351) * fix: include reviewer trace links Co-authored-by: Johannes du Plessis <51395795+johannes117@users.noreply.github.com> * feat: move reviewer trace-link toggle to dashboard Replace the OPEN_SWE_REVIEW_TRACE_LINK_ENABLED env var with a team-level 'Trace Links' toggle in the Open SWE Review dashboard tab. The toggle is read per-publish via get_team_review_trace_links_enabled(); the per-run review_trace_link_enabled config override still forces it off. --------- Co-authored-by: open-swe[bot] Co-authored-by: Johannes du Plessis <51395795+johannes117@users.noreply.github.com> Co-authored-by: Johannes du Plessis --- agent/dashboard/team_settings.py | 9 ++++ agent/reviewer_publish.py | 37 +++++++------ agent/tools/publish_review.py | 33 +++++++++--- tests/test_reviewer_publish.py | 91 ++++++++++++++++++++++++++++++++ ui/src/lib/api.ts | 1 + ui/src/routes/review.tsx | 12 +++++ 6 files changed, 162 insertions(+), 21 deletions(-) diff --git a/agent/dashboard/team_settings.py b/agent/dashboard/team_settings.py index d6d05a8d..3dc81c3b 100644 --- a/agent/dashboard/team_settings.py +++ b/agent/dashboard/team_settings.py @@ -34,6 +34,7 @@ class TeamSettingsUpdate(BaseModel): trigger_mode: TriggerMode = "every_push" review_draft_prs: bool = False pr_summaries: bool = True + review_trace_links: bool = True autofix_mode: AutofixMode = "off" autofix_severity_threshold: AutofixMode = "medium" default_agent_model: str | None = None @@ -87,6 +88,7 @@ def _default_settings() -> dict[str, Any]: "trigger_mode": "every_push", "review_draft_prs": False, "pr_summaries": True, + "review_trace_links": True, "autofix_mode": "off", "autofix_severity_threshold": "medium", "default_agent_model": fallback_model, @@ -129,6 +131,7 @@ async def upsert_team_settings(update: TeamSettingsUpdate) -> dict[str, Any]: "trigger_mode": update.trigger_mode, "review_draft_prs": update.review_draft_prs, "pr_summaries": update.pr_summaries, + "review_trace_links": update.review_trace_links, "autofix_mode": update.autofix_mode, "autofix_severity_threshold": update.autofix_severity_threshold, "default_agent_model": update.default_agent_model, @@ -166,6 +169,12 @@ async def get_team_default_model( return _resolve_default_pair(model, effort) +async def get_team_review_trace_links_enabled() -> bool: + """Return whether GitHub review bodies should include a LangSmith trace link.""" + settings = await get_team_settings() + return bool(settings.get("review_trace_links", True)) + + async def get_team_default_subagent_model( role: Literal["agent", "reviewer"], ) -> tuple[str, str]: diff --git a/agent/reviewer_publish.py b/agent/reviewer_publish.py index 4b508f6d..d16427cc 100644 --- a/agent/reviewer_publish.py +++ b/agent/reviewer_publish.py @@ -213,14 +213,8 @@ def render_inline_comment_payload(finding: Finding) -> dict[str, Any] | None: return payload -def render_review_body(*, pr_number: int, surfaced_count: int) -> str: - """Compose the top-level review body. - - Two fixed shapes — no agent prose: - - - 0 surfaced findings: a single "no issues" line. - - N surfaced findings: a single "found N potential issue(s)" line. - """ +def render_review_body(*, pr_number: int, surfaced_count: int, trace_url: str | None = None) -> str: + """Compose the top-level review body.""" if surfaced_count == 0: headline = ( "## ✅ Open SWE Review: No issues found\n\n" @@ -229,7 +223,12 @@ def render_review_body(*, pr_number: int, surfaced_count: int) -> str: else: issue_word = "issue" if surfaced_count == 1 else "issues" headline = f"**Open SWE Review** found {surfaced_count} potential {issue_word}." - return f"{headline}\n\n" + + parts = [headline] + if trace_url: + parts.append(f"[View Open SWE trace]({trace_url})") + parts.append(f"") + return "\n\n".join(parts) async def post_pull_request_review( @@ -553,12 +552,20 @@ async def fetch_review_thread_id_for_comment( ) return None data = response.json() - threads = ( - data.get("data", {}) - .get("repository", {}) - .get("pullRequest", {}) - .get("reviewThreads", {}) - ) + if not isinstance(data, dict): + return None + data_root = data.get("data") + if not isinstance(data_root, dict): + return None + repository = data_root.get("repository") + if not isinstance(repository, dict): + return None + pull_request = repository.get("pullRequest") + if not isinstance(pull_request, dict): + return None + threads = pull_request.get("reviewThreads") + if not isinstance(threads, dict): + return None for thread in threads.get("nodes", []) or []: comment_ids = { c.get("databaseId") for c in (thread.get("comments", {}).get("nodes") or []) diff --git a/agent/tools/publish_review.py b/agent/tools/publish_review.py index eb1381ca..6dce08aa 100644 --- a/agent/tools/publish_review.py +++ b/agent/tools/publish_review.py @@ -7,6 +7,7 @@ from typing import Any from langgraph.config import get_config +from ..dashboard.team_settings import get_team_review_trace_links_enabled from ..reviewer_diff import compute_diff_line_set, fetch_pr_diff, is_range_in_diff from ..reviewer_findings import ( Finding, @@ -40,6 +41,7 @@ from ..utils.github_token import ( get_github_token, invalidate_cached_github_token, ) +from ..utils.langsmith import get_langsmith_trace_url from ..utils.slack import post_slack_thread_reply @@ -73,11 +75,12 @@ def publish_review( return {"success": False, "error": f"Invalid severity_threshold: {severity_threshold}"} config = get_config() - configurable = config.get("configurable", {}) if isinstance(config, dict) else {} - repo_config = configurable.get("repo") if isinstance(configurable, dict) else None - pr_number = configurable.get("pr_number") if isinstance(configurable, dict) else None - head_sha = configurable.get("head_sha") if isinstance(configurable, dict) else None - is_re_review = bool(configurable.get("re_review")) if isinstance(configurable, dict) else False + raw_configurable = config.get("configurable", {}) if isinstance(config, dict) else {} + configurable = raw_configurable if isinstance(raw_configurable, dict) else {} + repo_config = configurable.get("repo") + pr_number = configurable.get("pr_number") + head_sha = configurable.get("head_sha") + is_re_review = bool(configurable.get("re_review")) if ( not isinstance(repo_config, dict) @@ -115,6 +118,7 @@ def publish_review( cap=cap, is_re_review=is_re_review, langgraph_run_id=_current_run_id(config), + trace_link_config_override=configurable.get("review_trace_link_enabled"), ) ) except GitHubAuthError as exc: @@ -135,6 +139,16 @@ def _cast_severity(value: str) -> Severity: return value # type: ignore[return-value] +async def _resolve_review_trace_url(thread_id: str, config_override: object) -> str | None: + if config_override is False: + return None + if not await get_team_review_trace_links_enabled(): + return None + if not thread_id: + return None + return get_langsmith_trace_url(thread_id) + + def _is_reviewer_eval_mode(configurable: dict[str, Any]) -> bool: return configurable.get("reviewer_eval") is True or configurable.get("eval") is True @@ -184,8 +198,10 @@ async def _publish_review_async( cap: int, is_re_review: bool, langgraph_run_id: str | None = None, + trace_link_config_override: object = None, ) -> dict[str, Any]: thread_id = get_thread_id_from_runtime() + review_trace_url = await _resolve_review_trace_url(thread_id, trace_link_config_override) findings = await _backfill_findings_from_pr_threads( thread_id=thread_id, owner=owner, @@ -245,6 +261,7 @@ async def _publish_review_async( review_body = render_review_body( pr_number=pr_number, surfaced_count=len(inline_comments), + trace_url=review_trace_url, ) review_response = await post_pull_request_review( @@ -274,7 +291,11 @@ async def _publish_review_async( ) if dropped_ids and valid_with_payload: retry_inline = [p for _, p in valid_with_payload] - retry_body = render_review_body(pr_number=pr_number, surfaced_count=len(retry_inline)) + retry_body = render_review_body( + pr_number=pr_number, + surfaced_count=len(retry_inline), + trace_url=review_trace_url, + ) retry_response = await post_pull_request_review( owner=owner, repo=repo, diff --git a/tests/test_reviewer_publish.py b/tests/test_reviewer_publish.py index 6eeb3007..db3e0c72 100644 --- a/tests/test_reviewer_publish.py +++ b/tests/test_reviewer_publish.py @@ -184,6 +184,16 @@ def test_render_review_body_no_findings_message() -> None: assert "" in body +def test_render_review_body_includes_trace_link_when_provided() -> None: + body = render_review_body( + pr_number=123, + surfaced_count=0, + trace_url="https://smith.langchain.com/o/t/project/p/t/thread-id", + ) + assert "[View Open SWE trace](https://smith.langchain.com/o/t/project/p/t/thread-id)" in body + assert body.endswith("") + + def test_publish_review_eval_mode_does_not_call_github() -> None: from agent.tools.publish_review import publish_review @@ -223,6 +233,87 @@ def test_publish_review_eval_mode_does_not_call_github() -> None: set_meta.assert_awaited_once_with("tid", last_reviewed_sha="sha") +def test_publish_review_forwards_trace_link_config_override() -> None: + from agent.tools.publish_review import publish_review + + publish_async = AsyncMock(return_value={"success": True}) + with ( + patch( + "agent.tools.publish_review.get_config", + return_value={ + "configurable": { + "thread_id": "reviewer-thread-id", + "repo": {"owner": "o", "name": "r"}, + "pr_number": 7, + "head_sha": "sha", + "review_trace_link_enabled": False, + }, + "metadata": {}, + }, + ), + patch("agent.tools.publish_review.get_github_token", return_value="token"), + patch("agent.tools.publish_review._publish_review_async", publish_async), + ): + result = publish_review() + + assert result == {"success": True} + assert publish_async.call_args.kwargs["trace_link_config_override"] is False + + +@pytest.mark.asyncio +async def test_resolve_review_trace_url_enabled_by_team_setting() -> None: + from agent.tools.publish_review import _resolve_review_trace_url + + with ( + patch( + "agent.tools.publish_review.get_team_review_trace_links_enabled", + AsyncMock(return_value=True), + ), + patch( + "agent.tools.publish_review.get_langsmith_trace_url", + return_value="https://smith/t", + ), + ): + url = await _resolve_review_trace_url("reviewer-thread-id", None) + + assert url == "https://smith/t" + + +@pytest.mark.asyncio +async def test_resolve_review_trace_url_disabled_by_team_setting() -> None: + from agent.tools.publish_review import _resolve_review_trace_url + + trace_url = MagicMock(return_value="https://smith/t") + with ( + patch( + "agent.tools.publish_review.get_team_review_trace_links_enabled", + AsyncMock(return_value=False), + ), + patch("agent.tools.publish_review.get_langsmith_trace_url", trace_url), + ): + url = await _resolve_review_trace_url("reviewer-thread-id", None) + + assert url is None + trace_url.assert_not_called() + + +@pytest.mark.asyncio +async def test_resolve_review_trace_url_config_override_skips_team_lookup() -> None: + from agent.tools.publish_review import _resolve_review_trace_url + + team_lookup = AsyncMock(return_value=True) + trace_url = MagicMock(return_value="https://smith/t") + with ( + patch("agent.tools.publish_review.get_team_review_trace_links_enabled", team_lookup), + patch("agent.tools.publish_review.get_langsmith_trace_url", trace_url), + ): + url = await _resolve_review_trace_url("reviewer-thread-id", False) + + assert url is None + team_lookup.assert_not_called() + trace_url.assert_not_called() + + @pytest.mark.asyncio async def test_resolve_review_thread_returns_true_on_success() -> None: response = MagicMock() diff --git a/ui/src/lib/api.ts b/ui/src/lib/api.ts index f0ab913d..450c4b3b 100644 --- a/ui/src/lib/api.ts +++ b/ui/src/lib/api.ts @@ -97,6 +97,7 @@ export interface TeamSettings { trigger_mode: TriggerMode; review_draft_prs: boolean; pr_summaries: boolean; + review_trace_links: boolean; autofix_mode: AutofixMode; autofix_severity_threshold: AutofixMode; default_agent_model?: string | null; diff --git a/ui/src/routes/review.tsx b/ui/src/routes/review.tsx index 46744294..3f8b4647 100644 --- a/ui/src/routes/review.tsx +++ b/ui/src/routes/review.tsx @@ -48,6 +48,7 @@ const DEFAULT_SETTINGS: TeamSettings = { trigger_mode: "every_push", review_draft_prs: false, pr_summaries: true, + review_trace_links: true, autofix_mode: "off", autofix_severity_threshold: "medium", default_agent_model: null, @@ -176,6 +177,17 @@ function ReviewPage() { /> } /> + persist({ review_trace_links: v })} + disabled={!canEdit} + /> + } + />