Merge remote-tracking branch 'origin/main' into feat/pdf-to-markdown-tool

Author: Zvonimir

.codex/review-prompt.md

@@ -24,10 +24,11 @@ When both exist, report blockers first.
 
 ### Review Method
 
-Do two passes:
+Do three passes:
 
-1. **Blockers pass** — Scan for correctness bugs, security issues, API/schema contract breaks, missing migrations, data integrity risks, and missing tests for changed behavior. These are `🔴 Bug` comments.
-2. **Maintainability pass** — Scan for code bloat, readability issues, naming problems, pattern violations, hardcoded values. These are `🟡 Issue`, `🔵 Nit`, or `💡 Suggestion` comments.
+1. **Context + risk-map pass (mandatory)** — Read full touched files (not only hunk lines) and identify high-risk zones: platform/runtime boundaries, event handlers, async cleanup (`finally`), auth/validation boundaries, and repeated predicates/guards.
+2. **Blockers pass** — Scan for correctness bugs, security issues, API/schema contract breaks, missing migrations, data integrity risks, and missing tests for changed behavior. These are `🔴 Bug` comments.
+3. **Maintainability pass** — Scan for code bloat, readability issues, naming problems, pattern violations, hardcoded values, and architecture drift in touched areas. These are `🟡 Issue`, `🔵 Nit`, or `💡 Suggestion` comments.
 
 ### Comment Gate
 
@@ -53,9 +54,12 @@ If any check fails, skip the comment.
 - Boundary conditions — empty arrays, null inputs, zero values, maximum values.
 - Error handling — swallowed errors, missing error propagation, unhelpful error messages. Do not flag missing error handling for internal code that cannot reasonably fail.
 - Streaming/multipart handlers — verify a request cannot send multiple responses (e.g., multi-file parts triggering repeated `res.json()` calls). If a route expects one file, ensure parser limits and single-response guards exist.
+- Unsafe runtime assumptions hidden by type assertions (`as ...`, `as any`, non-null `!`) when values come from events, external I/O, or platform-specific APIs.
+- Platform/runtime compatibility assumptions — usage of globals/APIs (`window`, `document`, `Node`, `process`, browser-only APIs) in cross-runtime code paths must be guarded.
 
 #### Security
 
+
 - Injection risks (SQL, command, XSS) when handling user input.
 - Hardcoded secrets — API keys, passwords, tokens in code.
 - Missing input validation at system boundaries (user input, external APIs). Not for internal function calls.
@@ -77,18 +81,25 @@ If any check fails, skip the comment.
 - Changed behavior must have updated tests reflecting the new expectations.
 - If tests are present but brittle (testing implementation details rather than behavior), flag it.
 - For single-file upload endpoints, look for regression coverage of multi-file/malformed multipart inputs and confirm no double-response behavior.
+- Prefer tests that validate production behavior directly. If a test re-implements production decision logic locally, and could stay green while runtime behavior regresses, flag it and suggest importing shared runtime logic or testing via a higher-level behavior path.
 
 Missing tests for changed behavior are blockers (`🔴 Bug`) only when the change affects user-facing behavior, API contracts, or data integrity. Missing tests for internal refactors or trivial changes are `🟡 Issue`.
 
 ### Pass 2: Maintainability
 
+#### Architecture Direction (Touched Area)
+
+- Evaluate whether the diff makes the touched area more or less maintainable (coupling, cohesion, readability of control flow).
+- Flag **architecture drift** when business decisions become more scattered (same guard/predicate duplicated across multiple call paths, UI/state/network logic further entangled, or test/runtime logic diverging).
+- When the same invariant-like predicate appears repeatedly in changed code, prefer a named helper/shared utility if it clearly reduces divergence risk.
+
 #### Code Bloat and Unnecessary Complexity
 
 - **Excessive code** — More lines than necessary. Could this be done in fewer lines without sacrificing clarity?
 - **Over-engineering** — Abstractions, helpers, or utilities for one-time operations. Premature generalization. Feature flags or config for things that could just be code.
 - **Speculative generality** — Code handling hypothetical future requirements nobody asked for.
 - **Dead code** — Unused variables, unreachable branches, commented-out code.
-- **Duplicate code** — Same logic repeated instead of extracted. But do not suggest extraction for only 2-3 similar lines — that is premature abstraction.
+- **Duplicate code** — Same logic repeated instead of extracted. Do not suggest extraction for only 2-3 similar lines unless the repeated logic encodes a correctness invariant across multiple paths (e.g., identical guard logic in multiple `finally` blocks).
 
 #### Readability and Naming
 
@@ -169,4 +180,4 @@ The `line` field must refer to the line number in the new version of the file (r
 
 ## Summary
 
-Write a brief (2–4 sentence) overall assessment in the `summary` field. Lead with blockers if any exist. Mention whether the PR is clean/minimal or has code quality issues. If the PR looks good, say so.
+Write a brief (2–4 sentence) overall assessment in the `summary` field. Lead with blockers if any exist. Mention whether the PR is clean/minimal or has code quality issues. Include one sentence on maintainability direction in touched areas (improved / neutral / worsened, and why). If the PR looks good, say so.

apps/agent/src/app/(protected)/chat.tsx

@@ -43,6 +43,10 @@ import {
   toToolExecutionMode,
   toToolExecutionSettings,
 } from "@/shared/toolExecutionMode";
+import {
+  isThinkingVisible,
+  shouldStopGenerating,
+} from "@/shared/thinkingIndicator";
 
 function normalizeStreamingMarkdown(content: string): string {
   const fencePattern = /^(`{3,})[^`]*$/gm;
@@ -321,7 +325,9 @@ export default function ChatPage() {
 
       addAssistantCompletion(completion);
     } finally {
-      setIsGenerating(false);
+      if (shouldStopGenerating(pendingToolCalls.current.size)) {
+        setIsGenerating(false);
+      }
     }
   }
 
@@ -418,7 +424,9 @@ export default function ChatPage() {
         timeout: 5000,
       });
     } finally {
-      setIsGenerating(false);
+      if (shouldStopGenerating(pendingToolCalls.current.size)) {
+        setIsGenerating(false);
+      }
     }
   }
 
@@ -440,7 +448,9 @@ export default function ChatPage() {
         timeout: 5000,
       });
     } finally {
-      setIsGenerating(false);
+      if (shouldStopGenerating(pendingToolCalls.current.size)) {
+        setIsGenerating(false);
+      }
     }
   }
 
@@ -477,7 +487,9 @@ export default function ChatPage() {
 
       addAssistantCompletion(completion);
     } finally {
-      setIsGenerating(false);
+      if (shouldStopGenerating(pendingToolCalls.current.size)) {
+        setIsGenerating(false);
+      }
     }
   }
 
@@ -792,7 +804,7 @@ export default function ChatPage() {
 
                 return <View key={i}>{messageContent}</View>;
               })}
-              {isGenerating && streamingContent === null && <Chat.Thinking />}
+              {isThinkingVisible(isGenerating, streamingContent) && <Chat.Thinking />}
               {streamingContent !== null && (
                 <Chat.Message icon="assistant">
                   <Markdown>

apps/agent/src/components/Chat/Input.tsx

@@ -36,13 +36,20 @@ import {
   getChatInputHeightFromText,
 } from "../../shared/chatInputHeight";
 import { getChatInputKeyAction } from "../../shared/chatInputKeyPress";
+import {
+  SCROLLBAR_HIDE_DELAY_MS,
+  SCROLLBAR_TRACK_INSET,
+  SCROLLBAR_MIN_THUMB_HEIGHT,
+  SCROLLBAR_THUMB_WIDTH,
+  SCROLLBAR_TRACK_WIDTH,
+  SCROLLBAR_THUMB_BORDER_RADIUS,
+  SCROLLBAR_THUMB_OPACITY,
+  SCROLLBAR_TRACK_RIGHT,
+} from "../../shared/customScrollbar";
 
 import ChatInputFilesSelected from "./Input/FilesSelected";
 import ChatInputToolsSelector from "./Input/ToolsSelector";
 
-const CHAT_INPUT_SCROLLBAR_HIDE_DELAY_MS = 5000;
-const CHAT_INPUT_SCROLLBAR_TRACK_INSET = 8;
-const CHAT_INPUT_SCROLLBAR_MIN_THUMB_HEIGHT = 24;
 const CHAT_INPUT_TEST_ID = "chat-input";
 const CHAT_INPUT_SELECTOR = `[data-testid="${CHAT_INPUT_TEST_ID}"]`;
 
@@ -99,7 +106,7 @@ export default function ChatInput({
   const [inputHeight, setInputHeight] = useState(CHAT_INPUT_MIN_HEIGHT);
   const [isCustomScrollbarVisible, setIsCustomScrollbarVisible] = useState(false);
   const [customScrollbarThumbTop, setCustomScrollbarThumbTop] = useState(
-    CHAT_INPUT_SCROLLBAR_TRACK_INSET,
+    SCROLLBAR_TRACK_INSET,
   );
   const [customScrollbarThumbHeight, setCustomScrollbarThumbHeight] = useState(0);
   const hideScrollbarTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(
@@ -112,6 +119,7 @@ export default function ChatInput({
     startScrollTop: number;
   } | null>(null);
   const restoreUserSelectRef = useRef<string | null>(null);
+  const modeDropdownContainerRef = useRef<View>(null);
   const isBusy = disabled || isUploading;
   const isWeb = Platform.OS === "web";
   const isInputAtMaxHeight = inputHeight >= CHAT_INPUT_MAX_HEIGHT;
@@ -122,6 +130,29 @@ export default function ChatInput({
     (m) => m.value === toolExecutionMode,
   )!;
 
+  // Close mode dropdown on click outside (web only)
+  useEffect(() => {
+    if (!isModeDropdownOpen || !isWeb) return;
+    const handler = (e: MouseEvent) => {
+      // Ignore clicks inside the dropdown container (trigger + menu)
+      const container = modeDropdownContainerRef.current;
+      if (
+        isHTMLElement(container) &&
+        typeof Node !== "undefined" &&
+        e.target instanceof Node &&
+        container.contains(e.target)
+      )
+        return;
+      setIsModeDropdownOpen(false);
+    };
+    // setTimeout(0) prevents the opening click from immediately triggering close
+    const id = setTimeout(() => document.addEventListener("click", handler), 0);
+    return () => {
+      clearTimeout(id);
+      document.removeEventListener("click", handler);
+    };
+  }, [isModeDropdownOpen, isWeb]);
+
   const clearHideScrollbarTimeout = () => {
     if (!hideScrollbarTimeoutRef.current) return;
     clearTimeout(hideScrollbarTimeoutRef.current);
@@ -138,7 +169,7 @@ export default function ChatInput({
     hideScrollbarTimeoutRef.current = setTimeout(() => {
       setIsCustomScrollbarVisible(false);
       hideScrollbarTimeoutRef.current = null;
-    }, CHAT_INPUT_SCROLLBAR_HIDE_DELAY_MS);
+    }, SCROLLBAR_HIDE_DELAY_MS);
   };
 
   const updateCustomScrollbar = ({
@@ -155,27 +186,27 @@ export default function ChatInput({
       viewportHeight >= CHAT_INPUT_MAX_HEIGHT &&
       contentHeight > viewportHeight + 1;
     if (!isScrollable) {
-      setCustomScrollbarThumbTop(CHAT_INPUT_SCROLLBAR_TRACK_INSET);
+      setCustomScrollbarThumbTop(SCROLLBAR_TRACK_INSET);
       setCustomScrollbarThumbHeight(0);
       hideCustomScrollbar();
       return;
     }
 
     const trackHeight = Math.max(
       0,
-      viewportHeight - CHAT_INPUT_SCROLLBAR_TRACK_INSET * 2,
+      viewportHeight - SCROLLBAR_TRACK_INSET * 2,
     );
     if (trackHeight <= 0) return;
 
     const thumbHeight = Math.max(
-      CHAT_INPUT_SCROLLBAR_MIN_THUMB_HEIGHT,
+      SCROLLBAR_MIN_THUMB_HEIGHT,
       Math.min(trackHeight, (viewportHeight / contentHeight) * trackHeight),
     );
     const maxScrollTop = Math.max(1, contentHeight - viewportHeight);
     const clampedScrollTop = Math.min(maxScrollTop, Math.max(0, scrollTop));
     const maxThumbOffset = Math.max(0, trackHeight - thumbHeight);
     const thumbTop =
-      CHAT_INPUT_SCROLLBAR_TRACK_INSET +
+      SCROLLBAR_TRACK_INSET +
       (clampedScrollTop / maxScrollTop) * maxThumbOffset;
 
     setCustomScrollbarThumbTop(thumbTop);
@@ -203,10 +234,10 @@ export default function ChatInput({
 
     const trackHeight = Math.max(
       1,
-      viewportHeight - CHAT_INPUT_SCROLLBAR_TRACK_INSET * 2,
+      viewportHeight - SCROLLBAR_TRACK_INSET * 2,
     );
     const thumbHeight = Math.max(
-      CHAT_INPUT_SCROLLBAR_MIN_THUMB_HEIGHT,
+      SCROLLBAR_MIN_THUMB_HEIGHT,
       Math.min(trackHeight, (viewportHeight / contentHeight) * trackHeight),
     );
     const maxThumbOffset = Math.max(1, trackHeight - thumbHeight);
@@ -345,7 +376,7 @@ export default function ChatInput({
     setMessage("");
     setSelectedFiles([]);
     setInputHeight(CHAT_INPUT_MIN_HEIGHT);
-    setCustomScrollbarThumbTop(CHAT_INPUT_SCROLLBAR_TRACK_INSET);
+    setCustomScrollbarThumbTop(SCROLLBAR_TRACK_INSET);
     setCustomScrollbarThumbHeight(0);
     inputScrollTopRef.current = 0;
     hideCustomScrollbar();
@@ -497,7 +528,7 @@ export default function ChatInput({
               </Text>
             </Pressable>
 
-            <View style={styles.modeSelectorContainer}>
+            <View ref={modeDropdownContainerRef} style={styles.modeSelectorContainer}>
               <Pressable
                 style={[
                   styles.modeSelectorTrigger,
@@ -535,14 +566,20 @@ export default function ChatInput({
                   ]}
                 >
                   {TOOL_EXECUTION_MODE_OPTIONS.map((option) => {
+                    const isSelected = option.value === toolExecutionMode;
                     return (
                       <Pressable
                         key={option.value}
-                        style={[
+                        style={(state) => [
                           styles.modeSelectorOption,
-                          option.value === toolExecutionMode && {
+                          isSelected && {
                             backgroundColor: colors.backgroundFlat,
                           },
+                          "hovered" in state &&
+                            (state as any).hovered &&
+                            !isSelected && {
+                              backgroundColor: colors.backgroundFlat + "66",
+                            },
                         ]}
                         onPress={() => {
                           onToolExecutionModeChange?.(option.value);
@@ -639,16 +676,16 @@ const styles = StyleSheet.create({
   inputScrollbarTrack: {
     position: "absolute",
     top: 0,
-    right: 5,
+    right: SCROLLBAR_TRACK_RIGHT,
     bottom: 0,
-    width: 8,
+    width: SCROLLBAR_TRACK_WIDTH,
   },
   inputScrollbarThumb: {
     position: "absolute",
     right: 0,
-    width: 6,
-    borderRadius: 999,
-    opacity: 0.72,
+    width: SCROLLBAR_THUMB_WIDTH,
+    borderRadius: SCROLLBAR_THUMB_BORDER_RADIUS,
+    opacity: SCROLLBAR_THUMB_OPACITY,
   },
   toolbar: {
     flexDirection: "row",