From d3ee3f999b013b9488c93dabffacb7ec1e28f6ae Mon Sep 17 00:00:00 2001 From: Logan Yang Date: Sat, 14 Mar 2026 16:17:37 -0700 Subject: [PATCH] fix: agent loop improvements, clickable citations, and UI fixes (#2287) * fix: prevent agent ReAct loop from getting stuck with small local models Small local models (e.g. qwen3.5-4b-mlx) get stuck calling localSearch repeatedly with near-identical queries and never synthesizing an answer. - Remove QueryExpander from agent loop to avoid model contention on single-connection local models (retriever still expands internally) - Add query deduplication that pre-filters tool call batches using Jaccard word-overlap similarity before execution - Force synthesis via raw model (without tools) after 2 consecutive iterations where all tool calls are duplicates - Add agent-specific system prompt guidance for search limits - Strip leaked chat template role tokens from intermediate content - Add intermediate model output logging for debugging Co-Authored-By: Claude Opus 4.6 * feat: make inline citation numbers clickable and restore Max Sources setting Inline citation numbers like [1], [2] in chat responses are now clickable links that open the corresponding source note. Also restores the Max Sources slider in QA settings, reverting the hardcoded DEFAULT_MAX_SOURCE_CHUNKS. Co-Authored-By: Claude Opus 4.6 * fix: remove excessive 32px bottom padding from chat view on desktop The padding-bottom on .view-content used max(safe-area + 4px, 32px), forcing a 32px minimum even on desktop where safe-area-inset-bottom is 0. Simplified to safe-area + 4px so desktop gets minimal padding while devices with safe areas still get proper spacing. Co-Authored-By: Claude Opus 4.6 * fix(agent): track dedup queries post-execution to keep failed searches retryable Move previousSearchQueries tracking from before to after each localSearch tool call executes. Previously, queries were marked as seen before any tool ran, so if a call was aborted or the loop threw early, those queries were still blocked from retrying on the next iteration. Now each query is added to previousSearchQueries immediately after its tool call returns, keeping the dedup effective for completed calls while leaving transient failures retryable. Co-Authored-By: Claude Sonnet 4.6 * fix(citations): preserve Obsidian internal-link attributes on inline citation anchors When building clickable inline citation links (e.g. [1], [2]), the previous code only copied the href from the source section anchor. Obsidian vault-note links have additional attributes such as data-href and class="internal-link" that are required for in-app note navigation. Without them, clicking an inline citation resolves the link as a plain relative URL instead of opening the corresponding note. Fix: store the source anchor element rather than just the href, then copy all attributes onto the generated citation anchor before setting the citation-specific class and aria-label overrides. Co-Authored-By: Claude Sonnet 4.6 * fix(agent): only mark localSearch queries as seen after successful execution Change the dedup tracking condition from always tracking queries post-execution to only tracking when the tool call succeeds (result.success is true). This ensures transient failures (e.g. temporary index unavailability) leave the query retryable: if a localSearch fails, the model can issue the same query again on the next iteration. Previously, even a failed execution would mark the query as seen, blocking recovery. Co-Authored-By: Claude Sonnet 4.6 * fix: deduplicate inline citations after source consolidation When multiple chunks come from the same note, the LLM cites them separately. After consolidation maps them to the same source number, adjacent duplicates like [1][1] or [1] and [1] now collapse to [1]. Co-Authored-By: Claude Opus 4.6 * chore: update GPT-5.2 to GPT-5.4 in builtin models Co-Authored-By: Claude Opus 4.6 * fix: add dynamic status bar clearance for desktop chat view The chat input was hidden behind Obsidian's status bar on desktop. Adds ChatViewLayout which measures the geometric overlap between the view-content and the status bar, setting a CSS variable to provide the exact padding needed. Adapts to any theme: no clearance for auto-hide themes (opacity: 0) or themes that already account for the status bar in their layout; correct clearance for themes where the bar is visible and overlapping. Re-checks on theme switches via debounced css-change listener. Co-Authored-By: Claude Opus 4.6 (1M context) * fix: suppress thinking tokens during streaming when reasoning is disabled When a local model (e.g. Qwen 3.5 via LM Studio) emits text-level tags but the user has not enabled the reasoning capability, the thinking content was visibly rendered during streaming and only stripped once the tag arrived. Now tracks the position of excluded think blocks across chunks and truncates fullResponse to the safe prefix on each chunk, so thinking content never appears. Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 --- docs/llm-providers.md | 2 +- docs/models-and-parameters.md | 4 +- src/LLMProviders/ChatOpenRouter.ts | 2 +- src/LLMProviders/chainManager.ts | 6 +- .../chainRunner/AutonomousAgentChainRunner.ts | 225 +++++++++++++----- .../chainRunner/VaultQAChainRunner.ts | 17 +- .../chainRunner/utils/AgentReasoningState.ts | 3 + .../chainRunner/utils/ThinkBlockStreamer.ts | 83 +++++-- .../chainRunner/utils/citationUtils.test.ts | 113 ++++++++- .../chainRunner/utils/citationUtils.ts | 46 +++- .../utils/queryDeduplication.test.ts | 126 ++++++++++ .../chainRunner/utils/queryDeduplication.ts | 74 ++++++ src/components/CopilotView.tsx | 5 + .../chat-components/ChatSingleMessage.tsx | 106 +++++++++ .../chat-components/ChatViewLayout.ts | 83 +++++++ src/components/ui/ModelParametersEditor.tsx | 6 +- src/constants.ts | 10 +- src/settings/v2/components/QASettings.tsx | 12 + src/styles/tailwind.css | 17 +- src/tools/SearchTools.ts | 10 +- 20 files changed, 830 insertions(+), 120 deletions(-) create mode 100644 src/LLMProviders/chainRunner/utils/queryDeduplication.test.ts create mode 100644 src/LLMProviders/chainRunner/utils/queryDeduplication.ts create mode 100644 src/components/chat-components/ChatViewLayout.ts diff --git a/docs/llm-providers.md b/docs/llm-providers.md index bc2b202a..bd0e3c70 100644 --- a/docs/llm-providers.md +++ b/docs/llm-providers.md @@ -31,7 +31,7 @@ OpenRouter is a gateway that provides access to hundreds of models from many pro Direct access to GPT-4.1, GPT-5, and other OpenAI models. - **Get a key**: https://platform.openai.com/api-keys -- **Models include**: GPT-5.2, GPT-5 mini, GPT-5 nano, GPT-4.1, GPT-4.1 mini, GPT-4.1 nano, o4-mini (reasoning) +- **Models include**: GPT-5.4, GPT-5 mini, GPT-5 nano, GPT-4.1, GPT-4.1 mini, GPT-4.1 nano, o4-mini (reasoning) - **Setting key**: `openAIApiKey` ### Anthropic diff --git a/docs/models-and-parameters.md b/docs/models-and-parameters.md index 3def0cf6..36f284c4 100644 --- a/docs/models-and-parameters.md +++ b/docs/models-and-parameters.md @@ -17,9 +17,9 @@ Copilot comes with a set of built-in models across many providers. Some are alwa | google/gemini-2.5-pro | OpenRouter | Vision | | google/gemini-3-flash-preview | OpenRouter | Vision, Reasoning | | google/gemini-3.1-pro-preview | OpenRouter | Vision, Reasoning | -| openai/gpt-5.2 | OpenRouter | Vision | +| openai/gpt-5.4 | OpenRouter | Vision | | openai/gpt-5-mini | OpenRouter | Vision | -| gpt-5.2 | OpenAI | Vision | +| gpt-5.4 | OpenAI | Vision | | gpt-5-mini | OpenAI | Vision | | gpt-4.1 | OpenAI | Vision | | gpt-4.1-mini | OpenAI | Vision | diff --git a/src/LLMProviders/ChatOpenRouter.ts b/src/LLMProviders/ChatOpenRouter.ts index 12953350..325dbf46 100644 --- a/src/LLMProviders/ChatOpenRouter.ts +++ b/src/LLMProviders/ChatOpenRouter.ts @@ -30,7 +30,7 @@ export interface ChatOpenRouterInput extends BaseChatModelParams { * Reasoning effort level: "minimal", "low", "medium", "high", or "xhigh" * Controls the amount of reasoning the model uses * Note: "minimal" will be treated as "low" for OpenRouter - * Note: "xhigh" is only supported by GPT-5.2 models + * Note: "xhigh" is only supported by GPT-5.4 models */ reasoningEffort?: "minimal" | "low" | "medium" | "high" | "xhigh"; diff --git a/src/LLMProviders/chainManager.ts b/src/LLMProviders/chainManager.ts index e490d7e5..0437e20c 100644 --- a/src/LLMProviders/chainManager.ts +++ b/src/LLMProviders/chainManager.ts @@ -6,7 +6,7 @@ import { setChainType, } from "@/aiParams"; import ChainFactory, { ChainType, Document } from "@/chainFactory"; -import { BUILTIN_CHAT_MODELS, DEFAULT_MAX_SOURCE_CHUNKS, USER_SENDER } from "@/constants"; +import { BUILTIN_CHAT_MODELS, USER_SENDER } from "@/constants"; import { AutonomousAgentChainRunner, ChainRunner, @@ -220,12 +220,12 @@ export default class ChainManager { const retriever = settings.enableSemanticSearchV3 ? new (await import("@/search/hybridRetriever")).HybridRetriever({ minSimilarityScore: 0.01, - maxK: DEFAULT_MAX_SOURCE_CHUNKS, + maxK: settings.maxSourceChunks, salientTerms: [], }) : new (await import("@/search/v3/TieredLexicalRetriever")).TieredLexicalRetriever(app, { minSimilarityScore: 0.01, - maxK: DEFAULT_MAX_SOURCE_CHUNKS, + maxK: settings.maxSourceChunks, salientTerms: [], textWeight: undefined, returnAll: false, diff --git a/src/LLMProviders/chainRunner/AutonomousAgentChainRunner.ts b/src/LLMProviders/chainRunner/AutonomousAgentChainRunner.ts index b3fbd27a..a195dd8f 100644 --- a/src/LLMProviders/chainRunner/AutonomousAgentChainRunner.ts +++ b/src/LLMProviders/chainRunner/AutonomousAgentChainRunner.ts @@ -43,9 +43,14 @@ import { serializeReasoningBlock, summarizeToolCall, summarizeToolResult, - QueryExpansionInfo, } from "./utils/AgentReasoningState"; -import { QueryExpander } from "@/search/v3/QueryExpander"; +import { findDuplicateQuery, stripLeakedRoleLines } from "./utils/queryDeduplication"; + +const AGENT_LOOP_GUIDANCE = `## Agent Behavior +- You have a limited number of tool calls. Use them wisely. +- NEVER search for the same or very similar query twice. If results were insufficient, try substantially different terms. +- After 1-2 searches, synthesize an answer from the results you have. Do not keep searching unless the results are clearly insufficient. +- If you have enough information to answer, respond directly without calling any more tools.`; type AgentSource = { title: string; @@ -89,6 +94,7 @@ interface AgentRunContext { */ interface ReActLoopParams { boundModel: Runnable; + chatModel: Runnable; // Raw model without tools, used for forced synthesis tools: StructuredTool[]; messages: BaseMessage[]; originalPrompt: string; @@ -286,6 +292,9 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { return serializeReasoningBlock(this.reasoningState); } + // TODO: Unify system prompt construction -- this static method and prepareAgentConversation() + // both independently gather tool metadata, build tool instructions, and append AGENT_LOOP_GUIDANCE. + // Extract a shared helper like buildAgentSystemPromptSuffix(tools) to avoid drift. /** * Generate system prompt for the autonomous agent. * Note: Tool schemas are handled by bindTools(), so we only include @@ -310,10 +319,12 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { .map((meta) => `For ${meta.displayName}: ${meta.customPromptInstructions}`) .join("\n"); + const parts = [basePrompt]; if (toolInstructions) { - return `${basePrompt}\n\n## Tool Guidelines\n${toolInstructions}`; + parts.push(`## Tool Guidelines\n${toolInstructions}`); } - return basePrompt; + parts.push(AGENT_LOOP_GUIDANCE); + return parts.join("\n\n"); } /** @@ -422,6 +433,7 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { // Run the simplified ReAct loop with native tool calling const loopResult = await this.runReActLoop({ boundModel: context.loopDeps.boundModel, + chatModel, tools: context.loopDeps.availableTools, messages: context.messages, originalPrompt: context.originalUserPrompt, @@ -575,10 +587,11 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { .map((meta) => `For ${meta.displayName}: ${meta.customPromptInstructions}`) .join("\n"); - // Combine system message with tool guidelines + // Combine system message with tool guidelines and agent loop guidance const systemContent = [ systemMessage?.content || "", toolInstructions ? `\n## Tool Guidelines\n${toolInstructions}` : "", + AGENT_LOOP_GUIDANCE, ] .filter(Boolean) .join("\n\n"); @@ -642,6 +655,8 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { const collectedSources: AgentSource[] = []; const loopStartTime = Date.now(); + const previousSearchQueries: string[] = []; + let consecutiveAllSkipped = 0; let iteration = 0; let responseMetadata: ResponseMetadata | undefined; @@ -671,11 +686,19 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { tokenUsage: streamingResult.tokenUsage ?? undefined, }; + const trimmedContent = content?.trim(); + logInfo( + `[Agent] Iteration ${iteration} model output:`, + trimmedContent ? trimmedContent.slice(0, 200) : "(empty)" + ); + // Check for native tool calls const toolCalls = aiMessage.tool_calls || []; + logInfo(`[Agent] Iteration ${iteration}: ${toolCalls.length} tool call(s)`); // No tool calls = final response if (toolCalls.length === 0) { + logInfo(`[Agent] Iteration ${iteration}: Final response (no tool calls)`); // Stop reasoning timer and finalize the reasoning block this.stopReasoningTimer(); this.reasoningState.status = "complete"; @@ -732,22 +755,72 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { }; } - // Add AI message with tool calls - but DON'T accumulate intermediate content - // Intermediate content (like "I'll search for..." ) should not appear in final response - messages.push(aiMessage); + // Clean leaked role tokens from intermediate content before adding to conversation. + // Small local models leak chat template fragments (e.g. <|im_start|>user -> "user") + // that would otherwise pollute the conversation history and confuse subsequent iterations. + const cleanedContent = stripLeakedRoleLines(content); + const intermediateMessage = new AIMessage({ + content: cleanedContent, + tool_calls: aiMessage.tool_calls, + }); + messages.push(intermediateMessage); // For iterations > 1, the model's content often contains its summary of findings // from previous tool calls. Extract first sentence as a "finding summary". // (Iteration 1 has no previous findings - its content is just "I'll search for...") - if (iteration > 1 && content && content.trim().length > 0) { - const findingSummary = extractFirstSentence(content); + if (iteration > 1 && cleanedContent && cleanedContent.trim().length > 0) { + const findingSummary = extractFirstSentence(cleanedContent); if (findingSummary) { this.addReasoningStep(findingSummary); } } - // Execute each tool + // --- Pre-deduplicate tool calls before execution --- + // Prevents within-batch duplicates (model emits 3 similar searches at once) + // and cross-iteration duplicates (model retries a query from a previous iteration). + // Duplicates are silently handled: ToolMessage feedback is sent but no reasoning step shown. + // TODO: Make dedup a ToolRegistry metadata property (e.g. deduplicateQueries: true) + // so the loop handles it generically instead of hardcoding "localSearch". + const uniqueToolCalls: typeof toolCalls = []; + const batchQueries: string[] = []; + // Parallel array: for each entry in uniqueToolCalls, the associated localSearch query + // (or null for non-localSearch calls). Used to track queries post-execution. + const uniqueToolCallQueries: Array = []; + for (const tc of toolCalls) { + if (tc.name === "localSearch") { + const query = (tc.args as Record)?.query as string | undefined; + if (query) { + const duplicate = + findDuplicateQuery(query, previousSearchQueries) ?? + findDuplicateQuery(query, batchQueries); + if (duplicate) { + logInfo(`[Agent] Dedup: "${query}" (similar to: "${duplicate}")`); + messages.push( + createToolResultMessage( + tc.id || generateToolCallId(), + tc.name, + `You already searched for a similar query: "${duplicate}". Synthesize your answer from existing results.` + ) + ); + continue; + } + batchQueries.push(query); + uniqueToolCallQueries.push(query); + } else { + uniqueToolCallQueries.push(null); + } + } else { + uniqueToolCallQueries.push(null); + } + uniqueToolCalls.push(tc); + } + // Note: queries are tracked in previousSearchQueries after each tool executes, + // not here, so that transient failures leave queries retryable on the next iteration. + + // Execute unique tool calls + for (let tcIdx = 0; tcIdx < uniqueToolCalls.length; tcIdx++) { + const tc = uniqueToolCalls[tcIdx]; if (abortController.signal.aborted) break; const toolCall = { @@ -755,58 +828,10 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { args: tc.args as Record, }; - // Pre-expand query for localSearch to show expanded terms BEFORE search - let preExpandedTerms: QueryExpansionInfo | undefined; - if (tc.name === "localSearch") { - const query = toolCall.args.query as string | undefined; - if (query) { - try { - const expander = new QueryExpander({ - getChatModel: async () => { - return this.chainManager.chatModelManager.getChatModel(); - }, - }); - const expansion = await expander.expand(query); - // Compute recall terms (all terms used for search) - const seen = new Set(); - const recallTerms: string[] = []; - const addTerm = (term: unknown) => { - if (typeof term !== "string") return; - const trimmed = term.trim(); - // Filter out invalid terms like "[object Object]" - if (!trimmed || trimmed === "[object Object]" || trimmed.startsWith("[object ")) - return; - const normalized = trimmed.toLowerCase(); - if (!seen.has(normalized)) { - seen.add(normalized); - recallTerms.push(trimmed); - } - }; - if (expansion.originalQuery) addTerm(expansion.originalQuery); - (expansion.salientTerms || []).forEach(addTerm); - (expansion.expandedQueries || []).forEach(addTerm); - - preExpandedTerms = { - originalQuery: expansion.originalQuery, - salientTerms: expansion.salientTerms, - expandedQueries: expansion.expandedQueries, - recallTerms, - }; - } catch { - // Ignore expansion errors, fall back to basic summary - } - } - } - // Add tool call step (shown in both rolling display and expanded view) - const toolCallSummary = summarizeToolCall(tc.name, toolCall.args, preExpandedTerms); + const toolCallSummary = summarizeToolCall(tc.name, toolCall.args); this.addReasoningStep(toolCallSummary, tc.name); - // Inject pre-expanded query data into args to avoid double expansion in search - if (preExpandedTerms) { - toolCall.args._preExpandedQuery = preExpandedTerms; - } - logToolCall(toolCall, iteration); // Execute the tool @@ -834,6 +859,16 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { logToolResult(tc.name, result); + // Track the executed query only on success so transient failures remain retryable. + // A failed localSearch means no results were retrieved; the model should be able + // to issue the same query again on the next iteration if it chooses to. + if (tc.name === "localSearch" && result.success) { + const executedQuery = uniqueToolCallQueries[tcIdx]; + if (executedQuery) { + previousSearchQueries.push(executedQuery); + } + } + // Add tool result step (shown in rolling display - this is the "what was found") const resultSummary = summarizeToolResult(tc.name, result, sourceInfo, toolCall.args); this.addReasoningStep(resultSummary, tc.name); @@ -846,6 +881,61 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { ); messages.push(toolMessage); } + + // Detect stuck model: all tool calls were duplicates (none made it through dedup) + if (uniqueToolCalls.length === 0 && toolCalls.length > 0) { + consecutiveAllSkipped++; + logInfo( + `[Agent] All ${toolCalls.length} tool call(s) skipped as duplicates ` + + `(${consecutiveAllSkipped} consecutive)` + ); + + if (consecutiveAllSkipped >= 2) { + // Model is stuck in a search loop -- force synthesis without tools + logInfo("[Agent] Model stuck in search loop, forcing synthesis without tools"); + this.addReasoningStep("Synthesizing answer from search results"); + + messages.push( + new HumanMessage( + "You have already searched and found relevant results. Do not call any tools. " + + "Answer the following question now based ONLY on the search results above:\n\n" + + originalPrompt + ) + ); + + // Call raw model (without tools) to guarantee text-only response + const synthesis = await this.streamModelResponse( + params.chatModel, + messages, + abortController, + updateCurrentAiMessage + ); + + responseMetadata = { + wasTruncated: synthesis.streamingResult.wasTruncated, + tokenUsage: synthesis.streamingResult.tokenUsage ?? undefined, + }; + + // Finalize as final response + this.stopReasoningTimer(); + this.reasoningState.status = "complete"; + const reasoningBlock = this.buildReasoningBlockMarkup(); + const finalContent = + synthesis.content || "Unable to synthesize a response from the search results."; + const finalResponse = reasoningBlock + ? reasoningBlock + "\n\n" + finalContent + : finalContent; + updateCurrentAiMessage(finalResponse); + + return { + finalResponse, + sources: collectedSources, + responseMetadata, + }; + } + } else { + consecutiveAllSkipped = 0; + } } // Stop reasoning timer @@ -921,6 +1011,8 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { true // excludeThinking = true for agent mode ); + let rawContent = ""; // Accumulate raw content before think-stripping, for debug logging + try { const stream = await withSuppressedTokenWarnings(() => boundModel.stream(messages, { @@ -946,6 +1038,10 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { } } + // Accumulate raw content for debug logging (before think-stripping) + const chunkContent = typeof chunk.content === "string" ? chunk.content : ""; + if (chunkContent) rawContent += chunkContent; + // Process chunk through ThinkBlockStreamer to strip thinking content thinkStreamer.processChunk(chunk); } @@ -954,6 +1050,15 @@ export class AutonomousAgentChainRunner extends CopilotPlusChainRunner { const streamingResult = thinkStreamer.close(); const fullContent = streamingResult.content; + // Log raw vs stripped content difference for debugging model behavior + const rawTrimmed = rawContent.trim(); + const strippedTrimmed = fullContent.trim(); + if (rawTrimmed && !strippedTrimmed) { + logInfo( + `[Agent] Model produced content that was entirely stripped (likely think blocks): ${rawTrimmed.slice(0, 300)}` + ); + } + // Build tool calls from accumulated chunks (with sanitization for empty objects) const toolCalls = buildToolCallsFromChunks(toolCallChunks); diff --git a/src/LLMProviders/chainRunner/VaultQAChainRunner.ts b/src/LLMProviders/chainRunner/VaultQAChainRunner.ts index d904ddcc..7a8b1a46 100644 --- a/src/LLMProviders/chainRunner/VaultQAChainRunner.ts +++ b/src/LLMProviders/chainRunner/VaultQAChainRunner.ts @@ -1,9 +1,4 @@ -import { - ABORT_REASON, - DEFAULT_MAX_SOURCE_CHUNKS, - ModelCapability, - RETRIEVED_DOCUMENT_TAG, -} from "@/constants"; +import { ABORT_REASON, ModelCapability, RETRIEVED_DOCUMENT_TAG } from "@/constants"; import { getStandaloneQuestion } from "@/chainUtils"; import { LayerToMessagesConverter } from "@/context/LayerToMessagesConverter"; import { logInfo } from "@/logger"; @@ -106,7 +101,7 @@ export class VaultQAChainRunner extends BaseChainRunner { const hasTagTerms = tags.length > 0; const filterRetriever = new FilterRetriever(app, { salientTerms: hasTagTerms ? [...tags] : [], - maxK: DEFAULT_MAX_SOURCE_CHUNKS, + maxK: settings.maxSourceChunks, returnAll: hasTagTerms, }); const filterDocs = await filterRetriever.getRelevantDocuments(standaloneQuestion); @@ -119,7 +114,7 @@ export class VaultQAChainRunner extends BaseChainRunner { app, { minSimilarityScore: 0.01, - maxK: DEFAULT_MAX_SOURCE_CHUNKS, + maxK: settings.maxSourceChunks, salientTerms: hasTagTerms ? [...tags] : [], tagTerms: tags, returnAll: hasTagTerms, @@ -134,8 +129,8 @@ export class VaultQAChainRunner extends BaseChainRunner { const { filterResults, searchResults } = mergeFilterAndSearchResults(filterDocs, searchDocs); // Cap total docs to prevent oversized prompts (filter results prioritized) const merged = [...filterResults, ...searchResults]; - const retrieverCapReached = merged.length > DEFAULT_MAX_SOURCE_CHUNKS; - const retrievedDocs = merged.slice(0, DEFAULT_MAX_SOURCE_CHUNKS); + const retrieverCapReached = merged.length > settings.maxSourceChunks; + const retrievedDocs = merged.slice(0, settings.maxSourceChunks); // Store retrieved documents for sources this.chainManager.storeRetrieverDocuments(retrievedDocs); @@ -165,7 +160,7 @@ export class VaultQAChainRunner extends BaseChainRunner { const sourceCatalog = formatSourceCatalog(sourceEntries).join("\n"); const capNotice = retrieverCapReached - ? `\n\nIMPORTANT: The retrieval limit of ${DEFAULT_MAX_SOURCE_CHUNKS} documents was reached. ${merged.length - DEFAULT_MAX_SOURCE_CHUNKS} additional matching documents were omitted. Inform the user: "Note: The retrieval cap was reached — some matching documents were not included. Upgrade to Copilot Plus for more complete answers."` + ? `\n\nIMPORTANT: The retrieval limit of ${settings.maxSourceChunks} documents was reached. ${merged.length - settings.maxSourceChunks} additional matching documents were omitted. Inform the user: "Note: The retrieval cap was reached — some matching documents were not included. Upgrade to Copilot Plus for more complete answers."` : ""; const qaInstructions = diff --git a/src/LLMProviders/chainRunner/utils/AgentReasoningState.ts b/src/LLMProviders/chainRunner/utils/AgentReasoningState.ts index 08045d4f..184a57e4 100644 --- a/src/LLMProviders/chainRunner/utils/AgentReasoningState.ts +++ b/src/LLMProviders/chainRunner/utils/AgentReasoningState.ts @@ -278,6 +278,9 @@ export function summarizeToolResult( } } +// TODO: The `expansion` parameter and QueryExpansionInfo interface are now dead code -- +// the agent runner no longer pre-expands queries. Clean up the expansion branch below +// and the _preExpandedQuery schema field in SearchTools.ts. /** * Generate a summary for when a tool is being called. * diff --git a/src/LLMProviders/chainRunner/utils/ThinkBlockStreamer.ts b/src/LLMProviders/chainRunner/utils/ThinkBlockStreamer.ts index a77c687a..be0e27ed 100644 --- a/src/LLMProviders/chainRunner/utils/ThinkBlockStreamer.ts +++ b/src/LLMProviders/chainRunner/utils/ThinkBlockStreamer.ts @@ -25,6 +25,9 @@ export class ThinkBlockStreamer { private tokenUsage: TokenUsage | null = null; // Track if we've handled text-level think tags (e.g., from nvidia/nemotron) private hasHandledTextLevelThinkTag = false; + // Character index where an excluded text-level think block started. + // -1 means we're not currently inside an excluded block. + private excludedThinkBlockStart = -1; // Native tool call accumulation private toolCallChunks: Map = new Map(); @@ -38,33 +41,24 @@ export class ThinkBlockStreamer { } /** - * Handle text-level think tags embedded in content (e.g., nvidia/nemotron models). + * Handle text-level think tags embedded in content (e.g., nvidia/nemotron, qwen3 models). * Some models output thinking with only closing tag, no opening tag. * This method detects and fixes this during streaming. + * + * When excludeThinking is true, thinking content is suppressed during streaming + * (not just stripped after the close tag arrives) by tracking the position where + * the excluded block started and truncating fullResponse on each chunk. */ private handleTextLevelThinkTags() { + if (this.excludeThinking) { + this.handleExcludedThinkTags(); + return; + } + const hasCloseTag = this.fullResponse.includes(""); const hasOpenTag = this.fullResponse.includes(""); - // Skip if there's no tag yet - if (!hasCloseTag) { - return; - } - - // If excludeThinking is true, strip all think content - if (this.excludeThinking) { - // Handle case: content with but no (malformed) - // Strip everything before and including - if (!hasOpenTag) { - const closeTagIndex = this.fullResponse.indexOf(""); - this.fullResponse = this.fullResponse.substring(closeTagIndex + "".length).trim(); - } else { - // Handle case: proper ... tags - // Strip all think blocks - this.fullResponse = this.fullResponse.replace(/[\s\S]*?<\/think>/g, "").trim(); - } - return; - } + if (!hasCloseTag) return; // excludeThinking is false - fix missing opening tag if needed if (!hasOpenTag && !this.hasHandledTextLevelThinkTag) { @@ -77,6 +71,55 @@ export class ThinkBlockStreamer { } } + /** + * Strip text-level think tags when excludeThinking is true. + * Handles three streaming states: + * 1. Not inside a think block -- look for `` to enter one + * 2. Inside an incomplete block (no `` yet) -- truncate to pre-block content + * 3. Block just closed -- strip the complete block and exit the state + */ + private handleExcludedThinkTags() { + // Currently inside an excluded block from a previous chunk + if (this.excludedThinkBlockStart >= 0) { + const closeIdx = this.fullResponse.indexOf("", this.excludedThinkBlockStart); + if (closeIdx !== -1) { + // Block closed -- stitch content before and after the block + const before = this.fullResponse.substring(0, this.excludedThinkBlockStart); + const after = this.fullResponse.substring(closeIdx + "".length); + this.fullResponse = (before + after).trimStart(); + this.excludedThinkBlockStart = -1; + } else { + // Still streaming thinking -- truncate to the safe prefix + this.fullResponse = this.fullResponse.substring(0, this.excludedThinkBlockStart); + } + return; + } + + // Not inside a block -- check for a new one + const openIdx = this.fullResponse.indexOf(""); + if (openIdx !== -1) { + this.excludedThinkBlockStart = openIdx; + const closeIdx = this.fullResponse.indexOf("", openIdx); + if (closeIdx !== -1) { + // Complete block in one pass + const before = this.fullResponse.substring(0, openIdx); + const after = this.fullResponse.substring(closeIdx + "".length); + this.fullResponse = (before + after).trimStart(); + this.excludedThinkBlockStart = -1; + } else { + // Incomplete -- truncate + this.fullResponse = this.fullResponse.substring(0, openIdx); + } + return; + } + + // Handle malformed: only without (e.g., some chat templates) + const closeIdx = this.fullResponse.indexOf(""); + if (closeIdx !== -1) { + this.fullResponse = this.fullResponse.substring(closeIdx + "".length).trimStart(); + } + } + private handleClaudeChunk(content: any[]) { let textContent = ""; let hasThinkingContent = false; diff --git a/src/LLMProviders/chainRunner/utils/citationUtils.test.ts b/src/LLMProviders/chainRunner/utils/citationUtils.test.ts index 8f23971a..5f619673 100644 --- a/src/LLMProviders/chainRunner/utils/citationUtils.test.ts +++ b/src/LLMProviders/chainRunner/utils/citationUtils.test.ts @@ -1,5 +1,6 @@ import { addFallbackSources, + deduplicateAdjacentCitations, formatSourceCatalog, getLocalSearchGuidance, getQACitationInstructions, @@ -8,6 +9,7 @@ import { normalizeCitations, processInlineCitations, sanitizeContentForCitations, + updateCitationsForConsolidation, type SourceCatalogEntry, } from "./citationUtils"; @@ -465,7 +467,8 @@ More content // Verify citations in text point to correct consolidated sources expect(result).toContain("references [1] and also [2]"); // [^14]->[1], [^17]->[2] expect(result).toContain("uses [3] again"); // [^3]->[3] - expect(result).toContain("cites [1] and [1]"); // [^14]->[1], [^22]->[1] (consolidated) + expect(result).toContain("cites [1]"); // [^14]->[1], [^22]->[1] (consolidated + deduplicated) + expect(result).not.toContain("cites [1] and [1]"); // adjacent dupes should be collapsed expect(result).toContain("mentions [3] once more"); // [^3]->[3] }); @@ -497,5 +500,113 @@ More content expect(result).not.toContain("[^8]"); // Should not contain any unconverted citations expect(result).not.toContain("[^7]"); // Should not contain any unconverted citations }); + + it("should deduplicate adjacent citations after consolidation", () => { + // Two chunks from the same note cited adjacently: [^1][^2] both -> [[Same Note]] + const content = `Key finding here [^1][^2] and another point [^3]. + +#### Sources: +[^1]: [[Same Note]] +[^2]: [[Same Note]] +[^3]: [[Other Note]]`; + + const result = processInlineCitations(content); + + // After consolidation, [^1] and [^2] both map to source 1 + // Adjacent [1][1] should be collapsed to [1] + expect(result).toContain("finding here [1] and another"); + expect(result).not.toContain("[1][1]"); + expect(result).toContain( + '[1][[Same Note]]' + ); + expect(result).toContain( + '[2][[Other Note]]' + ); + }); + + it("should deduplicate citations separated by 'and' after consolidation", () => { + // Use separate brackets since buildCitationMap scans [^N] individually + const content = `Claim here [^1] and [^2] and more [^3]. + +#### Sources: +[^1]: [[Note A]] +[^2]: [[Note A]] +[^3]: [[Note B]]`; + + const result = processInlineCitations(content); + + // [^1] and [^2] both map to [[Note A]] -> consolidated to source 1 + // "[1] and [1]" should collapse to "[1]" + expect(result).toContain("Claim here [1] and more"); + expect(result).not.toContain("[1] and [1]"); + }); + }); + + describe("updateCitationsForConsolidation", () => { + it("should deduplicate numbers within a bracket group", () => { + const map = new Map([ + [1, 1], + [2, 1], + [3, 2], + ]); + expect(updateCitationsForConsolidation("[1, 2] text [3]", map)).toBe("[1] text [2]"); + }); + + it("should preserve order of first occurrence", () => { + const map = new Map([ + [1, 1], + [2, 1], + [3, 1], + ]); + expect(updateCitationsForConsolidation("[1, 2, 3]", map)).toBe("[1]"); + }); + }); + + describe("deduplicateAdjacentCitations", () => { + it("should collapse identical adjacent brackets", () => { + expect(deduplicateAdjacentCitations("[1][1]")).toBe("[1]"); + }); + + it("should collapse triple identical adjacent brackets", () => { + expect(deduplicateAdjacentCitations("[1][1][1]")).toBe("[1]"); + }); + + it("should not collapse different adjacent brackets", () => { + expect(deduplicateAdjacentCitations("[1][2]")).toBe("[1][2]"); + }); + + it("should collapse when second is a subset of first", () => { + expect(deduplicateAdjacentCitations("[1, 2][1]")).toBe("[1, 2]"); + }); + + it("should not collapse when second has new numbers", () => { + expect(deduplicateAdjacentCitations("[1][1, 2]")).toBe("[1][1, 2]"); + }); + + it("should handle mixed cases in sequence", () => { + expect(deduplicateAdjacentCitations("[1][1][2]")).toBe("[1][2]"); + }); + + it("should handle spaces between brackets", () => { + expect(deduplicateAdjacentCitations("[1] [1]")).toBe("[1]"); + }); + + it("should preserve surrounding text", () => { + expect(deduplicateAdjacentCitations("claim [1][1] and more [2]")).toBe( + "claim [1] and more [2]" + ); + }); + + it("should collapse duplicates separated by 'and'", () => { + expect(deduplicateAdjacentCitations("cites [1] and [1]")).toBe("cites [1]"); + }); + + it("should not collapse different citations separated by 'and'", () => { + expect(deduplicateAdjacentCitations("cites [1] and [2]")).toBe("cites [1] and [2]"); + }); + + it("should collapse duplicates separated by comma", () => { + expect(deduplicateAdjacentCitations("cites [1], [1]")).toBe("cites [1]"); + }); }); }); diff --git a/src/LLMProviders/chainRunner/utils/citationUtils.ts b/src/LLMProviders/chainRunner/utils/citationUtils.ts index 88ebc446..55861e27 100644 --- a/src/LLMProviders/chainRunner/utils/citationUtils.ts +++ b/src/LLMProviders/chainRunner/utils/citationUtils.ts @@ -400,6 +400,7 @@ export function consolidateDuplicateSources(items: string[]): { /** * Updates citations in content to reflect consolidated numbering. + * Deduplicates within each bracket group after remapping. */ export function updateCitationsForConsolidation( content: string, @@ -409,14 +410,46 @@ export function updateCitationsForConsolidation( return content.replace(/\[(\d+(?:\s*,\s*\d+)*)\]/g, (_match, nums) => { const parts = nums.split(/\s*,\s*/); - const remappedParts = parts.map((n: string) => { - const oldNum = parseInt(n, 10); - return String(consolidationMap.get(oldNum) || oldNum); - }); - return `[${remappedParts.join(", ")}]`; + const seen = new Set(); + const unique: number[] = []; + for (const n of parts) { + const remapped = consolidationMap.get(parseInt(n, 10)) || parseInt(n, 10); + if (!seen.has(remapped)) { + seen.add(remapped); + unique.push(remapped); + } + } + return `[${unique.join(", ")}]`; }); } +/** + * Collapses duplicate citation brackets that appear next to each other. + * Handles truly adjacent brackets ([1][1]) and brackets separated by + * connectors like " and " or ", " ([1] and [1]). + * Only collapses when the second bracket is a subset of the first. + */ +export function deduplicateAdjacentCitations(content: string): string { + let result = content; + let prev; + do { + prev = result; + // Match citation brackets separated by optional whitespace or connectors (" and ", ", ") + result = result.replace( + /\[(\d+(?:\s*,\s*\d+)*)\](?:\s*(?:and|,)\s*|\s*)\[(\d+(?:\s*,\s*\d+)*)\]/g, + (match, first, second) => { + const firstNums = new Set(first.split(/\s*,\s*/).map((s: string) => s.trim())); + const secondNums = second.split(/\s*,\s*/).map((s: string) => s.trim()); + if (secondNums.every((n: string) => firstNums.has(n))) { + return `[${[...firstNums].join(", ")}]`; + } + return match; + } + ); + } while (result !== prev); + return result; +} + interface SourcesDisplayItem { index: number; html: string; @@ -538,9 +571,10 @@ export function processInlineCitations( let items = convertFootnoteDefinitions(sourcesBlock, citationMap); const { uniqueItems, consolidationMap } = consolidateDuplicateSources(items); - // Update citations to reflect consolidation + // Update citations to reflect consolidation and deduplicate if (consolidationMap.size > 0) { mainContent = updateCitationsForConsolidation(mainContent, consolidationMap); + mainContent = deduplicateAdjacentCitations(mainContent); items = uniqueItems; } diff --git a/src/LLMProviders/chainRunner/utils/queryDeduplication.test.ts b/src/LLMProviders/chainRunner/utils/queryDeduplication.test.ts new file mode 100644 index 00000000..5ec6a32b --- /dev/null +++ b/src/LLMProviders/chainRunner/utils/queryDeduplication.test.ts @@ -0,0 +1,126 @@ +import { computeWordOverlap, findDuplicateQuery, stripLeakedRoleLines } from "./queryDeduplication"; + +describe("computeWordOverlap", () => { + it("returns 1 for identical strings", () => { + expect(computeWordOverlap("hello world", "hello world")).toBe(1); + }); + + it("returns 1 for both empty strings", () => { + expect(computeWordOverlap("", "")).toBe(1); + }); + + it("returns 0 when one string is empty", () => { + expect(computeWordOverlap("hello", "")).toBe(0); + expect(computeWordOverlap("", "hello")).toBe(0); + }); + + it("returns 0 for completely disjoint strings", () => { + expect(computeWordOverlap("hello world", "foo bar")).toBe(0); + }); + + it("is case insensitive", () => { + expect(computeWordOverlap("Hello World", "hello world")).toBe(1); + }); + + it("computes correct Jaccard similarity for partial overlap", () => { + // "paul graham mistakes" vs "paul graham errors" + // intersection: {paul, graham} = 2, union: {paul, graham, mistakes, errors} = 4 + expect(computeWordOverlap("paul graham mistakes", "paul graham errors")).toBe(0.5); + }); + + it("catches inflection variants with sufficient shared context", () => { + // "Paul Graham mistake founders" vs "Paul Graham mistakes founders" + // intersection: {paul, graham, founders} = 3, union: {paul, graham, mistake, founders, mistakes} = 5 + const overlap = computeWordOverlap( + "Paul Graham mistake founders", + "Paul Graham mistakes founders" + ); + expect(overlap).toBe(0.6); + }); + + it("handles duplicate words in input", () => { + // Sets deduplicate, so "hello hello" -> {"hello"} + expect(computeWordOverlap("hello hello", "hello")).toBe(1); + }); +}); + +describe("findDuplicateQuery", () => { + it("returns null for empty previous queries", () => { + expect(findDuplicateQuery("hello world", [])).toBeNull(); + }); + + it("finds exact duplicate", () => { + expect(findDuplicateQuery("hello world", ["hello world"])).toBe("hello world"); + }); + + it("finds near-duplicate above threshold", () => { + const previous = ["Paul Graham mistake founders"]; + const result = findDuplicateQuery("Paul Graham mistakes founders", previous); + expect(result).toBe("Paul Graham mistake founders"); + }); + + it("returns null when below threshold", () => { + const previous = ["completely different query about cooking"]; + expect(findDuplicateQuery("Paul Graham mistakes founders", previous)).toBeNull(); + }); + + it("returns first match when multiple duplicates exist", () => { + const previous = ["search query alpha beta", "search query alpha gamma"]; + const result = findDuplicateQuery("search query alpha beta", previous); + expect(result).toBe("search query alpha beta"); + }); + + it("respects custom threshold", () => { + // overlap = 0.5, default threshold catches it + const previous = ["paul graham mistakes"]; + expect(findDuplicateQuery("paul graham errors", previous, 0.5)).toBe("paul graham mistakes"); + // Higher threshold misses it + expect(findDuplicateQuery("paul graham errors", previous, 0.6)).toBeNull(); + }); +}); + +describe("stripLeakedRoleLines", () => { + it("strips bare 'user' lines", () => { + // "user\n\nHello" splits to ["user", "", "Hello"] -> filter removes "user" -> "\nHello" + expect(stripLeakedRoleLines("user\n\nHello")).toBe("\nHello"); + }); + + it("strips bare 'assistant' lines", () => { + expect(stripLeakedRoleLines("assistant\nI will help")).toBe("I will help"); + }); + + it("strips bare 'system' lines", () => { + expect(stripLeakedRoleLines("system\nYou are helpful")).toBe("You are helpful"); + }); + + it("strips multiple role lines from real model output", () => { + // Real case: "user\n\nuser\n Paul Graham" -> removes both "user" lines + expect(stripLeakedRoleLines("user\n\nuser\n Paul Graham")).toBe("\n Paul Graham"); + }); + + it("preserves 'user' when part of longer text", () => { + expect(stripLeakedRoleLines("The user asked about this")).toBe("The user asked about this"); + }); + + it("preserves 'assistant' in normal sentences", () => { + expect(stripLeakedRoleLines("My assistant helped me")).toBe("My assistant helped me"); + }); + + it("is case sensitive (only strips lowercase)", () => { + expect(stripLeakedRoleLines("User\nHello")).toBe("User\nHello"); + }); + + it("handles empty and null-ish input", () => { + expect(stripLeakedRoleLines("")).toBe(""); + expect(stripLeakedRoleLines(" ")).toBe(" "); + }); + + it("preserves indented role words (code safety)", () => { + // Indented "system" or "user" in code should NOT be stripped + expect(stripLeakedRoleLines(" user \nHello")).toBe(" user \nHello"); + }); + + it("strips role word with trailing whitespace at column 0", () => { + expect(stripLeakedRoleLines("user \nHello")).toBe("Hello"); + }); +}); diff --git a/src/LLMProviders/chainRunner/utils/queryDeduplication.ts b/src/LLMProviders/chainRunner/utils/queryDeduplication.ts new file mode 100644 index 00000000..dc41a0c3 --- /dev/null +++ b/src/LLMProviders/chainRunner/utils/queryDeduplication.ts @@ -0,0 +1,74 @@ +/** + * Utilities for the ReAct agent loop with small local models. + * - Query deduplication to prevent near-identical repeated searches + * - Leaked role token cleanup for models that leak chat template fragments + */ + +/** + * Compute Jaccard similarity between two strings based on lowercased word sets. + * + * @param a - First string + * @param b - Second string + * @returns Jaccard similarity coefficient (0-1) + */ +export function computeWordOverlap(a: string, b: string): number { + const wordsA = new Set(a.toLowerCase().split(/\s+/).filter(Boolean)); + const wordsB = new Set(b.toLowerCase().split(/\s+/).filter(Boolean)); + + if (wordsA.size === 0 && wordsB.size === 0) return 1; + if (wordsA.size === 0 || wordsB.size === 0) return 0; + + let intersection = 0; + for (const word of wordsA) { + if (wordsB.has(word)) intersection++; + } + + const union = wordsA.size + wordsB.size - intersection; + return intersection / union; +} + +/** + * Check if a query is a near-duplicate of any previously issued query. + * + * @param query - The new query to check + * @param previousQueries - List of previously issued queries + * @param threshold - Jaccard similarity threshold above which queries are considered duplicates (default 0.5) + * @returns The first matching previous query if found, or null + */ +export function findDuplicateQuery( + query: string, + previousQueries: string[], + threshold = 0.5 +): string | null { + for (const prev of previousQueries) { + if (computeWordOverlap(query, prev) >= threshold) { + return prev; + } + } + return null; +} + +/** + * Regex matching lines that contain ONLY a leaked chat template role identifier + * starting at column 0. After stripSpecialTokens removes markers like `<|im_start|>`, + * the bare role name (e.g. "user", "assistant") can remain. These pollute conversation + * history and confuse small models on subsequent iterations. + * No leading \s* -- preserves indented code lines like " system". + */ +const LEAKED_ROLE_LINE = /^(user|assistant|system)\s*$/; + +/** + * Strip lines that are only leaked chat template role identifiers. + * Applied to intermediate model output (when tool calls are present) to prevent + * leaked tokens like "user" from polluting the conversation history. + * + * @param text - Model output text after special token stripping + * @returns Cleaned text with role-only lines removed + */ +export function stripLeakedRoleLines(text: string): string { + if (!text) return text; + return text + .split("\n") + .filter((line) => !LEAKED_ROLE_LINE.test(line)) + .join("\n"); +} diff --git a/src/components/CopilotView.tsx b/src/components/CopilotView.tsx index 49a06e4a..184bf276 100644 --- a/src/components/CopilotView.tsx +++ b/src/components/CopilotView.tsx @@ -1,5 +1,6 @@ import ChainManager from "@/LLMProviders/chainManager"; import Chat from "@/components/Chat"; +import { ChatViewLayout } from "@/components/chat-components/ChatViewLayout"; import { CHAT_VIEWTYPE } from "@/constants"; import { AppContext, EventTargetContext } from "@/context"; import CopilotPlugin from "@/main"; @@ -19,6 +20,7 @@ export default class CopilotView extends ItemView { private handleSaveAsNote: (() => Promise) | null = null; private keyboardObserver: MutationObserver | null = null; private drawerHideObserver: MutationObserver | null = null; + private layout: ChatViewLayout | null = null; private lastDrawerEl: HTMLElement | null = null; eventTarget: EventTarget; @@ -61,6 +63,7 @@ export default class CopilotView extends ItemView { }; this.renderView(handleSaveAsNote, updateUserMessageHistory); + this.layout = new ChatViewLayout(this.containerEl, this.app.workspace); this.setupMobileKeyboardObserver(); this.setupDrawerHideObserver(); @@ -206,6 +209,8 @@ export default class CopilotView extends ItemView { this.keyboardObserver = null; this.drawerHideObserver?.disconnect(); this.drawerHideObserver = null; + this.layout?.destroy(); + this.layout = null; // Reason: Clean up the class on the tracked drawer element when the view is closed. // Use lastDrawerEl instead of querying closest(), because the view may have already // been detached from the drawer DOM by the time onClose fires. diff --git a/src/components/chat-components/ChatSingleMessage.tsx b/src/components/chat-components/ChatSingleMessage.tsx index fda2849f..cbde7bd6 100644 --- a/src/components/chat-components/ChatSingleMessage.tsx +++ b/src/components/chat-components/ChatSingleMessage.tsx @@ -83,6 +83,111 @@ export const normalizeFootnoteRendering = (root: HTMLElement): void => { }); }; +const INLINE_CITATION_RE = /\[(\d+(?:\s*,\s*\d+)*)\]/g; + +/** + * Makes inline citation numbers (e.g., [1], [2]) clickable by linking them + * to the corresponding source note. Reads the source mapping from the + * rendered .copilot-sources section in the same message. + */ +export const linkInlineCitations = (root: HTMLElement): void => { + // Build citation number -> source anchor mapping from the rendered sources section. + // We store the anchor element (not just the href) so we can copy Obsidian-specific + // attributes like data-href and class="internal-link" onto the inline citation link. + const sourceItems = root.querySelectorAll(".copilot-sources__item"); + if (sourceItems.length === 0) return; + + const citationAnchors = new Map(); + sourceItems.forEach((item) => { + const indexEl = item.querySelector(".copilot-sources__index"); + const textEl = item.querySelector(".copilot-sources__text"); + if (!indexEl || !textEl) return; + + const indexMatch = indexEl.textContent?.match(/\[(\d+)\]/); + if (!indexMatch) return; + const num = parseInt(indexMatch[1], 10); + + const link = textEl.querySelector("a"); + if (link) { + citationAnchors.set(num, link); + } + }); + + if (citationAnchors.size === 0) return; + + // Collect text nodes that contain citation patterns (outside sources section) + const sourcesEl = root.querySelector(".copilot-sources"); + const textNodes: Text[] = []; + const walker = document.createTreeWalker(root, NodeFilter.SHOW_TEXT, { + acceptNode(node) { + if (sourcesEl?.contains(node)) return NodeFilter.FILTER_REJECT; + if (node.parentElement?.closest("code, pre")) return NodeFilter.FILTER_REJECT; + INLINE_CITATION_RE.lastIndex = 0; + if (INLINE_CITATION_RE.test(node.textContent || "")) { + INLINE_CITATION_RE.lastIndex = 0; + return NodeFilter.FILTER_ACCEPT; + } + return NodeFilter.FILTER_REJECT; + }, + }); + + let n: Text | null; + while ((n = walker.nextNode() as Text | null)) textNodes.push(n); + + // Replace citation text with clickable links + textNodes.forEach((node) => { + const text = node.textContent || ""; + INLINE_CITATION_RE.lastIndex = 0; + + const fragment = document.createDocumentFragment(); + let lastIndex = 0; + let match: RegExpExecArray | null; + + while ((match = INLINE_CITATION_RE.exec(text)) !== null) { + // Text before the citation + if (match.index > lastIndex) { + fragment.appendChild(document.createTextNode(text.slice(lastIndex, match.index))); + } + + const nums = match[1].split(/\s*,\s*/).map((s) => parseInt(s.trim(), 10)); + const allResolved = nums.every((num) => citationAnchors.has(num)); + + if (allResolved) { + const span = document.createElement("span"); + span.className = "copilot-citation-group"; + span.appendChild(document.createTextNode("[")); + nums.forEach((num, i) => { + if (i > 0) span.appendChild(document.createTextNode(", ")); + const sourceAnchor = citationAnchors.get(num)!; + const link = document.createElement("a"); + // Copy all attributes from the source anchor so Obsidian internal-link + // metadata (e.g. data-href, class="internal-link") is preserved. + for (const attr of Array.from(sourceAnchor.attributes)) { + link.setAttribute(attr.name, attr.value); + } + // Override class and add our citation-specific styling + link.className = `copilot-citation-link${sourceAnchor.className ? ` ${sourceAnchor.className}` : ""}`; + link.textContent = String(num); + link.setAttribute("aria-label", `Source ${num}`); + span.appendChild(link); + }); + span.appendChild(document.createTextNode("]")); + fragment.appendChild(span); + } else { + fragment.appendChild(document.createTextNode(match[0])); + } + + lastIndex = match.index + match[0].length; + } + + if (lastIndex < text.length) { + fragment.appendChild(document.createTextNode(text.slice(lastIndex))); + } + + node.parentNode?.replaceChild(fragment, node); + }); +}; + function MessageContext({ context }: { context: ChatMessage["context"] }) { if ( !context || @@ -572,6 +677,7 @@ const ChatSingleMessage: React.FC = ({ MarkdownRenderer.renderMarkdown(segment.content, textDiv, "", componentRef.current!); normalizeFootnoteRendering(textDiv); + linkInlineCitations(textDiv); currentIndex++; } else if (segment.type === "toolCall" && segment.toolCall) { const toolCallId = segment.toolCall.id; diff --git a/src/components/chat-components/ChatViewLayout.ts b/src/components/chat-components/ChatViewLayout.ts new file mode 100644 index 00000000..2ae6f10e --- /dev/null +++ b/src/components/chat-components/ChatViewLayout.ts @@ -0,0 +1,83 @@ +import { Platform, Workspace } from "obsidian"; + +/** Wait for CSS transitions to settle before re-measuring after a theme switch. */ +const CSS_CHANGE_DEBOUNCE_MS = 600; + +/** + * Manages layout concerns for the Copilot chat view, such as status bar + * clearance and (in the future) chat input collapse state. + * + * Instantiated once per CopilotView and tied to its lifecycle. + */ +export class ChatViewLayout { + private debounceTimer: ReturnType | null = null; + private cssChangeRef: ReturnType | null = null; + + constructor( + private containerEl: HTMLElement, + private workspace: Workspace + ) { + this.setupStatusBarClearance(); + } + + /** + * Tear down observers and timers. Call from CopilotView.onClose(). + */ + destroy(): void { + if (this.debounceTimer) { + clearTimeout(this.debounceTimer); + this.debounceTimer = null; + } + if (this.cssChangeRef) { + this.workspace.offref(this.cssChangeRef); + this.cssChangeRef = null; + } + } + + /** + * Measure how much the status bar overlaps the view-content and expose + * the overlap as a CSS variable so padding adapts to any theme. + * + * Works by temporarily zeroing the clearance, measuring the geometric + * overlap, then setting the correct value -- all within one synchronous + * reflow so nothing flickers. Themes that already position content above + * the status bar (no overlap) get 0 clearance automatically. Auto-hide + * themes (opacity: 0) also get 0 since the bar is transparent. + */ + private setupStatusBarClearance(): void { + if (Platform.isMobile) return; + + const syncClearance = () => { + // Re-query each time to avoid stale references after theme reloads. + const statusBar = document.querySelector(".status-bar") as HTMLElement | null; + const viewContent = this.containerEl.querySelector(".view-content") as HTMLElement | null; + if (!statusBar || !viewContent) return; + + // Zero out clearance and force reflow to measure natural overlap. + viewContent.style.setProperty("--copilot-status-bar-clearance", "0px"); + const overlap = + viewContent.getBoundingClientRect().bottom - statusBar.getBoundingClientRect().top; + + if (overlap <= 0) { + // Theme layout already clears the status bar. + return; + } + + // Overlap exists -- only add clearance if the bar is actually visible. + const s = getComputedStyle(statusBar); + const hidden = + s.display === "none" || s.visibility === "hidden" || parseFloat(s.opacity) === 0; + viewContent.style.setProperty( + "--copilot-status-bar-clearance", + `${hidden ? 0 : Math.ceil(overlap)}px` + ); + }; + + syncClearance(); + + this.cssChangeRef = this.workspace.on("css-change", () => { + if (this.debounceTimer) clearTimeout(this.debounceTimer); + this.debounceTimer = setTimeout(syncClearance, CSS_CHANGE_DEBOUNCE_MS); + }); + } +} diff --git a/src/components/ui/ModelParametersEditor.tsx b/src/components/ui/ModelParametersEditor.tsx index 26573df4..b934ec2e 100644 --- a/src/components/ui/ModelParametersEditor.tsx +++ b/src/components/ui/ModelParametersEditor.tsx @@ -217,7 +217,7 @@ export function ModelParametersEditor({ (opt) => opt.value !== ReasoningEffort.MINIMAL && opt.value !== ReasoningEffort.XHIGH ), - ...(model.name.startsWith("gpt-5.2") && model.provider === "openai" + ...(model.name.startsWith("gpt-5.4") && model.provider === "openai" ? [{ value: ReasoningEffort.XHIGH, label: "Extra High" }] : []), ]} @@ -234,8 +234,8 @@ export function ModelParametersEditor({
  • Low: Faster responses, basic reasoning (default)
  • Medium: Balanced performance
  • High: Thorough reasoning, slower responses
    • - {model.name.startsWith("gpt-5.2") && model.provider === "openai" && ( -
  • Extra High: Maximum reasoning depth (GPT-5.2 only)
    • + {model.name.startsWith("gpt-5.4") && model.provider === "openai" && ( +
  • Extra High: Maximum reasoning depth (GPT-5.4 only)
    • )} {!hasReasoningCapability && !isOpenAIReasoningModel && ( diff --git a/src/constants.ts b/src/constants.ts index e9da9fe9..f3a1df6b 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -173,7 +173,7 @@ export const DEFAULT_OLLAMA_NUM_CTX = 131072; export enum ChatModels { COPILOT_PLUS_FLASH = "copilot-plus-flash", - GPT_5_2 = "gpt-5.2", + GPT_5_4 = "gpt-5.4", GPT_5_mini = "gpt-5-mini", GPT_5_nano = "gpt-5-nano", GPT_41 = "gpt-4.1", @@ -197,7 +197,7 @@ export enum ChatModels { OPENROUTER_GEMINI_3_PRO_PREVIEW = "google/gemini-3.1-pro-preview", OPENROUTER_GEMINI_2_5_FLASH = "google/gemini-2.5-flash", OPENROUTER_GEMINI_2_5_PRO = "google/gemini-2.5-pro", - OPENROUTER_GPT_5_2 = "openai/gpt-5.2", + OPENROUTER_GPT_5_4 = "openai/gpt-5.4", OPENROUTER_GPT_5_MINI = "openai/gpt-5-mini", OPENROUTER_GROK_4_1_FAST = "x-ai/grok-4.1-fast", SILICONFLOW_DEEPSEEK_V3 = "deepseek-ai/DeepSeek-V3", @@ -281,7 +281,7 @@ export const BUILTIN_CHAT_MODELS: CustomModel[] = [ capabilities: [ModelCapability.VISION], }, { - name: ChatModels.OPENROUTER_GPT_5_2, + name: ChatModels.OPENROUTER_GPT_5_4, provider: ChatModelProviders.OPENROUTERAI, enabled: true, isBuiltIn: true, @@ -317,7 +317,7 @@ export const BUILTIN_CHAT_MODELS: CustomModel[] = [ capabilities: [ModelCapability.VISION], }, { - name: ChatModels.GPT_5_2, + name: ChatModels.GPT_5_4, provider: ChatModelProviders.OPENAI, enabled: true, isBuiltIn: true, @@ -613,7 +613,7 @@ export const ProviderInfo: Record = { curlBaseURL: "https://api.openai.com/v1", keyManagementURL: "https://platform.openai.com/api-keys", listModelURL: "https://api.openai.com/v1/models", - testModel: ChatModels.GPT_5_2, + testModel: ChatModels.GPT_5_4, }, [ChatModelProviders.XAI]: { label: "XAI", diff --git a/src/settings/v2/components/QASettings.tsx b/src/settings/v2/components/QASettings.tsx index 7a2b4833..99c43fd8 100644 --- a/src/settings/v2/components/QASettings.tsx +++ b/src/settings/v2/components/QASettings.tsx @@ -188,6 +188,18 @@ export const QASettings: React.FC = () => { placeholder="Strategy" /> + {/* Max Sources */} + updateSetting("maxSourceChunks", value)} + /> + {/* Embedding-related settings - Only shown when semantic search is enabled */} {settings.enableSemanticSearchV3 && ( <> diff --git a/src/styles/tailwind.css b/src/styles/tailwind.css index 88ad6e16..9937144a 100644 --- a/src/styles/tailwind.css +++ b/src/styles/tailwind.css @@ -897,11 +897,12 @@ If your plugin does not need CSS, delete this file. } /* use 'important' to prevent ob default css style from being third-party theme overridden */ +/* --copilot-status-bar-clearance is set dynamically by ChatViewLayout */ .workspace-leaf-content[data-type="copilot-chat-view"] .view-content { - padding-bottom: max( - calc(var(--safe-area-inset-bottom, 0px) + var(--size-4-1)), - var(--size-4-8) + padding-bottom: calc( + var(--safe-area-inset-bottom, 0px) + var(--copilot-status-bar-clearance, 0px) + + var(--size-4-1) ) !important; } @@ -1039,6 +1040,16 @@ body.is-mobile .workspace-drawer.copilot-keyboard-open .workspace-drawer-tab-opt color: var(--text-accent); } +/* Inline citation links */ +.copilot-citation-link { + color: var(--text-accent); + cursor: pointer; + text-decoration: none; +} +.copilot-citation-link:hover { + text-decoration: underline; +} + /* fix the width of the popover on mobile */ body.is-mobile div[data-radix-popper-content-wrapper] { min-width: fit-content !important; diff --git a/src/tools/SearchTools.ts b/src/tools/SearchTools.ts index 5684770d..4c9db140 100644 --- a/src/tools/SearchTools.ts +++ b/src/tools/SearchTools.ts @@ -1,5 +1,5 @@ import { getStandaloneQuestion } from "@/chainUtils"; -import { DEFAULT_MAX_SOURCE_CHUNKS, TEXT_WEIGHT } from "@/constants"; +import { TEXT_WEIGHT } from "@/constants"; import { BrevilabsClient } from "@/LLMProviders/brevilabsClient"; import { hasSelfHostSearchKey, selfHostWebSearch } from "@/LLMProviders/selfHostServices"; import { logInfo } from "@/logger"; @@ -100,10 +100,11 @@ async function performLexicalSearch({ forceLexical?: boolean; preExpandedQuery?: QueryExpansionInfo; }) { + const settings = getSettings(); // Extract tag terms for self-host retriever (server-side tag filtering) const tagTerms = salientTerms.filter((term) => term.startsWith("#")); - const effectiveMaxK = DEFAULT_MAX_SOURCE_CHUNKS; + const effectiveMaxK = settings.maxSourceChunks; logInfo(`lexicalSearch effectiveMaxK: ${effectiveMaxK}, forceLexical: ${forceLexical}`); @@ -254,8 +255,9 @@ const semanticSearchTool = createLangChainTool({ schema: localSearchSchema, func: async ({ timeRange: rawTimeRange, query, salientTerms }) => { const timeRange = validateTimeRange(rawTimeRange); + const settings = getSettings(); - const effectiveMaxK = DEFAULT_MAX_SOURCE_CHUNKS; + const effectiveMaxK = settings.maxSourceChunks; logInfo(`semanticSearch effectiveMaxK: ${effectiveMaxK}`); @@ -362,7 +364,7 @@ async function performMiyoSearch({ timeRange?: { startTime: number; endTime: number }; }) { const tagTerms = salientTerms.filter((term) => term.startsWith("#")); - const effectiveMaxK = DEFAULT_MAX_SOURCE_CHUNKS; + const effectiveMaxK = getSettings().maxSourceChunks; // FilterRetriever for local tag/title/time-range matches const filterRetriever = new FilterRetriever(app, {