Fix Observatory stack trace links

Author: quic-boyuc

devtools/observatory/lenses/stack_trace.py

@@ -35,10 +35,11 @@ def _get_stack_trace(cls) -> List[Dict[str, Any]]:
                 continue
 
             github_link = None
-            if git_info.github_link and repo_root:
+            link_root = git_info.commit_blob_url or git_info.branch_blob_url or git_info.github_link
+            if link_root and repo_root:
                 try:
                     rel_path = os.path.relpath(frame_info.filename, repo_root)
-                    github_link = f"{git_info.github_link}/{rel_path}#L{frame_info.lineno}"
+                    github_link = f"{link_root}/{rel_path}#L{frame_info.lineno}"
                 except Exception:
                     pass
 

devtools/observatory/tests/test_utils_and_stacktrace.py

@@ -0,0 +1,95 @@
+# Copyright (c) Qualcomm Innovation Center, Inc.
+# All rights reserved
+#
+# This source code is licensed under the BSD-style license found in the
+# LICENSE file in the root directory of this source tree.
+
+from __future__ import annotations
+
+from types import SimpleNamespace
+
+import pytest
+
+from executorch.devtools.observatory.lenses.stack_trace import StackTraceLens
+from executorch.devtools.observatory import utils
+
+
+@pytest.mark.parametrize(
+    "remote_url, expected",
+    [
+        ("git@github.com:pytorch/executorch.git", "https://github.com/pytorch/executorch"),
+        ("git@github.qualcomm.com:MLG/executorch", "https://github.qualcomm.com/MLG/executorch"),
+        (
+            "ssh://git@github.enterprise.local:8022/org/repo.git",
+            "https://github.enterprise.local:8022/org/repo",
+        ),
+        ("https://github.com/pytorch/executorch.git", "https://github.com/pytorch/executorch"),
+        ("file:///tmp/executorch", None),
+    ],
+)
+def test_normalize_remote_url(remote_url, expected):
+    assert utils._normalize_remote_url(remote_url) == expected  # type: ignore[attr-defined]
+
+
+def test_stack_trace_uses_commit_links(monkeypatch, tmp_path):
+    repo_root = tmp_path / "repo"
+    repo_root.mkdir()
+    source_file = repo_root / "pkg" / "module.py"
+    source_file.parent.mkdir(parents=True)
+    source_file.write_text("line = 1\n")
+
+    frame = SimpleNamespace(
+        filename=str(source_file),
+        function="test_fn",
+        lineno=1,
+        code_context=["line = 1\n"],
+    )
+
+    git_info = SimpleNamespace(
+        commit_blob_url="https://github.com/org/repo/blob/abc123",
+        branch_blob_url="https://github.com/org/repo/blob/main",
+        github_link="https://github.com/org/repo/tree/main",
+    )
+
+    from executorch.devtools.observatory.lenses import stack_trace as stack_trace_mod
+
+    monkeypatch.setattr(stack_trace_mod, "inspect", SimpleNamespace(stack=lambda: [frame]))
+    monkeypatch.setattr(stack_trace_mod, "get_repo_root", lambda: str(repo_root))
+    monkeypatch.setattr(stack_trace_mod, "get_git_info", lambda: git_info)
+    monkeypatch.setattr(stack_trace_mod, "is_in_repo", lambda path: str(path).startswith(str(repo_root)))
+
+    frames = StackTraceLens._get_stack_trace()
+    assert len(frames) == 1
+    assert frames[0]["link"] == "https://github.com/org/repo/blob/abc123/pkg/module.py#L1"
+
+
+def test_stack_trace_falls_back_to_branch_links(monkeypatch, tmp_path):
+    repo_root = tmp_path / "repo"
+    repo_root.mkdir()
+    source_file = repo_root / "pkg" / "fallback.py"
+    source_file.parent.mkdir(parents=True)
+    source_file.write_text("line = 2\n")
+
+    frame = SimpleNamespace(
+        filename=str(source_file),
+        function="fallback_fn",
+        lineno=2,
+        code_context=["line = 2\n"],
+    )
+
+    git_info = SimpleNamespace(
+        commit_blob_url=None,
+        branch_blob_url="https://github.com/org/repo/blob/main",
+        github_link="https://github.com/org/repo/tree/main",
+    )
+
+    from executorch.devtools.observatory.lenses import stack_trace as stack_trace_mod
+
+    monkeypatch.setattr(stack_trace_mod, "inspect", SimpleNamespace(stack=lambda: [frame]))
+    monkeypatch.setattr(stack_trace_mod, "get_repo_root", lambda: str(repo_root))
+    monkeypatch.setattr(stack_trace_mod, "get_git_info", lambda: git_info)
+    monkeypatch.setattr(stack_trace_mod, "is_in_repo", lambda path: str(path).startswith(str(repo_root)))
+
+    frames = StackTraceLens._get_stack_trace()
+    assert len(frames) == 1
+    assert frames[0]["link"] == "https://github.com/org/repo/blob/main/pkg/fallback.py#L2"

devtools/observatory/utils.py

@@ -11,23 +11,73 @@
 import subprocess
 from dataclasses import dataclass
 from typing import Optional
+from urllib.parse import urlparse
 
 
 @dataclass
 class GitInfo:
     """Git repository information for source links."""
 
     remote_url: Optional[str] = None
+    remote_https_url: Optional[str] = None
     branch: Optional[str] = None
     commit_hash: Optional[str] = None
     is_dirty: bool = False
     github_link: Optional[str] = None
+    branch_blob_url: Optional[str] = None
+    commit_blob_url: Optional[str] = None
 
 
 _cached_git_info: Optional[GitInfo] = None
 _cached_repo_root: Optional[str] = None
 
 
+def _strip_git_suffix(value: str) -> str:
+    return value[:-4] if value.endswith(".git") else value
+
+
+def _normalize_remote_url(remote_url: str) -> Optional[str]:
+    """Return an https URL for browser links regardless of git remote scheme."""
+
+    if not remote_url:
+        return None
+
+    remote_url = remote_url.strip()
+    if not remote_url:
+        return None
+
+    if remote_url.startswith("git@"):
+        try:
+            user_host, path = remote_url.split(":", 1)
+        except ValueError:
+            return None
+        host = user_host.split("@", 1)[1]
+        cleaned_path = _strip_git_suffix(path.lstrip("/").rstrip("/"))
+        return f"https://{host}/{cleaned_path}" if cleaned_path else f"https://{host}"
+
+    if remote_url.startswith("ssh://"):
+        parsed = urlparse(remote_url)
+        host = parsed.hostname
+        if not host:
+            return None
+        port_suffix = f":{parsed.port}" if parsed.port and parsed.port not in (22,) else ""
+        cleaned_path = _strip_git_suffix(parsed.path.lstrip("/").rstrip("/"))
+        return f"https://{host}{port_suffix}/{cleaned_path}" if cleaned_path else f"https://{host}{port_suffix}"
+
+    parsed = urlparse(remote_url)
+    if parsed.scheme in {"http", "https"}:
+        host = parsed.hostname
+        if not host:
+            return None
+        port_suffix = ""
+        if parsed.port and not (parsed.scheme == "http" and parsed.port == 80) and not (parsed.scheme == "https" and parsed.port == 443):
+            port_suffix = f":{parsed.port}"
+        cleaned_path = _strip_git_suffix(parsed.path.lstrip("/").rstrip("/"))
+        return f"https://{host}{port_suffix}/{cleaned_path}" if cleaned_path else f"https://{host}{port_suffix}"
+
+    return None
+
+
 def get_repo_root() -> Optional[str]:
     """Return repository root, or None when unavailable."""
 
@@ -94,6 +144,7 @@ def get_git_info() -> GitInfo:
             )
             if remote_url.returncode == 0:
                 info.remote_url = remote_url.stdout.strip()
+                info.remote_https_url = _normalize_remote_url(info.remote_url)
 
         commit = subprocess.run(
             ["git", "rev-parse", "HEAD"],
@@ -113,9 +164,11 @@ def get_git_info() -> GitInfo:
         if dirty.returncode == 0:
             info.is_dirty = bool(dirty.stdout.strip())
 
-        if info.remote_url and info.branch:
-            base = info.remote_url[:-4] if info.remote_url.endswith(".git") else info.remote_url
-            info.github_link = f"{base}/tree/{info.branch}"
+        if info.remote_https_url and info.branch:
+            info.github_link = f"{info.remote_https_url}/tree/{info.branch}"
+            info.branch_blob_url = f"{info.remote_https_url}/blob/{info.branch}"
+        if info.remote_https_url and info.commit_hash:
+            info.commit_blob_url = f"{info.remote_https_url}/blob/{info.commit_hash}"
     except Exception as exc:
         logging.debug("[Observatory] Failed to query git info: %s", exc)