Skip to content

Commit d5b5f48

Browse files
committed
fix: address Copilot review feedback for SchemaStore LRU and prompt constants
- Replace O(n) timestamp scan with O(1) Map insertion order LRU - Evict in while loop until size <= maxEntries - Fire schema-change event on eviction for UI consistency - Rename INDEX_ADVISOR_* constants to QUERY_PERFORMANCE_* to match updated persona
1 parent d3dbaa7 commit d5b5f48

2 files changed

Lines changed: 39 additions & 32 deletions

File tree

src/commands/llmEnhancedCommands/promptTemplates.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -207,14 +207,14 @@ export function setLastPromptSource(source: PromptSource): void {
207207
lastPromptSource = source;
208208
}
209209

210-
const INDEX_ADVISOR_ROLE = 'Query Performance Analyst';
210+
const QUERY_PERFORMANCE_ANALYST_ROLE = 'Query Performance Analyst';
211211
const QUERY_GENERATOR_ROLE = 'MongoDB Query Generator assistant';
212212

213-
const INDEX_ADVISOR_TASK_FIND =
213+
const QUERY_PERFORMANCE_TASK_FIND =
214214
'analyze MongoDB API query performance based on the data provided';
215-
const INDEX_ADVISOR_TASK_AGGREGATE =
215+
const QUERY_PERFORMANCE_TASK_AGGREGATE =
216216
'analyze MongoDB API aggregation pipeline performance based on the data provided';
217-
const INDEX_ADVISOR_TASK_COUNT =
217+
const QUERY_PERFORMANCE_TASK_COUNT =
218218
'analyze MongoDB API count query performance based on the data provided';
219219
const QUERY_GENERATOR_TASK =
220220
"generate MongoDB queries based on the user's natural language description and the provided schema information";
@@ -245,9 +245,9 @@ const SINGLE_COLLECTION_QUERY_MESSAGES = [
245245
];
246246

247247
export const FIND_QUERY_PROMPT_TEMPLATE = `
248-
${createPriorityDeclaration(INDEX_ADVISOR_ROLE)}
248+
${createPriorityDeclaration(QUERY_PERFORMANCE_ANALYST_ROLE)}
249249
250-
${createSecurityInstructions(FIND_QUERY_MESSAGES, INDEX_ADVISOR_TASK_FIND)}
250+
${createSecurityInstructions(FIND_QUERY_MESSAGES, QUERY_PERFORMANCE_TASK_FIND)}
251251
252252
## DATA PLACEHOLDERS
253253
The subsequent user messages will provide the following data that you should use to fill in your analysis:
@@ -396,9 +396,9 @@ ${CRITICAL_JSON_REMINDER}
396396
`;
397397

398398
export const AGGREGATE_QUERY_PROMPT_TEMPLATE = `
399-
${createPriorityDeclaration(INDEX_ADVISOR_ROLE)}
399+
${createPriorityDeclaration(QUERY_PERFORMANCE_ANALYST_ROLE)}
400400
401-
${createSecurityInstructions(AGGREGATE_QUERY_MESSAGES, INDEX_ADVISOR_TASK_AGGREGATE)}
401+
${createSecurityInstructions(AGGREGATE_QUERY_MESSAGES, QUERY_PERFORMANCE_TASK_AGGREGATE)}
402402
403403
## DATA PLACEHOLDERS
404404
The subsequent user messages will provide the following data that you should use to fill in your analysis:
@@ -555,9 +555,9 @@ ${CRITICAL_JSON_REMINDER}
555555
`;
556556

557557
export const COUNT_QUERY_PROMPT_TEMPLATE = `
558-
${createPriorityDeclaration(INDEX_ADVISOR_ROLE)}
558+
${createPriorityDeclaration(QUERY_PERFORMANCE_ANALYST_ROLE)}
559559
560-
${createSecurityInstructions(COUNT_QUERY_MESSAGES, INDEX_ADVISOR_TASK_COUNT)}
560+
${createSecurityInstructions(COUNT_QUERY_MESSAGES, QUERY_PERFORMANCE_TASK_COUNT)}
561561
562562
## DATA PLACEHOLDERS
563563
The subsequent user messages will provide the following data that you should use to fill in your analysis:

src/documentdb/SchemaStore.ts

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -88,35 +88,42 @@ export class SchemaStore implements vscode.Disposable {
8888

8989
// ── LRU tracking ──
9090

91-
/** Mark a key as recently accessed. */
91+
/** Mark a key as recently accessed (moves to end of insertion order). */
9292
private _touchKey(key: string): void {
93-
this._accessOrder.set(key, Date.now());
93+
this._accessOrder.delete(key);
94+
this._accessOrder.set(key, 0);
9495
}
9596

96-
/** Evict the least-recently-used entry if over the limit. */
97+
/** Evict least-recently-used entries until the store is at or below capacity. */
9798
private _evictIfNeeded(): void {
98-
if (this._analyzers.size <= this._maxEntries) {
99-
return;
100-
}
101-
102-
let oldestKey: string | undefined;
103-
let oldestTime = Infinity;
104-
for (const [key, time] of this._accessOrder) {
105-
if (time < oldestTime) {
106-
oldestTime = time;
107-
oldestKey = key;
99+
while (this._analyzers.size > this._maxEntries) {
100+
const oldestKey = this._accessOrder.keys().next().value as string | undefined;
101+
if (oldestKey !== undefined) {
102+
this._analyzers.delete(oldestKey);
103+
this._accessOrder.delete(oldestKey);
104+
this._evictionCount++;
105+
const pending = this._pendingNotifications.get(oldestKey);
106+
if (pending !== undefined) {
107+
clearTimeout(pending);
108+
this._pendingNotifications.delete(oldestKey);
109+
}
110+
// Fire a schema-change event so UI consumers know the schema was removed.
111+
const [clusterId, db, coll] = oldestKey.split('::', 3);
112+
if (clusterId && db && coll) {
113+
this._onDidChangeSchema.fire({ clusterId, databaseName: db, collectionName: coll });
114+
}
115+
continue;
108116
}
109-
}
110117

111-
if (oldestKey !== undefined) {
112-
this._analyzers.delete(oldestKey);
113-
this._accessOrder.delete(oldestKey);
114-
this._evictionCount++;
115-
const pending = this._pendingNotifications.get(oldestKey);
116-
if (pending !== undefined) {
117-
clearTimeout(pending);
118-
this._pendingNotifications.delete(oldestKey);
118+
// Fallback: _accessOrder may be out of sync with _analyzers.
119+
// Pick any entry from _analyzers and register it for future LRU.
120+
const fallbackKey = this._analyzers.keys().next().value as string | undefined;
121+
if (fallbackKey !== undefined) {
122+
this._touchKey(fallbackKey);
123+
continue;
119124
}
125+
126+
return;
120127
}
121128
}
122129

0 commit comments

Comments
 (0)