Skip to content

Commit cf0ce85

Browse files
authored
Merge pull request #163 from rajbos/copilot/add-diagnostics-storage-panel
Add Azure Storage backend configuration panel to diagnostics
2 parents 3062b5b + 842983a commit cf0ce85

5 files changed

Lines changed: 493 additions & 74 deletions

File tree

.github/copilot-instructions.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,21 @@ The entire extension's logic is contained within the `CopilotTokenTracker` class
3636
- **Focused Modifications**: Make surgical, precise changes that address the specific requirements without affecting other functionality.
3737
- **Preserve Existing Structure**: Maintain the existing code organization and file structure. Don't refactor or reorganize code unless it's essential for the task.
3838

39+
## Logging Best Practices
40+
41+
**CRITICAL**: Do NOT add debug logging statements like `log('[DEBUG] message')` for troubleshooting during development. This approach has been found to interfere with the output channel and can hide existing log messages from appearing.
42+
43+
- **Use Existing Logs**: The extension already has comprehensive logging throughout. Review existing log statements to understand what's being tracked.
44+
- **Minimal Logging**: Only add logging if absolutely necessary for a new feature. Keep messages concise and informative.
45+
- **Remove Debug Logs**: Any temporary debug logging added during development MUST be removed before committing code.
46+
- **Log Methods**: Use the appropriate method for the severity:
47+
- `log(message)` - Standard informational messages
48+
- `warn(message)` - Warnings or recoverable errors
49+
- `error(message)` - Critical errors
50+
- **No Debug Prefixes**: Avoid prefixing messages with `[DEBUG]` or similar markers. The log output is already timestamped and categorized.
51+
52+
If you need to troubleshoot execution flow, prefer using VS Code's debugger with breakpoints rather than adding log statements.
53+
3954
## Key Files & Conventions
4055

4156
- **`src/extension.ts`**: The single source file containing all logic.

src/backend/facade.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ export class BackendFacade {
7676
getCopilotSessionFiles: deps.getCopilotSessionFiles,
7777
estimateTokensFromText: deps.estimateTokensFromText,
7878
getModelFromRequest: deps.getModelFromRequest,
79-
getSessionFileDataCached: deps.getSessionFileDataCached
79+
getSessionFileDataCached: deps.getSessionFileDataCached,
80+
updateTokenStats: deps.updateTokenStats
8081
},
8182
this.credentialService,
8283
this.dataPlaneService,
@@ -226,6 +227,7 @@ export class BackendFacade {
226227
const settings = this.getSettings();
227228
const result = await this.syncService.syncToBackendStore(force, settings, this.isConfigured(settings));
228229
this.clearQueryCache();
230+
// UI update is now handled by syncService after successful completion
229231
return result;
230232
}
231233

@@ -478,8 +480,8 @@ export class BackendFacade {
478480
const next = applyDraftToSettings(previousSettings, draft, consentAt);
479481
await this.updateConfiguration(next);
480482
this.startTimerIfEnabled();
481-
this.deps.updateTokenStats?.();
482483
this.clearQueryCache();
484+
// UI update happens automatically after sync completes via syncService callback
483485
return { state: await this.getConfigPanelState(), message: 'Settings saved.' };
484486
}
485487

src/backend/services/syncService.ts

Lines changed: 163 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ export interface SyncServiceDeps {
6060
getModelFromRequest: (request: ChatRequest) => string;
6161
// Cache integration for performance
6262
getSessionFileDataCached?: (sessionFilePath: string, mtime: number) => Promise<SessionFileCache>;
63+
// UI refresh callback after successful sync
64+
updateTokenStats?: () => Promise<void>;
6365
}
6466

6567
/**
@@ -128,8 +130,8 @@ export class SyncService {
128130
}
129131
});
130132
}, intervalMs);
131-
// Immediate initial sync
132-
this.syncToBackendStore(false, settings, isConfigured).catch((e) => {
133+
// Immediate initial sync (forced to ensure settings changes take effect)
134+
this.syncToBackendStore(true, settings, isConfigured).catch((e) => {
133135
this.deps.warn(`Backend sync initial sync failed: ${e?.message ?? e}`);
134136
});
135137
} catch (e) {
@@ -163,17 +165,23 @@ export class SyncService {
163165
}
164166

165167
/**
166-
* Process a session file using cached data.
168+
* Process a session file using cached data for token counts but extracting accurate timestamps.
167169
* Returns true if successful, false if cache miss (caller should parse file).
168170
* Validates all cached data at runtime to prevent injection/corruption.
171+
*
172+
* CRITICAL: We parse the file to extract actual interaction timestamps and create per-day
173+
* rollups, but use cached token counts for performance. This ensures accurate day assignment
174+
* while still benefiting from cached calculations.
169175
*/
170176
private async processCachedSessionFile(
171177
sessionFile: string,
172178
fileMtimeMs: number,
173179
workspaceId: string,
174180
machineId: string,
175181
userId: string | undefined,
176-
rollups: Map<string, { key: DailyRollupKey; value: DailyRollupValue }>
182+
rollups: Map<string, { key: DailyRollupKey; value: DailyRollupValue }>,
183+
startMs: number,
184+
now: Date
177185
): Promise<boolean> {
178186
try {
179187
const cachedData = await this.deps.getSessionFileDataCached!(sessionFile, fileMtimeMs);
@@ -192,39 +200,123 @@ export class SyncService {
192200
return false;
193201
}
194202

195-
// Expand cached modelUsage into rollups
196-
const dayKey = this.utility.toUtcDayKey(new Date(fileMtimeMs));
203+
// Parse the session file to get actual request timestamps and create per-day rollups
204+
// This ensures accurate day assignment while using cached token counts
205+
const content = await fs.promises.readFile(sessionFile, 'utf8');
197206

198-
// CRITICAL FIX: Only assign interactions to first model to prevent inflation
199-
// When a file has multiple models, interactions should be counted once, not per-model
200-
let interactionsAssigned = false;
207+
// Map to track per-day per-model interactions for proper distribution
208+
const dayModelInteractions = new Map<string, Map<string, number>>();
201209

202-
for (const [model, usage] of Object.entries(cachedData.modelUsage)) {
203-
// Validate usage object structure
204-
if (!usage || typeof usage !== 'object') {
205-
this.deps.warn(`Backend sync: invalid usage object for model ${model} in ${sessionFile}`);
206-
continue;
210+
// Handle JSONL format (Copilot CLI)
211+
if (sessionFile.endsWith('.jsonl')) {
212+
const lines = content.trim().split('\n');
213+
const todayKey = this.utility.toUtcDayKey(now);
214+
let lineCount = 0;
215+
let processedLines = 0;
216+
217+
for (const line of lines) {
218+
lineCount++;
219+
if (!line.trim()) { continue; }
220+
try {
221+
const event = JSON.parse(line);
222+
if (!event || typeof event !== 'object') { continue; }
223+
224+
const normalizedTs = this.utility.normalizeTimestampToMs(event.timestamp);
225+
const eventMs = Number.isFinite(normalizedTs) ? normalizedTs : fileMtimeMs;
226+
if (!eventMs || eventMs < startMs) { continue; }
227+
228+
const dayKey = this.utility.toUtcDayKey(new Date(eventMs));
229+
const model = (event.model || 'gpt-4o').toString();
230+
const isFileFromToday = dayKey === todayKey;
231+
if (isFileFromToday && processedLines < 3) {
232+
this.deps.log(`Backend sync: file ${sessionFile.split(/[/\\]/).pop()} line ${lineCount}: eventMs=${new Date(eventMs).toISOString()}, dayKey=${dayKey}, type=${event.type}`);
233+
processedLines++;
207234
}
208-
if (!Number.isFinite(usage.inputTokens) || usage.inputTokens < 0) {
209-
this.deps.warn(`Backend sync: invalid inputTokens for model ${model} in ${sessionFile}`);
210-
continue;
235+
// Track interaction for this day+model (count all events, not just user.message)
236+
if (!dayModelInteractions.has(dayKey)) {
237+
dayModelInteractions.set(dayKey, new Map());
238+
}
239+
const dayMap = dayModelInteractions.get(dayKey)!;
240+
dayMap.set(model, (dayMap.get(model) || 0) + 1);
241+
} catch {
242+
// skip malformed line
243+
}
211244
}
212-
if (!Number.isFinite(usage.outputTokens) || usage.outputTokens < 0) {
213-
this.deps.warn(`Backend sync: invalid outputTokens for model ${model} in ${sessionFile}`);
214-
continue;
245+
} else {
246+
// Handle JSON format (VS Code Copilot Chat)
247+
try {
248+
const sessionJson = JSON.parse(content);
249+
if (!sessionJson || typeof sessionJson !== 'object') {
250+
return false;
251+
}
252+
const sessionObj = sessionJson as Record<string, unknown>;
253+
const requests = Array.isArray(sessionObj.requests) ? (sessionObj.requests as unknown[]) : [];
254+
255+
for (const request of requests) {
256+
const req = request as ChatRequest;
257+
const normalizedTs = this.utility.normalizeTimestampToMs(
258+
typeof req.timestamp !== 'undefined' ? req.timestamp : (sessionObj.lastMessageDate as unknown)
259+
);
260+
const eventMs = Number.isFinite(normalizedTs) ? normalizedTs : fileMtimeMs;
261+
if (!eventMs || eventMs < startMs) { continue; }
262+
263+
const dayKey = this.utility.toUtcDayKey(new Date(eventMs));
264+
const model = this.deps.getModelFromRequest(req);
265+
266+
267+
// Track interaction for this day+model
268+
if (!dayModelInteractions.has(dayKey)) {
269+
dayModelInteractions.set(dayKey, new Map());
270+
}
271+
const dayMap = dayModelInteractions.get(dayKey)!;
272+
dayMap.set(model, (dayMap.get(model) || 0) + 1);
273+
}
274+
} catch (e) {
275+
this.deps.warn(`Backend sync: failed to parse JSON for ${sessionFile}: ${e}`);
276+
return false;
215277
}
216-
217-
const key: DailyRollupKey = { day: dayKey, model, workspaceId, machineId, userId };
218-
// Only assign interactions to the first valid model to prevent inflation
219-
const interactionsForThisModel = interactionsAssigned ? 0 : cachedData.interactions;
220-
interactionsAssigned = true;
221-
222-
upsertDailyRollup(rollups, key, {
223-
inputTokens: usage.inputTokens,
224-
outputTokens: usage.outputTokens,
225-
interactions: interactionsForThisModel
226-
});
227278
}
279+
280+
// Now distribute cached token counts proportionally across day+model combinations
281+
// based on the actual interaction distribution we just calculated
282+
for (const [dayKey, modelMap] of dayModelInteractions) {
283+
for (const [model, interactions] of modelMap) {
284+
const cachedUsage = cachedData.modelUsage[model];
285+
if (!cachedUsage) { continue; }
286+
287+
// Validate usage object structure
288+
if (!Number.isFinite(cachedUsage.inputTokens) || cachedUsage.inputTokens < 0) {
289+
this.deps.warn(`Backend sync: invalid inputTokens for model ${model}`);
290+
continue;
291+
}
292+
if (!Number.isFinite(cachedUsage.outputTokens) || cachedUsage.outputTokens < 0) {
293+
this.deps.warn(`Backend sync: invalid outputTokens for model ${model}`);
294+
continue;
295+
}
296+
297+
const key: DailyRollupKey = { day: dayKey, model, workspaceId, machineId, userId };
298+
299+
// For simplicity, if a file spans multiple days, distribute tokens proportionally
300+
// In practice, most session files are from a single day, so this is accurate
301+
const totalInteractionsForModel = Array.from(dayModelInteractions.values())
302+
.reduce((sum, m) => sum + (m.get(model) || 0), 0);
303+
304+
const tokenRatio = totalInteractionsForModel > 0 ? interactions / totalInteractionsForModel : 1;
305+
306+
upsertDailyRollup(rollups, key, {
307+
inputTokens: Math.round(cachedUsage.inputTokens * tokenRatio),
308+
outputTokens: Math.round(cachedUsage.outputTokens * tokenRatio),
309+
interactions: interactions
310+
});
311+
}
312+
}
313+
314+
// Log if this file had data for multiple days (for debugging)
315+
if (dayModelInteractions.size > 1) {
316+
const days = Array.from(dayModelInteractions.keys()).sort();
317+
this.deps.log(`Backend sync: file ${sessionFile.split(/[/\\]/).pop()} spans ${days.length} days: ${days.join(', ')}`);
318+
}
319+
228320
return true;
229321
} catch (e) {
230322
// Differentiate between cache miss (expected) and errors (unexpected)
@@ -308,6 +400,11 @@ export class SyncService {
308400
start.setUTCHours(0, 0, 0, 0);
309401
start.setUTCDate(start.getUTCDate() - (lookbackDays - 1));
310402
const startMs = start.getTime();
403+
404+
// Log the date range being processed
405+
const todayKey = this.utility.toUtcDayKey(now);
406+
const startKey = this.utility.toUtcDayKey(start);
407+
this.deps.log(`Backend sync: processing sessions from ${startKey} to ${todayKey} (lookback ${lookbackDays} days)`);
311408

312409
const machineId = vscode.env.machineId;
313410
const rollups = new Map<string, { key: DailyRollupKey; value: DailyRollupValue }>();
@@ -322,6 +419,10 @@ export class SyncService {
322419
const useCachedData = !!this.deps.getSessionFileDataCached;
323420
let cacheHits = 0;
324421
let cacheMisses = 0;
422+
let filesSkipped = 0;
423+
let filesProcessed = 0;
424+
425+
this.deps.log(`Backend sync: analyzing ${sessionFiles.length} session files`);
325426

326427
for (const sessionFile of sessionFiles) {
327428
let fileMtimeMs: number | undefined;
@@ -330,10 +431,13 @@ export class SyncService {
330431
const fileStat = await fs.promises.stat(sessionFile);
331432
fileMtimeMs = fileStat.mtimeMs;
332433

434+
333435
// Skip files older than lookback period
334436
if (fileMtimeMs < startMs) {
437+
filesSkipped++;
335438
continue;
336439
}
440+
filesProcessed++;
337441
} catch (e) {
338442
this.deps.warn(`Backend sync: failed to stat session file ${sessionFile}: ${e}`);
339443
continue;
@@ -342,15 +446,19 @@ export class SyncService {
342446
const workspaceId = this.utility.extractWorkspaceIdFromSessionPath(sessionFile);
343447
await this.ensureWorkspaceNameResolved(workspaceId, sessionFile, workspaceNamesById);
344448

345-
// Try to use cached data first (much faster than parsing)
449+
// Try to use cached data first (faster than full recomputation)
450+
// Note: We still parse the file to get accurate day keys from timestamps,
451+
// but use cached token counts for performance
346452
if (useCachedData) {
347453
const cacheSuccess = await this.processCachedSessionFile(
348454
sessionFile,
349455
fileMtimeMs,
350456
workspaceId,
351457
machineId,
352458
userId,
353-
rollups
459+
rollups,
460+
startMs,
461+
now
354462
);
355463

356464
if (cacheSuccess) {
@@ -474,6 +582,8 @@ export class SyncService {
474582
if (useCachedData) {
475583
this.logCachePerformance(cacheHits, cacheMisses);
476584
}
585+
586+
this.deps.log(`Backend sync: processed ${filesProcessed} files, skipped ${filesSkipped} files outside lookback period`);
477587

478588
return { rollups, workspaceNamesById, machineNamesById };
479589
}
@@ -502,6 +612,8 @@ export class SyncService {
502612
// Avoid excessive syncing when UI refreshes frequently.
503613
const lastSyncAt = this.deps.context?.globalState.get<number>('backend.lastSyncAt');
504614
if (!force && lastSyncAt && Date.now() - lastSyncAt < BACKEND_SYNC_MIN_INTERVAL_MS) {
615+
const secondsSinceLastSync = Math.round((Date.now() - lastSyncAt) / 1000);
616+
this.deps.log(`Backend sync: skipping (last sync was ${secondsSinceLastSync}s ago, minimum interval is ${BACKEND_SYNC_MIN_INTERVAL_MS / 1000}s)`);
505617
return;
506618
}
507619

@@ -518,6 +630,17 @@ export class SyncService {
518630

519631
const resolvedIdentity = await this.resolveEffectiveUserIdentityForSync(settings, sharingPolicy.includeUserDimension);
520632
const { rollups, workspaceNamesById, machineNamesById } = await this.computeDailyRollupsFromLocalSessions({ lookbackDays: settings.lookbackDays, userId: resolvedIdentity.userId });
633+
634+
// Log day keys being synced for better visibility
635+
const dayKeys = new Set<string>();
636+
for (const { key } of rollups.values()) {
637+
dayKeys.add(key.day);
638+
}
639+
const sortedDays = Array.from(dayKeys).sort();
640+
if (sortedDays.length > 0) {
641+
this.deps.log(`Backend sync: processing data for ${sortedDays.length} days: ${sortedDays.join(', ')}`);
642+
}
643+
521644
this.deps.log(`Backend sync: upserting ${rollups.size} rollup entities (lookback ${settings.lookbackDays} days)`);
522645

523646
const tableClient = this.dataPlaneService.createTableClient(settings, creds.tableCredential);
@@ -570,6 +693,13 @@ export class SyncService {
570693
}
571694

572695
this.deps.log('Backend sync: completed');
696+
697+
// Trigger UI refresh to update status bar and panels with latest sync data
698+
try {
699+
await this.deps.updateTokenStats?.();
700+
} catch (e) {
701+
this.deps.warn(`Backend sync: failed to update UI: ${e}`);
702+
}
573703
} catch (e: any) {
574704
// Keep local mode functional.
575705
const secretsToRedact = await this.credentialService.getBackendSecretsToRedactForError(settings);

0 commit comments

Comments
 (0)