From 7e71861a4d01eeaacc975134d6c404fa436adb89 Mon Sep 17 00:00:00 2001 From: Andrew Beal Date: Tue, 6 Jan 2026 20:49:02 +0000 Subject: [PATCH] refactor: improve plan execution with stricter scope and error handling - Add currentProvider getter to AI classes for provider access - Enhance planning prompt to discourage over-engineering and scope creep - Return Unknown enum instead of throwing for invalid AI functions - Add completion reminders to execution steps - Fix ChatPlanArea height calculation timing - Update tests to handle Unknown function gracefully --- AIClasses/BaseAIClass.ts | 4 ++ .../Functions/CreatePlan.ts | 2 +- AIClasses/IAIClass.ts | 2 + AIPrompts/PlanningAgentSystemPrompt.ts | 43 ++++++++++++++++++- Components/ChatPlanArea.svelte | 13 +++--- Enums/AIFunction.ts | 8 ++-- Enums/Copy.ts | 8 +++- Services/AIFunctionService.ts | 1 + Types/ExecutionPlan.ts | 8 ++-- __tests__/Services/AIFunctionService.test.ts | 18 +++++--- __tests__/Services/ChatService.test.ts | 8 ++-- 11 files changed, 88 insertions(+), 27 deletions(-) diff --git a/AIClasses/BaseAIClass.ts b/AIClasses/BaseAIClass.ts index 232044a..e6497f5 100644 --- a/AIClasses/BaseAIClass.ts +++ b/AIClasses/BaseAIClass.ts @@ -38,6 +38,10 @@ export abstract class BaseAIClass implements IAIClass { this.apiKey = this.settingsService.getApiKeyForProvider(provider); } + get currentProvider(): AIProvider { + return this.provider; + } + public set systemPrompt(systemPrompt: string) { this._systemPrompt = systemPrompt; } diff --git a/AIClasses/FunctionDefinitions/Functions/CreatePlan.ts b/AIClasses/FunctionDefinitions/Functions/CreatePlan.ts index a2885da..5590a9b 100644 --- a/AIClasses/FunctionDefinitions/Functions/CreatePlan.ts +++ b/AIClasses/FunctionDefinitions/Functions/CreatePlan.ts @@ -36,7 +36,7 @@ Do NOT use for simple, single-step operations that don't require planning.`, }, user_message: { type: "string", - description: "A short message to be displayed to the user explaining why you're requesting a plan. Examples: 'Creating a plan to organize your project notes', 'Developing a strategy to compile your research on AI topics'" + description: "A short message to be displayed to the user explaining why you're requesting a plan. This message should be very concise. Examples: 'Creating a plan to organize your project notes', 'Developing a strategy to compile your research on AI topics'" } }, required: ["goal", "user_message"] diff --git a/AIClasses/IAIClass.ts b/AIClasses/IAIClass.ts index 44a0f81..2801f8e 100644 --- a/AIClasses/IAIClass.ts +++ b/AIClasses/IAIClass.ts @@ -2,8 +2,10 @@ import type { IStreamChunk } from "Services/StreamingService"; import type { Conversation } from "Conversations/Conversation"; import type { Attachment } from "Conversations/Attachment"; import type { IAIFunctionDefinition } from "./FunctionDefinitions/IAIFunctionDefinition"; +import type { AIProvider } from "Enums/ApiProvider"; export interface IAIClass { + get currentProvider(): AIProvider; set systemPrompt(systemPrompt: string); set userInstruction(userInstruction: string); set toolDefinitions(toolDefinitions: IAIFunctionDefinition[]); diff --git a/AIPrompts/PlanningAgentSystemPrompt.ts b/AIPrompts/PlanningAgentSystemPrompt.ts index f693563..f5b3415 100644 --- a/AIPrompts/PlanningAgentSystemPrompt.ts +++ b/AIPrompts/PlanningAgentSystemPrompt.ts @@ -30,9 +30,9 @@ When you receive a planning request: - Identify which vault operations and tools will be needed - Consider dependencies between steps -### 2. Adaptive Planning Strategy +### 2. Scope Discipline & Plan Efficiency -**Scale your planning effort to match query complexity:** +#### 2.1. Scale your planning effort to match query complexity | Complexity | Exploration | Plan Detail | Example | |------------|-------------|-------------|---------| @@ -41,6 +41,45 @@ When you receive a planning request: | Complex | 6-10 searches | 8-15 steps | "Research and synthesize Z across vault" | | Advanced | 10+ searches | 15+ steps with sub-plans | "Comprehensive vault restructuring" | +- If you find yourself creating 10+ steps for a simple task, STOP and simplify + +#### 2.2. Do NOT Add Unnecessary Steps +❌ NEVER add steps for: +- Archiving/backing up unless explicitly requested +- Detailed documentation unless requested + +#### 2.3. Combine Related Operations +✅ GOOD: "Search for test.md and delete if found" +❌ BAD: "Step 1: Search for test.md" → "Step 2: Verify file exists" → "Step 3: Confirm with user (when redundant)" → "Step 4: Create backup (when not explicitly asked)" → "Step 5: Delete file" + +#### 2.4. Stay Within Scope +- Do NOT expand the task beyond what the user explicitly requested +- Do NOT add "nice to have" features or improvements +- If you identify optional enhancements, either note them separately or explicitly ask the user before including them in the main plan + +#### Example of Proper Scoping + +**User Request**: "Search my vault for a test note and delete it if it exists, then create a new test note with lorem ipsum" + +✅ **GOOD PLAN (2-3 steps)**: +1. Search vault for test.md and delete if found +2. Create new test.md with lorem ipsum content + +❌ **BAD PLAN (over-engineered)**: +1. Search vault for test note +2. Verify file existence and location +3. Request user confirmation for deletion +4. Create backup of existing file +5. Delete the test note +6. Verify deletion was successful +7. Create new test note file +8. Add lorem ipsum content +9. Verify file was created correctly +10. Document changes made +11. Consider next steps + +**Remember**: Users value efficiency. A concise, effective 2-step plan is superior to an exhaustive 10-step plan for simple tasks. + ### 3. Deep Contextual Analysis **Before creating any plan, you MUST conduct thorough exploratory work:** diff --git a/Components/ChatPlanArea.svelte b/Components/ChatPlanArea.svelte index 715f141..c688c16 100644 --- a/Components/ChatPlanArea.svelte +++ b/Components/ChatPlanArea.svelte @@ -50,13 +50,16 @@ } async function updateHeight() { - await tick(); - if (!contentDiv || stepElements.length === 0) return; + setTimeout(() => { + if (!contentDiv || stepElements.length === 0) { + return; + } - expandedHeight = contentDiv.scrollHeight; + expandedHeight = contentDiv.scrollHeight; - const stepsToShow = Math.min(3, stepElements.length); - collapsedHeight = (stepsToShow * stepHeight) + (Math.max(0, stepsToShow - 1) * separatorHeight); + const stepsToShow = Math.min(3, stepElements.length); + collapsedHeight = (stepsToShow * stepHeight) + (Math.max(0, stepsToShow - 1) * separatorHeight); + }, 1000); } async function scrollToActiveStep() { diff --git a/Enums/AIFunction.ts b/Enums/AIFunction.ts index 51ad6f6..2a0c0f2 100644 --- a/Enums/AIFunction.ts +++ b/Enums/AIFunction.ts @@ -1,5 +1,3 @@ -import { Exception } from "Helpers/Exception"; - export enum AIFunction { SearchVaultFiles = "search_vault_files", ReadVaultFiles = "read_vault_files", @@ -19,7 +17,9 @@ export enum AIFunction { CompleteStep = "complete_step", CompletePlan = "complete_plan", CancelPlan = "cancel_plan", - AskUserQuestion = "ask_user_question" + AskUserQuestion = "ask_user_question", + + Unknown = "unknown" } export function fromString(functionName: string): AIFunction { @@ -27,7 +27,7 @@ export function fromString(functionName: string): AIFunction { if (enumValue) { return enumValue as AIFunction; } - Exception.throw(`Unknown function name: ${functionName}`); + return AIFunction.Unknown; } export function isAIFunction(value: unknown, aiFunction: AIFunction): value is AIFunction { diff --git a/Enums/Copy.ts b/Enums/Copy.ts index e567ced..574627b 100644 --- a/Enums/Copy.ts +++ b/Enums/Copy.ts @@ -99,10 +99,16 @@ export enum Copy { ### Next Actions - Create your own simplified plan - Follow your own simplified plan`, + CompleteStepReminder = "Reminder: Mark each step of the plan as complete once it has been executed", StepDoesNotExistError = "Step {stepNumber} does not exist in the execution plan. Valid step numbers are 1-{totalSteps}.", StepMustBeCompletedInOrderError = "Cannot complete step {stepNumber}. The following steps have not yet been marked as completed: {incompleteSteps}. Steps must be completed in order.", StepCompletedWithNextStep = "Step {stepNumber} completed successfully. Now proceed with step {nextStepNumber}.", - AllStepsCompleted = "Step {stepNumber} completed successfully. All steps in the execution plan have been completed. Provide a final summary to the user based on the completed steps and overall success criteria.", + CompletionReminder = "After completing this step, you must mark it as complete to track progress. Failure to do so will cause the workflow to lose track of completion status.", + AllStepsCompleted = `Step {stepNumber} completed successfully. All steps in the execution plan have been completed. + +### NEXT REQUIRED ACTIONS +- Mark the entire plan as completed. +- Provide a final summary to the user based on the completed steps and overall success criteria.`, PlanExecutionCancelled = "Plan execution cancelled. Provide a summary to the user explaining what happened and any partial progress made.", PlanExecutionComplete = "Plan execution complete.", PlanningToolDenial = "Invalid tool call - this is an execution tool and cannot be called during the planning phase.", diff --git a/Services/AIFunctionService.ts b/Services/AIFunctionService.ts index e8de651..faa0c80 100644 --- a/Services/AIFunctionService.ts +++ b/Services/AIFunctionService.ts @@ -136,6 +136,7 @@ export class AIFunctionService { ); } + case AIFunction.Unknown: default: { const functionCallName = fromString(functionCall.name); const error = `Unknown function request ${functionCallName}` diff --git a/Types/ExecutionPlan.ts b/Types/ExecutionPlan.ts index ca7f5f8..c390f9c 100644 --- a/Types/ExecutionPlan.ts +++ b/Types/ExecutionPlan.ts @@ -75,9 +75,9 @@ export class ExecutionPlan { (stepNumber + 1).toString() ]), step: nextStep.step, - description: nextStep.description, instruction: nextStep.instruction, - ...(nextStep.context && { context: nextStep.context }) + ...(nextStep.context && { context: nextStep.context }), + completion_reminder: Copy.CompletionReminder }; } else { return { @@ -112,9 +112,9 @@ export class ExecutionPlan { plan: this.executionSteps.map((step, index) => `${index + 1}. ${step.description}`), firstStep: { step: firstStep.step, - description: firstStep.description, instruction: firstStep.instruction, - ...(firstStep.context && { context: firstStep.context }) + ...(firstStep.context && { context: firstStep.context }), + completion_reminder: Copy.CompletionReminder } }; } diff --git a/__tests__/Services/AIFunctionService.test.ts b/__tests__/Services/AIFunctionService.test.ts index 47476c6..4cdf55f 100644 --- a/__tests__/Services/AIFunctionService.test.ts +++ b/__tests__/Services/AIFunctionService.test.ts @@ -782,20 +782,26 @@ describe('AIFunctionService - Integration Tests', () => { }); describe('performAIFunction - Unknown Function', () => { - it('should throw error for unknown function', async () => { - await expect(service.performAIFunction({ + it('should return unknown response for unknown function', async () => { + const result = await service.performAIFunction({ name: 'UnknownFunction' as any, arguments: {}, toolId: 'tool_27' - } as any)).rejects.toThrow('Unknown function name: UnknownFunction'); + } as any); + expect(result.name).toBe('unknown'); + expect(result.toolId).toBe('tool_27'); + expect(result.response).toEqual({ error: 'Unknown function request unknown' }); }); - it('should throw error for invalid function', async () => { - await expect(service.performAIFunction({ + it('should return unknown response for invalid function', async () => { + const result = await service.performAIFunction({ name: 'InvalidFunction' as any, arguments: {}, toolId: 'tool_error' - } as any)).rejects.toThrow('Unknown function name: InvalidFunction'); + } as any); + expect(result.name).toBe('unknown'); + expect(result.toolId).toBe('tool_error'); + expect(result.response).toEqual({ error: 'Unknown function request unknown' }); }); }); diff --git a/__tests__/Services/ChatService.test.ts b/__tests__/Services/ChatService.test.ts index 62a5264..37ca197 100644 --- a/__tests__/Services/ChatService.test.ts +++ b/__tests__/Services/ChatService.test.ts @@ -126,10 +126,10 @@ describe('ChatService - Integration Tests (Sync Methods Only)', () => { expect(fromString('request_web_search')).toBe(AIFunction.RequestWebSearch); }); - it('should throw an error for unknown function names', () => { - expect(() => fromString('unknown_function')).toThrow('Unknown function name: unknown_function'); - expect(() => fromString('test_function')).toThrow('Unknown function name: test_function'); - expect(() => fromString('')).toThrow('Unknown function name: '); + it('should return Unknown for unknown function names', () => { + expect(fromString('unknown_function')).toBe(AIFunction.Unknown); + expect(fromString('test_function')).toBe(AIFunction.Unknown); + expect(fromString('')).toBe(AIFunction.Unknown); }); });