From 0a6234f93cab555ece2f3002619d4571b98aee9b Mon Sep 17 00:00:00 2001
From: Dotta <bippadotta@protonmail.com>
Date: Fri, 24 Apr 2026 17:16:05 -0500
Subject: [PATCH] Address file viewer PR review feedback

---
 server/src/routes/file-resources.ts    |  3 ++
 ui/src/components/ArtifactFileChip.tsx | 45 ++++++++++++++++++--------
 ui/src/components/FileViewerSheet.tsx  |  5 ++-
 ui/src/pages/IssueDetail.test.tsx      |  1 +
 4 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/server/src/routes/file-resources.ts b/server/src/routes/file-resources.ts
index f691de5fe38..847d351ed99 100644
--- a/server/src/routes/file-resources.ts
+++ b/server/src/routes/file-resources.ts
@@ -35,6 +35,9 @@ export function createFileResourceLimiter(opts: {
   return {
     acquire(key: string) {
       const now = Date.now();
+      for (const [windowKey, existing] of windowsByKey) {
+        if (now - existing.startedAt >= windowMs) windowsByKey.delete(windowKey);
+      }
       const window = windowsByKey.get(key);
       if (!window || now - window.startedAt >= windowMs) {
         windowsByKey.set(key, { startedAt: now, count: 1 });
diff --git a/ui/src/components/ArtifactFileChip.tsx b/ui/src/components/ArtifactFileChip.tsx
index e5fe34d902a..8012eb6f39f 100644
--- a/ui/src/components/ArtifactFileChip.tsx
+++ b/ui/src/components/ArtifactFileChip.tsx
@@ -36,13 +36,23 @@ export function ArtifactFileChip({
     ? `Open ${workspaceFileRef.displayPath}${lineSuffix} in the file viewer`
     : `Workspace file ${workspaceFileRef.displayPath}${lineSuffix}`);
 
-  const handleClick = (event: MouseEvent<HTMLAnchorElement>) => {
+  const classNames = cn(
+    "paperclip-artifact-file-chip inline-flex items-center gap-1 rounded-sm border border-border bg-muted/60 px-1.5 py-0.5 font-mono text-xs leading-tight text-foreground/90 align-baseline no-underline hover:bg-muted focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-1",
+    canOpen ? "cursor-pointer" : null,
+    className,
+  );
+  const content = (
+    <>
+      {showIcon ? <FileCode2 aria-hidden="true" className="h-3 w-3 shrink-0 opacity-70" /> : null}
+      <span className="truncate max-w-[38ch]">{display}</span>
+    </>
+  );
+
+  const handleClick = (event: MouseEvent<HTMLButtonElement>) => {
     if (event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) {
       return;
     }
     if (event.button !== 0) return;
-    if (!canOpen) return;
-    event.preventDefault();
     if (onOpen) onOpen(workspaceFileRef);
     else {
       const workspace = workspaceFileRef.workspaceKind === "execution_workspace" ? "execution" : "project";
@@ -55,22 +65,31 @@ export function ArtifactFileChip({
     }
   };
 
+  if (!canOpen) {
+    return (
+      <span
+        data-artifact-file-chip="true"
+        data-workspace-file-path={workspaceFileRef.relativePath}
+        aria-label={ariaLabel}
+        title={tooltip}
+        className={classNames}
+      >
+        {content}
+      </span>
+    );
+  }
+
   return (
-    <a
-      href="#"
-      role={canOpen ? "button" : undefined}
+    <button
+      type="button"
       data-artifact-file-chip="true"
       data-workspace-file-path={workspaceFileRef.relativePath}
       aria-label={ariaLabel}
       title={tooltip}
-      className={cn(
-        "paperclip-artifact-file-chip inline-flex items-center gap-1 rounded-sm border border-border bg-muted/60 px-1.5 py-0.5 font-mono text-xs leading-tight text-foreground/90 align-baseline no-underline hover:bg-muted focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-1",
-        className,
-      )}
+      className={classNames}
       onClick={handleClick}
     >
-      {showIcon ? <FileCode2 aria-hidden="true" className="h-3 w-3 shrink-0 opacity-70" /> : null}
-      <span className="truncate max-w-[38ch]">{display}</span>
-    </a>
+      {content}
+    </button>
   );
 }
diff --git a/ui/src/components/FileViewerSheet.tsx b/ui/src/components/FileViewerSheet.tsx
index 0a205f026c2..6937657d4a1 100644
--- a/ui/src/components/FileViewerSheet.tsx
+++ b/ui/src/components/FileViewerSheet.tsx
@@ -432,7 +432,6 @@ export function FileViewerSheet({
   const state = typeof stateProp !== "undefined" ? stateProp : viewer.state;
   const computedOpen = typeof openProp === "boolean" ? openProp : state !== null || showPromptWhenEmpty;
 
-  const [loadStart, setLoadStart] = useState<number>(() => Date.now());
   const [elapsedMs, setElapsedMs] = useState(0);
   const [copiedField, setCopiedField] = useState<"path" | "link" | null>(null);
   const [announcement, setAnnouncement] = useState<string>("");
@@ -442,10 +441,10 @@ export function FileViewerSheet({
       setElapsedMs(0);
       return;
     }
-    setLoadStart(Date.now());
+    const now = Date.now();
     setElapsedMs(0);
     const interval = window.setInterval(() => {
-      setElapsedMs(Date.now() - loadStart);
+      setElapsedMs(Date.now() - now);
     }, 75);
     return () => window.clearInterval(interval);
     // eslint-disable-next-line react-hooks/exhaustive-deps
diff --git a/ui/src/pages/IssueDetail.test.tsx b/ui/src/pages/IssueDetail.test.tsx
index f4fac0ee087..b035cc50e4a 100644
--- a/ui/src/pages/IssueDetail.test.tsx
+++ b/ui/src/pages/IssueDetail.test.tsx
@@ -355,6 +355,7 @@ vi.mock("@/components/ui/sheet", () => ({
   Sheet: ({ children, open }: { children?: ReactNode; open?: boolean }) => (open ? <div>{children}</div> : null),
   SheetContent: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
   SheetHeader: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
+  SheetDescription: ({ children }: { children?: ReactNode }) => <p>{children}</p>,
   SheetTitle: ({ children }: { children?: ReactNode }) => <h2>{children}</h2>,
 }));