mirror of
https://github.com/logancyang/obsidian-copilot.git
synced 2026-07-22 07:50:24 +00:00
fix(plus): planner skips getFileTree when active note is attached (#2457)
* fix(plus): planner skips getFileTree when active note is attached (fixes #2456) In Plus mode, the lightweight tool-planning step in CopilotPlusChainRunner.planToolCalls() always runs regardless of the Agent toggle. Its candidate-tools list includes getFileTree, and the planning prompt guideline ("For file structure queries, use getFileTree to explore the vault") was vague enough that asking "what's this note about?" with a note already attached would trigger getFileTree before the model answered — wasteful and surprising. The planner had no awareness of attached context: it received only the bare user message string, so it couldn't tell the difference between "what's this note" (note attached → answer from context) and "what's this note" (no attachment → maybe search for it). Fix: 1. planToolCalls takes a new `hasActiveContextNote` parameter. 2. Caller derives the flag from the envelope's L3_TURN layer by testing for `<active_note` markers. 3. Planning prompt: - The existing getFileTree guideline tightened from "file structure queries" to "ONLY when the user wants to discover or list notes/ folders" so the planner doesn't read "note" as a file-structure cue when content is already in context. - When hasActiveContextNote is true, append an explicit hint telling the model not to call getFileTree just because the user says "this note" / "this file" — and that it should still call getFileTree when the user explicitly wants OTHER notes / folders / paths. This preserves the legitimate cases ("are there other notes similar to this", "create a new note in the same folder", "list folders in projects/") because those phrases override the context hint. Verified: - npm run lint: pre-existing warnings only - npm test: 116 suites / 2105 tests pass - npm run build: succeeds Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plus): scope active-note detection to current-turn segments Address Codex P2: regexing the entire L3_TURN text is too permissive because compacted envelopes merge prior-turn context into a single "compacted_context" segment. An <active_note> tag from an earlier turn would falsely set hasActiveContextNote=true and incorrectly suppress getFileTree. Inspect L3_TURN segments instead and only count those with metadata.source === "current_turn" (the marker emitted by parseContextIntoSegments for non-stable, this-turn parsing). Compacted segments use source: "compacted" and won't match, so the optimization falls back to the safe default in compacted chats. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c8a6ad9374
commit
efee65ca7a
1 changed files with 26 additions and 3 deletions
|
|
@ -109,7 +109,8 @@ export class CopilotPlusChainRunner extends BaseChainRunner {
|
|||
*/
|
||||
private async planToolCalls(
|
||||
userMessage: string,
|
||||
chatModel: BaseChatModel
|
||||
chatModel: BaseChatModel,
|
||||
hasActiveContextNote: boolean = false
|
||||
): Promise<{ toolCalls: ToolCallWithExecutor[]; salientTerms: string[] }> {
|
||||
const availableTools = this.getAvailableToolsForPlanning();
|
||||
|
||||
|
|
@ -129,12 +130,19 @@ export class CopilotPlusChainRunner extends BaseChainRunner {
|
|||
const boundModel = modelWithTools.bindTools(availableTools);
|
||||
|
||||
// Build a lightweight planning prompt (no XML format instructions needed)
|
||||
// Reason: when an active note is attached in this turn, tell the planner not
|
||||
// to call getFileTree just because the user says "this note" / "this file" —
|
||||
// the note's content is already available via context, so getFileTree is
|
||||
// wasteful unless the user explicitly wants to discover OTHER notes.
|
||||
const activeContextHint = hasActiveContextNote
|
||||
? "\n- The user has an active note attached in this turn. Its content is already available; do NOT call getFileTree merely because the user says 'this note' or 'this file'. Only call getFileTree when the user explicitly wants to discover OTHER notes, list folders, or verify paths."
|
||||
: "";
|
||||
const planningPrompt = `You are a helpful AI assistant. Analyze the user's message and determine if any tools should be called.
|
||||
|
||||
Guidelines:
|
||||
- Use tools when the user's request requires external information or computation
|
||||
- For time-related queries, use getTimeRangeMs to convert time expressions to timestamps
|
||||
- For file structure queries, use getFileTree to explore the vault
|
||||
- Use getFileTree ONLY when the user wants to discover or list notes/folders in the vault — not to read content already in context${activeContextHint}
|
||||
- If no tools are needed, respond with your analysis
|
||||
|
||||
After analyzing, extract key search terms from the user's message that would be useful for searching notes:
|
||||
|
|
@ -787,7 +795,22 @@ Include your extracted terms as: [SALIENT_TERMS: term1, term2, term3]`;
|
|||
try {
|
||||
// Use model-based planning instead of Broca
|
||||
const chatModel = this.chainManager.chatModelManager.getChatModel();
|
||||
const planningResult = await this.planToolCalls(messageForAnalysis, chatModel);
|
||||
// Reason: detect active-note attachment from the envelope so the planner
|
||||
// can avoid calling getFileTree just to read a note that's already in
|
||||
// context. See issue #2456. Scope to current-turn segments only: in
|
||||
// compacted envelopes L3_TURN merges prior-turn context into a single
|
||||
// "compacted_context" segment (metadata.source === "compacted"), so an
|
||||
// <active_note> tag from an earlier turn could otherwise falsely trigger
|
||||
// the optimization.
|
||||
const l3TurnForPlanning = envelope.layers.find((l) => l.id === "L3_TURN");
|
||||
const hasActiveContextNote = !!l3TurnForPlanning?.segments?.some(
|
||||
(seg) => seg.metadata?.source === "current_turn" && /<active_note[\s>]/.test(seg.content)
|
||||
);
|
||||
const planningResult = await this.planToolCalls(
|
||||
messageForAnalysis,
|
||||
chatModel,
|
||||
hasActiveContextNote
|
||||
);
|
||||
|
||||
// Execute getTimeRangeMs immediately if present (needed for localSearch timeRange)
|
||||
// We execute it once here and remove it from toolCalls to avoid double execution
|
||||
|
|
|
|||
Loading…
Reference in a new issue