Support artifact file and line URL links (#1038)

From PR #1038.

Author: pfbyjy

frontend/src/components/artifacts-viewer.tsx

@@ -30,6 +30,8 @@ import {
   isBinaryRendererFile,
 } from "@/components/renderers/file-renderer";
 import { fetcher } from "@/lib/api";
+import { sameFilePath } from "@/lib/file-path";
+import type { LineRange } from "@/lib/line-range";
 
 // Truncate previews of files larger than 100KB so we don't blow up the
 // renderer pane on huge artifacts (matches TaskFilesPanel).
@@ -205,9 +207,32 @@ function collectFiles(nodes: TreeNode[]): TreeNode[] {
 
 interface ArtifactsViewerProps {
   filesUrl: string;
+  /**
+   * Deep-linked file to select once the listing loads (``?file=`` while
+   * ``tab=artifacts``). Accepts the tree path shown in the browser, the
+   * original storage path, or a suffix of either (bare file name).
+   */
+  initialFilePath?: string | null;
+  /** Line range to highlight in the selected file (``?lines=``). */
+  selectedLines?: LineRange | null;
+  onSelectLinesChange?: (range: LineRange | null) => void;
+  /**
+   * Reports the selected file's tree path (and its original storage path,
+   * when known) whenever a file is selected, for URL sync. The storage path
+   * lets the parent recognize a deep link that addressed the file by
+   * storage path — the two forms differ for multi-step artifacts. Never
+   * called with null — transient resets are not reported.
+   */
+  onSelectedFileChange?: (path: string, fullPath?: string) => void;
 }
 
-export function ArtifactsViewer({ filesUrl }: ArtifactsViewerProps) {
+export function ArtifactsViewer({
+  filesUrl,
+  initialFilePath,
+  selectedLines,
+  onSelectLinesChange,
+  onSelectedFileChange,
+}: ArtifactsViewerProps) {
   const { data, isLoading, error } = useSWR<ArtifactsListing>(
     `${filesUrl}?recursive=1`,
     fetcher,
@@ -227,7 +252,15 @@ export function ArtifactsViewer({ filesUrl }: ArtifactsViewerProps) {
   const [expandedDirs, setExpandedDirs] = useState<Set<string>>(new Set());
   const [viewMode, setViewMode] = useState<"rendered" | "raw">("rendered");
 
-  // First load: expand every dir and select the first file. We also re-run
+  // A deep-linked path owns the first selection; read through a ref so the
+  // load effect doesn't re-run when the parent echoes selections back.
+  const initialFilePathRef = useRef(initialFilePath);
+  useEffect(() => {
+    initialFilePathRef.current = initialFilePath;
+  });
+
+  // First load: expand every dir and select the deep-linked file if one is
+  // addressed (exact path or suffix), else the first file. We also re-run
   // this if the file set changes (e.g. trial finishes producing artifacts
   // while the drawer is open) but only fall back to a fresh selection when
   // the previously selected path no longer exists.
@@ -240,11 +273,45 @@ export function ArtifactsViewer({ filesUrl }: ArtifactsViewerProps) {
     setExpandedDirs(new Set(collectDirPaths(tree)));
     setSelectedPath((prev) => {
       if (prev && allFiles.some((f) => f.path === prev)) return prev;
+      const wanted = initialFilePathRef.current;
+      if (wanted) {
+        // Match against the relativized tree path and the original storage
+        // path: multi-step artifacts insert the step segment into the tree
+        // path (steps/setup/artifacts/x → setup/x), so a storage path from
+        // the files API is not a suffix of it and only fullPath can match.
+        const match =
+          allFiles.find((f) => f.path === wanted) ??
+          allFiles.find((f) => f.fullPath === wanted) ??
+          allFiles.find((f) => sameFilePath(f.path, wanted)) ??
+          allFiles.find(
+            (f) => f.fullPath != null && sameFilePath(f.fullPath, wanted),
+          );
+        // An unresolved deep link keeps the selection empty instead of
+        // falling through to the first file: reporting that fallback
+        // would wipe the ?file= / ?lines= address it couldn't resolve.
+        // The effect re-runs as the listing grows, so a late-arriving
+        // artifact still resolves.
+        return match?.path ?? null;
+      }
       const first = findFirstFile(tree);
       return first?.path ?? null;
     });
   }, [tree, allFiles]);
 
+  // Report file selections upward for URL sync. Nulls (transient resets)
+  // are never reported — they would wipe a live ?file= anchor.
+  const onSelectedFileChangeRef = useRef(onSelectedFileChange);
+  useEffect(() => {
+    onSelectedFileChangeRef.current = onSelectedFileChange;
+  });
+  useEffect(() => {
+    if (selectedPath === null) return;
+    const file = allFiles.find((f) => f.path === selectedPath);
+    onSelectedFileChangeRef.current?.(selectedPath, file?.fullPath);
+    // allFiles is a dependency only to read the fullPath; a listing refresh
+    // re-reports the same selection, which the parent treats as a no-op.
+  }, [selectedPath, allFiles]);
+
   const selectedFile = useMemo(
     () => allFiles.find((f) => f.path === selectedPath) ?? null,
     [allFiles, selectedPath],
@@ -365,6 +432,8 @@ export function ArtifactsViewer({ filesUrl }: ArtifactsViewerProps) {
         selectedFile={selectedFile}
         viewMode={viewMode}
         onViewModeChange={setViewMode}
+        selectedLines={selectedLines}
+        onSelectLinesChange={onSelectLinesChange}
       />
     </div>
   );
@@ -375,13 +444,17 @@ interface ArtifactContentPaneProps {
   selectedFile: TreeNode | null;
   viewMode: "rendered" | "raw";
   onViewModeChange: (mode: "rendered" | "raw") => void;
+  selectedLines?: LineRange | null;
+  onSelectLinesChange?: (range: LineRange | null) => void;
 }
 
 function ArtifactContentPane({
   filesUrl,
   selectedFile,
   viewMode,
   onViewModeChange,
+  selectedLines,
+  onSelectLinesChange,
 }: ArtifactContentPaneProps) {
   const contentRef = useRef<HTMLDivElement>(null);
   const [content, setContent] = useState<string | null>(null);
@@ -573,6 +646,8 @@ function ArtifactContentPane({
             content={isBinary ? null : content}
             fileSize={fileSize}
             viewMode={viewMode}
+            selectedLines={selectedLines}
+            onSelectLines={onSelectLinesChange}
           />
         )}
       </div>

frontend/src/components/trial-detail-panel.tsx

@@ -856,12 +856,25 @@ export function TrialDetailPanel({
   const [deleteDialogOpen, setDeleteDialogOpen] = useState(false);
   const [deleting, setDeleting] = useState(false);
   const [deleteError, setDeleteError] = useState<string | null>(null);
+  // ``?file=`` / ``?lines=`` are scoped by ``?tab=``: on the artifacts tab
+  // they address the artifact browser, otherwise the files tab. Each tab
+  // keeps its own state; the URL carries the active tab's pair.
   const [filesTargetPath, setFilesTargetPath] = useState<string | null>(() =>
-    getLiveParam("file"),
+    getLiveParam("tab") === "artifacts" ? null : getLiveParam("file"),
   );
   // Line-anchor range within the selected file (``?lines=L12-L20``).
   const [selectedLines, setSelectedLines] = useState<LineRange | null>(() =>
-    parseLineRange(getLiveParam("lines")),
+    getLiveParam("tab") === "artifacts"
+      ? null
+      : parseLineRange(getLiveParam("lines")),
+  );
+  const [artifactsTargetPath, setArtifactsTargetPath] = useState<
+    string | null
+  >(() => (getLiveParam("tab") === "artifacts" ? getLiveParam("file") : null));
+  const [artifactsLines, setArtifactsLines] = useState<LineRange | null>(() =>
+    getLiveParam("tab") === "artifacts"
+      ? parseLineRange(getLiveParam("lines"))
+      : null,
   );
 
   const hydratedFromUrl = useRef(false);
@@ -878,6 +891,15 @@ export function TrialDetailPanel({
     const urlFile = getLiveParam("file");
     const urlLines = parseLineRange(getLiveParam("lines"));
     if (urlTab && validTabs.has(urlTab)) setActiveTab(urlTab);
+    if (urlTab === "artifacts") {
+      // ?file=/?lines= address the artifact browser while tab=artifacts.
+      if (urlFile) {
+        setArtifactsTargetPath(urlFile);
+        artifactsTargetPathRef.current = urlFile;
+      }
+      if (urlLines) setArtifactsLines(urlLines);
+      return;
+    }
     if (urlFile) {
       setFilesTargetPath(urlFile);
       filesTargetPathRef.current = urlFile;
@@ -898,15 +920,35 @@ export function TrialDetailPanel({
     setFilesTargetPath(path);
   }, []);
 
-  // Navigating to a different trial keeps the file path (attempts share
+  // Same shape for the artifacts tab: its browser reports selections the
+  // same way, and a different file drops the artifact line anchor. The
+  // comparison also accepts the file's storage path — a deep link can
+  // address a multi-step artifact by storage path, and the browser echoes
+  // back the relativized tree path, which is not a suffix match of it.
+  const artifactsTargetPathRef = useRef<string | null>(artifactsTargetPath);
+  const handleArtifactsFileChange = useCallback(
+    (path: string | null, fullPath?: string) => {
+      const prev = artifactsTargetPathRef.current;
+      const same =
+        sameFilePath(prev, path) ||
+        (fullPath !== undefined && sameFilePath(prev, fullPath));
+      if (!same) setArtifactsLines(null);
+      artifactsTargetPathRef.current = path;
+      setArtifactsTargetPath(path);
+    },
+    []
+  );
+
+  // Navigating to a different trial keeps the file paths (attempts share
   // layouts, and comparing the same file across attempts is the point) but
-  // drops the line anchor — it addressed the previous trial's content and
-  // would highlight arbitrary lines here.
+  // drops the line anchors — they addressed the previous trial's content
+  // and would highlight arbitrary lines here.
   const lastTrialIdRef = useRef<string | null>(trial?.id ?? null);
   useEffect(() => {
     const id = trial?.id ?? null;
     if (id && lastTrialIdRef.current && id !== lastTrialIdRef.current) {
       setSelectedLines(null);
+      setArtifactsLines(null);
     }
     lastTrialIdRef.current = id;
   }, [trial?.id]);
@@ -929,14 +971,31 @@ export function TrialDetailPanel({
       next.delete("tab");
     }
 
-    if (filesTargetPath) {
-      next.set("file", filesTargetPath);
+    // ?file=/?lines= describe the active tab's view: the files tab's pair
+    // or the artifacts tab's pair. On tabs with no file view (summary,
+    // live, trajectory) they drop out of the URL — the tab states stay in
+    // React, so flipping back restores and re-writes them.
+    const paneFile =
+      activeTab === "artifacts"
+        ? artifactsTargetPath
+        : activeTab === "files"
+          ? filesTargetPath
+          : null;
+    const paneLines =
+      activeTab === "artifacts"
+        ? artifactsLines
+        : activeTab === "files"
+          ? selectedLines
+          : null;
+
+    if (paneFile) {
+      next.set("file", paneFile);
     } else {
       next.delete("file");
     }
 
-    if (selectedLines) {
-      next.set("lines", formatLineRange(selectedLines));
+    if (paneLines) {
+      next.set("lines", formatLineRange(paneLines));
     } else {
       next.delete("lines");
     }
@@ -945,7 +1004,14 @@ export function TrialDetailPanel({
       const url = `${window.location.pathname}${next.toString() ? `?${next.toString()}` : ""}`;
       window.history.replaceState(window.history.state, "", url);
     }
-  }, [isOpen, activeTab, filesTargetPath, selectedLines]);
+  }, [
+    isOpen,
+    activeTab,
+    filesTargetPath,
+    selectedLines,
+    artifactsTargetPath,
+    artifactsLines,
+  ]);
 
   const canRetry =
     allowRetry && (trial?.status === "failed" || trial?.status === "success");
@@ -1692,6 +1758,10 @@ export function TrialDetailPanel({
           <TabsContent value="artifacts" className="m-0 h-full p-0">
             <ArtifactsViewer
               filesUrl={`${apiBaseUrl}/trials/${trial.id}/files`}
+              initialFilePath={artifactsTargetPath}
+              selectedLines={artifactsLines}
+              onSelectLinesChange={setArtifactsLines}
+              onSelectedFileChange={handleArtifactsFileChange}
             />
           </TabsContent>