From 37178ff1b9b919e2105f2e3bf2f11d3a2180f75a Mon Sep 17 00:00:00 2001 From: Eyal Toledano Date: Thu, 8 May 2025 19:04:25 -0400 Subject: [PATCH] feat(telemetry): Integrate AI usage telemetry into update-subtask This commit applies the standard telemetry pattern to the update-subtask command and its corresponding MCP tool. Key Changes: 1. Core Logic (scripts/modules/task-manager/update-subtask-by-id.js): - The call to generateTextService now includes commandName: 'update-subtask' and outputType. - The full response { mainResult, telemetryData } is captured. - mainResult (the AI-generated text) is used for the appended content. - If running in CLI mode (outputFormat === 'text'), displayAiUsageSummary is called with the telemetryData. - The function now returns { updatedSubtask: ..., telemetryData: ... }. 2. Direct Function (mcp-server/src/core/direct-functions/update-subtask-by-id.js): - The call to the core updateSubtaskById function now passes the necessary context for telemetry (commandName, outputType). - The successful response object now correctly extracts coreResult.telemetryData and includes it in the data.telemetryData field returned to the MCP client. --- .../direct-functions/update-subtask-by-id.js | 19 +++-- scripts/modules/ai-services-unified.js | 24 +++--- .../task-manager/update-subtask-by-id.js | 80 +++++++------------ tasks/task_077.txt | 2 +- tasks/task_081.txt | 11 ++- tasks/tasks.json | 6 +- 6 files changed, 69 insertions(+), 73 deletions(-) diff --git a/mcp-server/src/core/direct-functions/update-subtask-by-id.js b/mcp-server/src/core/direct-functions/update-subtask-by-id.js index 1264cbce..f2c433cf 100644 --- a/mcp-server/src/core/direct-functions/update-subtask-by-id.js +++ b/mcp-server/src/core/direct-functions/update-subtask-by-id.js @@ -108,18 +108,24 @@ export async function updateSubtaskByIdDirect(args, log, context = {}) { try { // Execute core updateSubtaskById function - const updatedSubtask = await updateSubtaskById( + const coreResult = await updateSubtaskById( tasksPath, subtaskIdStr, prompt, useResearch, - { mcpLog: logWrapper, session, projectRoot }, + { + mcpLog: logWrapper, + session, + projectRoot, + commandName: 'update-subtask', + outputType: 'mcp' + }, 'json' ); - if (updatedSubtask === null) { + if (!coreResult || coreResult.updatedSubtask === null) { const message = `Subtask ${id} or its parent task not found.`; - logWrapper.error(message); // Log as error since it couldn't be found + logWrapper.error(message); return { success: false, error: { code: 'SUBTASK_NOT_FOUND', message: message }, @@ -136,9 +142,10 @@ export async function updateSubtaskByIdDirect(args, log, context = {}) { message: `Successfully updated subtask with ID ${subtaskIdStr}`, subtaskId: subtaskIdStr, parentId: subtaskIdStr.split('.')[0], - subtask: updatedSubtask, + subtask: coreResult.updatedSubtask, tasksPath, - useResearch + useResearch, + telemetryData: coreResult.telemetryData }, fromCache: false }; diff --git a/scripts/modules/ai-services-unified.js b/scripts/modules/ai-services-unified.js index 1a0facca..11a3ffc2 100644 --- a/scripts/modules/ai-services-unified.js +++ b/scripts/modules/ai-services-unified.js @@ -229,18 +229,22 @@ async function _attemptProviderCallWithRetries( while (retries <= MAX_RETRIES) { try { - log( - 'info', - `Attempt ${retries + 1}/${MAX_RETRIES + 1} calling ${fnName} (Provider: ${providerName}, Model: ${modelId}, Role: ${attemptRole})` - ); + if (getDebugFlag()) { + log( + 'info', + `Attempt ${retries + 1}/${MAX_RETRIES + 1} calling ${fnName} (Provider: ${providerName}, Model: ${modelId}, Role: ${attemptRole})` + ); + } // Call the specific provider function directly const result = await providerApiFn(callParams); - log( - 'info', - `${fnName} succeeded for role ${attemptRole} (Provider: ${providerName}) on attempt ${retries + 1}` - ); + if (getDebugFlag()) { + log( + 'info', + `${fnName} succeeded for role ${attemptRole} (Provider: ${providerName}) on attempt ${retries + 1}` + ); + } return result; } catch (error) { log( @@ -253,13 +257,13 @@ async function _attemptProviderCallWithRetries( const delay = INITIAL_RETRY_DELAY_MS * Math.pow(2, retries - 1); log( 'info', - `Retryable error detected. Retrying in ${delay / 1000}s...` + `Something went wrong on the provider side. Retrying in ${delay / 1000}s...` ); await new Promise((resolve) => setTimeout(resolve, delay)); } else { log( 'error', - `Non-retryable error or max retries reached for role ${attemptRole} (${fnName} / ${providerName}).` + `Something went wrong on the provider side. Max retries reached for role ${attemptRole} (${fnName} / ${providerName}).` ); throw error; } diff --git a/scripts/modules/task-manager/update-subtask-by-id.js b/scripts/modules/task-manager/update-subtask-by-id.js index 29a685d0..2c030436 100644 --- a/scripts/modules/task-manager/update-subtask-by-id.js +++ b/scripts/modules/task-manager/update-subtask-by-id.js @@ -7,7 +7,8 @@ import Table from 'cli-table3'; import { getStatusWithColor, startLoadingIndicator, - stopLoadingIndicator + stopLoadingIndicator, + displayAiUsageSummary } from '../ui.js'; import { log as consoleLog, @@ -154,8 +155,10 @@ async function updateSubtaskById( ); } - let generatedContentString = ''; // Initialize to empty string - let newlyAddedSnippet = ''; // <--- ADD THIS LINE: Variable to store the snippet for CLI display + let generatedContentString = ''; + let newlyAddedSnippet = ''; + let aiServiceResponse = null; + try { const parentContext = { id: parentTask.id, @@ -202,73 +205,44 @@ Output Requirements: const role = useResearch ? 'research' : 'main'; report('info', `Using AI text service with role: ${role}`); - // Store the entire response object from the AI service - const aiServiceResponse = await generateTextService({ + aiServiceResponse = await generateTextService({ prompt: userPrompt, systemPrompt: systemPrompt, role, session, projectRoot, - maxRetries: 2 + maxRetries: 2, + commandName: 'update-subtask', + outputType: isMCP ? 'mcp' : 'cli' }); - report( - 'info', - `>>> DEBUG: AI Service Response Object: ${JSON.stringify(aiServiceResponse, null, 2)}` - ); - report( - 'info', - `>>> DEBUG: Extracted generatedContentString: "${generatedContentString}"` - ); - - // Extract the actual text content from the mainResult property - // and ensure it's a string, defaulting to empty if not. if ( aiServiceResponse && aiServiceResponse.mainResult && - typeof aiServiceResponse.mainResult.text === 'string' + typeof aiServiceResponse.mainResult === 'string' ) { - generatedContentString = aiServiceResponse.mainResult.text; + generatedContentString = aiServiceResponse.mainResult; } else { - generatedContentString = ''; // Default to empty if mainResult.text is not a string or the path is invalid + generatedContentString = ''; + report( + 'warn', + 'AI service response did not contain expected text string.' + ); } - // The telemetryData would be in aiServiceResponse.telemetryData if needed elsewhere - - report( - 'success', - 'Successfully received response object from AI service' // Log message updated for clarity - ); if (outputFormat === 'text' && loadingIndicator) { stopLoadingIndicator(loadingIndicator); loadingIndicator = null; } - - // This check now correctly validates the extracted string - if (typeof generatedContentString !== 'string') { - report( - 'warn', - 'AI mainResult was not a valid text string. Treating as empty.' - ); - generatedContentString = ''; // Ensure it's a string for trim() later - } else if (generatedContentString.trim() !== '') { - report( - 'success', - `Successfully extracted text from AI response using role: ${role}.` - ); - } - // No need for an else here, as an empty string from mainResult is a valid scenario - // that will be handled by the `if (generatedContentString && generatedContentString.trim())` later. } catch (aiError) { report('error', `AI service call failed: ${aiError.message}`); if (outputFormat === 'text' && loadingIndicator) { - stopLoadingIndicator(loadingIndicator); // Ensure stop on error + stopLoadingIndicator(loadingIndicator); loadingIndicator = null; } throw aiError; } - // --- TIMESTAMP & FORMATTING LOGIC (Handled Locally) --- if (generatedContentString && generatedContentString.trim()) { // Check if the string is not empty const timestamp = new Date().toISOString(); @@ -277,21 +251,15 @@ Output Requirements: subtask.details = (subtask.details ? subtask.details + '\n' : '') + formattedBlock; - report( - 'info', - 'Appended timestamped, formatted block with AI-generated content to subtask.details.' - ); } else { report( 'warn', 'AI response was empty or whitespace after trimming. Original details remain unchanged.' ); - newlyAddedSnippet = 'No new details were added by the AI.'; // <--- ADD THIS LINE: Set message for CLI + newlyAddedSnippet = 'No new details were added by the AI.'; } - // --- END TIMESTAMP & FORMATTING LOGIC --- const updatedSubtask = parentTask.subtasks[subtaskIndex]; - report('info', 'Updated subtask details locally after AI generation.'); if (outputFormat === 'text' && getDebugFlag(session)) { console.log( @@ -349,7 +317,15 @@ Output Requirements: ) ); } - return updatedSubtask; + + if (outputFormat === 'text' && aiServiceResponse.telemetryData) { + displayAiUsageSummary(aiServiceResponse.telemetryData, 'cli'); + } + + return { + updatedSubtask: updatedSubtask, + telemetryData: aiServiceResponse.telemetryData + }; } catch (error) { if (outputFormat === 'text' && loadingIndicator) { stopLoadingIndicator(loadingIndicator); diff --git a/tasks/task_077.txt b/tasks/task_077.txt index a3dd4157..ef98667f 100644 --- a/tasks/task_077.txt +++ b/tasks/task_077.txt @@ -230,7 +230,7 @@ Apply telemetry pattern from telemetry.mdc: * Verify `handleApiResult` correctly passes `data.telemetryData` through. -## 11. Telemetry Integration for update-subtask-by-id [pending] +## 11. Telemetry Integration for update-subtask-by-id [in-progress] ### Dependencies: None ### Description: Integrate AI usage telemetry capture and propagation for the update-subtask-by-id functionality. ### Details: diff --git a/tasks/task_081.txt b/tasks/task_081.txt index 59daf16c..99d23434 100644 --- a/tasks/task_081.txt +++ b/tasks/task_081.txt @@ -99,9 +99,18 @@ The testing strategy for the expanded telemetry system should be comprehensive a # Subtasks: ## 1. Implement Additional Telemetry Data Collection Points [pending] ### Dependencies: None -### Description: Extend the telemetry system to capture new metrics including command execution frequency, feature usage patterns, performance metrics, error rates, session data, and system environment information. +### Description: Extend the telemetry system to capture new metrics including command execution frequency, feature usage patterns, performance metrics, error rates, session data, and system environment information. [Updated: 5/8/2025] [Updated: 5/8/2025] [Updated: 5/8/2025] ### Details: Create new telemetry event types and collection points throughout the codebase. Implement hooks in the command execution pipeline to track timing and frequency. Add performance monitoring for key operations using high-resolution timers. Capture system environment data at startup. Implement error tracking that records error types and frequencies. Add session tracking with start/end events and periodic heartbeats. + +This is a test note added via the MCP tool. The telemetry collection system should be thoroughly tested before implementation. + + +For future server integration, Prometheus time-series database with its companion storage solutions (like Cortex or Thanos) would be an excellent choice for handling our telemetry data. The local telemetry collection system should be designed with compatible data structures and metrics formatting that will allow seamless export to Prometheus once server-side infrastructure is in place. This approach would provide powerful querying capabilities, visualization options through Grafana, and scalable long-term storage. Consider implementing the OpenMetrics format locally to ensure compatibility with the Prometheus ecosystem. + + +Prometheus would be an excellent choice for server-side telemetry storage and analysis. When designing the local telemetry collection system, we should structure our metrics and events to be compatible with Prometheus' data model (time series with key-value pairs). This would allow for straightforward export to Prometheus once server infrastructure is established. For long-term storage, companion solutions like Cortex or Thanos could extend Prometheus' capabilities, enabling historical analysis and scalable retention. Additionally, adopting the OpenMetrics format locally would ensure seamless integration with the broader Prometheus ecosystem, including visualization through Grafana dashboards. + ## 2. Build Robust Local Telemetry Storage System [pending] ### Dependencies: None diff --git a/tasks/tasks.json b/tasks/tasks.json index dde3bf3c..6c34c86a 100644 --- a/tasks/tasks.json +++ b/tasks/tasks.json @@ -5015,7 +5015,7 @@ "title": "Telemetry Integration for update-subtask-by-id", "description": "Integrate AI usage telemetry capture and propagation for the update-subtask-by-id functionality.", "details": "\\\nApply telemetry pattern from telemetry.mdc:\n\n1. **Core (`scripts/modules/task-manager/update-subtask-by-id.js`):**\n * Verify if this function *actually* calls an AI service. If it only appends text, telemetry integration might not apply directly here, but ensure its callers handle telemetry if they use AI.\n * *If it calls AI:* Modify AI service call to include `commandName: \\'update-subtask\\'` and `outputType`.\n * *If it calls AI:* Receive `{ mainResult, telemetryData }`.\n * *If it calls AI:* Return object including `telemetryData`.\n * *If it calls AI:* Handle CLI display via `displayAiUsageSummary` if applicable.\n\n2. **Direct (`mcp-server/src/core/direct-functions/update-subtask-by-id.js`):**\n * *If core calls AI:* Pass `commandName`, `outputType: \\'mcp\\'` to core.\n * *If core calls AI:* Pass `outputFormat: \\'json\\'` if applicable.\n * *If core calls AI:* Receive `{ ..., telemetryData }` from core.\n * *If core calls AI:* Return `{ success: true, data: { ..., telemetryData } }`.\n\n3. **Tool (`mcp-server/src/tools/update-subtask.js`):**\n * Verify `handleApiResult` correctly passes `data.telemetryData` through (if present).\n", - "status": "pending", + "status": "in-progress", "dependencies": [], "parentTaskId": 77 }, @@ -5154,9 +5154,9 @@ { "id": 1, "title": "Implement Additional Telemetry Data Collection Points", - "description": "Extend the telemetry system to capture new metrics including command execution frequency, feature usage patterns, performance metrics, error rates, session data, and system environment information.", + "description": "Extend the telemetry system to capture new metrics including command execution frequency, feature usage patterns, performance metrics, error rates, session data, and system environment information. [Updated: 5/8/2025] [Updated: 5/8/2025] [Updated: 5/8/2025]", "dependencies": [], - "details": "Create new telemetry event types and collection points throughout the codebase. Implement hooks in the command execution pipeline to track timing and frequency. Add performance monitoring for key operations using high-resolution timers. Capture system environment data at startup. Implement error tracking that records error types and frequencies. Add session tracking with start/end events and periodic heartbeats.", + "details": "Create new telemetry event types and collection points throughout the codebase. Implement hooks in the command execution pipeline to track timing and frequency. Add performance monitoring for key operations using high-resolution timers. Capture system environment data at startup. Implement error tracking that records error types and frequencies. Add session tracking with start/end events and periodic heartbeats.\n\nThis is a test note added via the MCP tool. The telemetry collection system should be thoroughly tested before implementation.\n\n\nFor future server integration, Prometheus time-series database with its companion storage solutions (like Cortex or Thanos) would be an excellent choice for handling our telemetry data. The local telemetry collection system should be designed with compatible data structures and metrics formatting that will allow seamless export to Prometheus once server-side infrastructure is in place. This approach would provide powerful querying capabilities, visualization options through Grafana, and scalable long-term storage. Consider implementing the OpenMetrics format locally to ensure compatibility with the Prometheus ecosystem.\n\n\nPrometheus would be an excellent choice for server-side telemetry storage and analysis. When designing the local telemetry collection system, we should structure our metrics and events to be compatible with Prometheus' data model (time series with key-value pairs). This would allow for straightforward export to Prometheus once server infrastructure is established. For long-term storage, companion solutions like Cortex or Thanos could extend Prometheus' capabilities, enabling historical analysis and scalable retention. Additionally, adopting the OpenMetrics format locally would ensure seamless integration with the broader Prometheus ecosystem, including visualization through Grafana dashboards.\n", "status": "pending", "testStrategy": "Create unit tests for each new telemetry point. Implement integration tests that verify telemetry is captured during normal application usage. Add mock services to verify data format correctness." },