From 5b42428d6c12e759468daca27fc7d8d9a00d020b Mon Sep 17 00:00:00 2001 From: Sergei Shmakov Date: Tue, 8 Jul 2025 21:08:21 +0200 Subject: [PATCH 1/2] Refactors AI feedback context handling for changelog docs Centralizes management of AI feedback context and changelog document URIs into a dedicated provider, improving modularity and cleanup. Updates relevant usages to access changelog feedback context through the new provider, removing redundant context management logic from command files. Simplifies resource cleanup on document close and ensures consistent context availability for AI-related features. (#4449, #4480) --- src/commands/aiFeedback.ts | 2 +- src/commands/generateChangelog.ts | 49 ++----------------- src/container.ts | 9 ++++ src/plus/ai/utils/-webview/ai.utils.ts | 5 +- src/telemetry/aiFeedbackProvider.ts | 67 ++++++++++++++++++++++++++ 5 files changed, 82 insertions(+), 50 deletions(-) create mode 100644 src/telemetry/aiFeedbackProvider.ts diff --git a/src/commands/aiFeedback.ts b/src/commands/aiFeedback.ts index 8e88489cb10ad..8c36d98c3c810 100644 --- a/src/commands/aiFeedback.ts +++ b/src/commands/aiFeedback.ts @@ -49,7 +49,7 @@ const uriResponses = new UriMap(); let _updateContextDebounced: Deferrable<() => void> | undefined; async function sendFeedback(container: Container, uri: Uri, sentiment: AIFeedbackEvent['sentiment']): Promise { - const context = extractAIResultContext(uri); + const context = extractAIResultContext(container, uri); if (!context) return; try { diff --git a/src/commands/generateChangelog.ts b/src/commands/generateChangelog.ts index 5abcdb7ac88bb..e5aa58c20d870 100644 --- a/src/commands/generateChangelog.ts +++ b/src/commands/generateChangelog.ts @@ -1,4 +1,4 @@ -import type { CancellationToken, ProgressOptions, Uri } from 'vscode'; +import type { CancellationToken, ProgressOptions } from 'vscode'; import { ProgressLocation, window, workspace } from 'vscode'; import type { Source } from '../constants.telemetry'; import type { Container } from '../container'; @@ -6,13 +6,10 @@ import type { GitReference } from '../git/models/reference'; import { getChangesForChangelog } from '../git/utils/-webview/log.utils'; import { createRevisionRange, shortenRevision } from '../git/utils/revision.utils'; import { showGenericErrorMessage } from '../messages'; -import type { AIGenerateChangelogChanges, AIResultContext } from '../plus/ai/aiProviderService'; +import type { AIGenerateChangelogChanges } from '../plus/ai/aiProviderService'; import { getAIResultContext } from '../plus/ai/utils/-webview/ai.utils'; import { showComparisonPicker } from '../quickpicks/comparisonPicker'; import { command } from '../system/-webview/command'; -import { setContext } from '../system/-webview/context'; -import type { Deferrable } from '../system/function/debounce'; -import { debounce } from '../system/function/debounce'; import type { Lazy } from '../system/lazy'; import { lazy } from '../system/lazy'; import { Logger } from '../system/logger'; @@ -25,36 +22,6 @@ export interface GenerateChangelogCommandArgs { source?: Source; } -// Storage for AI feedback context associated with changelog documents -const changelogFeedbackContexts = new Map(); -export function getChangelogFeedbackContext(documentUri: string): AIResultContext | undefined { - return changelogFeedbackContexts.get(documentUri); -} -function setChangelogFeedbackContext(documentUri: string, context: AIResultContext): void { - changelogFeedbackContexts.set(documentUri, context); -} -function clearChangelogFeedbackContext(documentUri: string): void { - changelogFeedbackContexts.delete(documentUri); -} - -// Storage for changelog document URIs -const changelogUris = new Set(); -let _updateChangelogContextDebounced: Deferrable<() => void> | undefined; -function updateChangelogContext(): void { - _updateChangelogContextDebounced ??= debounce(() => { - void setContext('gitlens:tabs:ai:changelog', [...changelogUris]); - }, 100); - _updateChangelogContextDebounced(); -} -function addChangelogUri(uri: Uri): void { - changelogUris.add(uri); - updateChangelogContext(); -} -function removeChangelogUri(uri: Uri): void { - changelogUris.delete(uri); - updateChangelogContext(); -} - @command() export class GenerateChangelogCommand extends GlCommandBase { constructor(private readonly container: Container) { @@ -158,17 +125,7 @@ export async function generateChangelogAndOpenMarkdownDocument( const document = await workspace.openTextDocument({ language: 'markdown', content: content }); if (feedbackContext) { // Store feedback context for this document - setChangelogFeedbackContext(document.uri.toString(), feedbackContext); - // Add to changelog URIs context even for no-results documents - addChangelogUri(document.uri); - // Clean up context when document is closed - const disposable = workspace.onDidCloseTextDocument(closedDoc => { - if (closedDoc.uri.toString() === document.uri.toString()) { - clearChangelogFeedbackContext(document.uri.toString()); - removeChangelogUri(document.uri); - disposable.dispose(); - } - }); + container.aiFeedback.addChangelogDocument(document.uri, feedbackContext); } await window.showTextDocument(document); } diff --git a/src/container.ts b/src/container.ts index 58143fb312b0d..9f90a52eace28 100644 --- a/src/container.ts +++ b/src/container.ts @@ -57,6 +57,7 @@ import type { Storage } from './system/-webview/storage'; import { memoize } from './system/decorators/-webview/memoize'; import { log } from './system/decorators/log'; import { Logger } from './system/logger'; +import { AIFeedbackProvider } from './telemetry/aiFeedbackProvider'; import { TelemetryService } from './telemetry/telemetry'; import { UsageTracker } from './telemetry/usageTracker'; import { isWalkthroughSupported, WalkthroughStateProvider } from './telemetry/walkthroughStateProvider'; @@ -365,6 +366,14 @@ export class Container { return this._ai; } + private _aiFeedback: AIFeedbackProvider | undefined; + get aiFeedback(): AIFeedbackProvider { + if (this._aiFeedback == null) { + this._disposables.push((this._aiFeedback = new AIFeedbackProvider())); + } + return this._aiFeedback; + } + private _autolinks: AutolinksProvider | undefined; get autolinks(): AutolinksProvider { if (this._autolinks == null) { diff --git a/src/plus/ai/utils/-webview/ai.utils.ts b/src/plus/ai/utils/-webview/ai.utils.ts index 34d00b7522638..afd99817684f4 100644 --- a/src/plus/ai/utils/-webview/ai.utils.ts +++ b/src/plus/ai/utils/-webview/ai.utils.ts @@ -1,6 +1,5 @@ import type { Disposable, QuickInputButton } from 'vscode'; import { env, ThemeIcon, Uri, window } from 'vscode'; -import { getChangelogFeedbackContext } from '../../../../commands/generateChangelog'; import { Schemes } from '../../../../constants'; import type { AIProviders } from '../../../../constants.ai'; import type { Container } from '../../../../container'; @@ -283,7 +282,7 @@ export function getAIResultContext(result: AIResult): AIResultContext { }; } -export function extractAIResultContext(uri: Uri | undefined): AIResultContext | undefined { +export function extractAIResultContext(container: Container, uri: Uri | undefined): AIResultContext | undefined { if (uri?.scheme === Schemes.GitLensAIMarkdown) { const { authority } = uri; if (!authority) return undefined; @@ -300,7 +299,7 @@ export function extractAIResultContext(uri: Uri | undefined): AIResultContext | // Check for untitled documents with stored changelog feedback context if (uri?.scheme === 'untitled') { try { - return getChangelogFeedbackContext(uri.toString()); + return container.aiFeedback.getChangelogFeedback(uri.toString()); } catch { return undefined; } diff --git a/src/telemetry/aiFeedbackProvider.ts b/src/telemetry/aiFeedbackProvider.ts new file mode 100644 index 0000000000000..67a690addb7fa --- /dev/null +++ b/src/telemetry/aiFeedbackProvider.ts @@ -0,0 +1,67 @@ +import type { Disposable, Uri } from 'vscode'; +import { workspace } from 'vscode'; +import type { AIResultContext } from '../plus/ai/aiProviderService'; +import { setContext } from '../system/-webview/context'; +import type { Deferrable } from '../system/function/debounce'; +import { debounce } from '../system/function/debounce'; + +export class AIFeedbackProvider implements Disposable { + constructor() { + // Listen for document close events to clean up contexts + this._disposables.push( + workspace.onDidCloseTextDocument(document => this.removeChangelogDocument(document.uri)), + ); + } + + public addChangelogDocument(uri: Uri, context: AIResultContext): void { + this.setChangelogFeedback(uri.toString(), context); + this.addChangelogUri(uri); + } + + private removeChangelogDocument(uri: Uri): void { + this.deleteChangelogFeedback(uri.toString()); + this.removeChangelogUri(uri); + } + + private readonly _disposables: Disposable[] = []; + dispose(): void { + this._disposables.forEach(d => void d.dispose()); + this._changelogFeedbacks.clear(); + this._changelogUris.clear(); + this._updateChangelogContextDebounced = undefined; + } + + // Storage for changelog document URIs + private readonly _changelogUris = new Set(); + private _updateChangelogContextDebounced: Deferrable<() => void> | undefined; + private updateChangelogContext(): void { + this._updateChangelogContextDebounced ??= debounce(() => { + void setContext('gitlens:tabs:ai:changelog', [...this._changelogUris]); + }, 100); + this._updateChangelogContextDebounced(); + } + private addChangelogUri(uri: Uri): void { + if (!this._changelogUris.has(uri)) { + this._changelogUris.add(uri); + this.updateChangelogContext(); + } + } + private removeChangelogUri(uri: Uri): void { + if (this._changelogUris.has(uri)) { + this._changelogUris.delete(uri); + this.updateChangelogContext(); + } + } + + // Storage for AI feedback context associated with changelog documents + private readonly _changelogFeedbacks = new Map(); + getChangelogFeedback(documentUri: string): AIResultContext | undefined { + return this._changelogFeedbacks.get(documentUri); + } + private setChangelogFeedback(documentUri: string, context: AIResultContext): void { + this._changelogFeedbacks.set(documentUri, context); + } + private deleteChangelogFeedback(documentUri: string): void { + this._changelogFeedbacks.delete(documentUri); + } +} From 9cf17707150ac339f4ac6281c9b431ef4a830f38 Mon Sep 17 00:00:00 2001 From: Sergei Shmakov Date: Tue, 8 Jul 2025 21:30:16 +0200 Subject: [PATCH 2/2] Moves AI feedback URI state handling to provider Consolidates storage and context updates for AI feedback responses from the command layer into the provider, improving separation of concerns and maintainability. Ensures feedback state is managed centrally, reducing duplication and potential for inconsistencies. (#4449, #4480) --- src/commands/aiFeedback.ts | 22 ++---------- src/plus/ai/utils/-webview/ai.utils.ts | 2 +- src/telemetry/aiFeedbackProvider.ts | 50 ++++++++++++++++++++------ 3 files changed, 44 insertions(+), 30 deletions(-) diff --git a/src/commands/aiFeedback.ts b/src/commands/aiFeedback.ts index 8c36d98c3c810..40c8816130ed8 100644 --- a/src/commands/aiFeedback.ts +++ b/src/commands/aiFeedback.ts @@ -6,11 +6,7 @@ import type { AIResultContext } from '../plus/ai/aiProviderService'; import { extractAIResultContext } from '../plus/ai/utils/-webview/ai.utils'; import type { QuickPickItemOfT } from '../quickpicks/items/common'; import { command } from '../system/-webview/command'; -import { setContext } from '../system/-webview/context'; -import { UriMap } from '../system/-webview/uriMap'; -import type { Deferrable } from '../system/function/debounce'; -import { debounce } from '../system/function/debounce'; -import { filterMap, map } from '../system/iterable'; +import { map } from '../system/iterable'; import { Logger } from '../system/logger'; import { ActiveEditorCommand } from './commandBase'; import { getCommandUri } from './commandBase.utils'; @@ -45,15 +41,12 @@ export class AIFeedbackUnhelpfulCommand extends ActiveEditorCommand { type UnhelpfulResult = { reasons?: AIFeedbackUnhelpfulReasons[]; custom?: string }; -const uriResponses = new UriMap(); -let _updateContextDebounced: Deferrable<() => void> | undefined; - async function sendFeedback(container: Container, uri: Uri, sentiment: AIFeedbackEvent['sentiment']): Promise { const context = extractAIResultContext(container, uri); if (!context) return; try { - const previous = uriResponses.get(uri); + const previous = container.aiFeedback.getFeedbackResponse(uri); if (sentiment === previous) return; let unhelpful: UnhelpfulResult | undefined; @@ -61,16 +54,7 @@ async function sendFeedback(container: Container, uri: Uri, sentiment: AIFeedbac unhelpful = await showUnhelpfulFeedbackPicker(); } - uriResponses.set(uri, sentiment); - _updateContextDebounced ??= debounce(() => { - void setContext('gitlens:tabs:ai:helpful', [ - ...filterMap(uriResponses, ([uri, sentiment]) => (sentiment === 'helpful' ? uri : undefined)), - ]); - void setContext('gitlens:tabs:ai:unhelpful', [ - ...filterMap(uriResponses, ([uri, sentiment]) => (sentiment === 'unhelpful' ? uri : undefined)), - ]); - }, 100); - _updateContextDebounced(); + container.aiFeedback.setFeedbackResponse(uri, sentiment); sendFeedbackEvent(container, { source: 'ai:markdown-preview' }, context, sentiment, unhelpful); } catch (ex) { diff --git a/src/plus/ai/utils/-webview/ai.utils.ts b/src/plus/ai/utils/-webview/ai.utils.ts index afd99817684f4..8fc0611aa138f 100644 --- a/src/plus/ai/utils/-webview/ai.utils.ts +++ b/src/plus/ai/utils/-webview/ai.utils.ts @@ -299,7 +299,7 @@ export function extractAIResultContext(container: Container, uri: Uri | undefine // Check for untitled documents with stored changelog feedback context if (uri?.scheme === 'untitled') { try { - return container.aiFeedback.getChangelogFeedback(uri.toString()); + return container.aiFeedback.getChangelogDocument(uri.toString()); } catch { return undefined; } diff --git a/src/telemetry/aiFeedbackProvider.ts b/src/telemetry/aiFeedbackProvider.ts index 67a690addb7fa..91a14456bd350 100644 --- a/src/telemetry/aiFeedbackProvider.ts +++ b/src/telemetry/aiFeedbackProvider.ts @@ -1,9 +1,12 @@ import type { Disposable, Uri } from 'vscode'; import { workspace } from 'vscode'; +import type { AIFeedbackEvent } from '../constants.telemetry'; import type { AIResultContext } from '../plus/ai/aiProviderService'; import { setContext } from '../system/-webview/context'; +import { UriMap } from '../system/-webview/uriMap'; import type { Deferrable } from '../system/function/debounce'; import { debounce } from '../system/function/debounce'; +import { filterMap } from '../system/iterable'; export class AIFeedbackProvider implements Disposable { constructor() { @@ -14,20 +17,22 @@ export class AIFeedbackProvider implements Disposable { } public addChangelogDocument(uri: Uri, context: AIResultContext): void { - this.setChangelogFeedback(uri.toString(), context); + this.setChangelogDocument(uri.toString(), context); this.addChangelogUri(uri); } private removeChangelogDocument(uri: Uri): void { - this.deleteChangelogFeedback(uri.toString()); + this.deleteChangelogDocument(uri.toString()); this.removeChangelogUri(uri); } private readonly _disposables: Disposable[] = []; dispose(): void { this._disposables.forEach(d => void d.dispose()); - this._changelogFeedbacks.clear(); + this._uriResponses.clear(); + this._changelogDocuments.clear(); this._changelogUris.clear(); + this._updateFeedbackContextDebounced = undefined; this._updateChangelogContextDebounced = undefined; } @@ -54,14 +59,39 @@ export class AIFeedbackProvider implements Disposable { } // Storage for AI feedback context associated with changelog documents - private readonly _changelogFeedbacks = new Map(); - getChangelogFeedback(documentUri: string): AIResultContext | undefined { - return this._changelogFeedbacks.get(documentUri); + private readonly _changelogDocuments = new Map(); + getChangelogDocument(documentUri: string): AIResultContext | undefined { + return this._changelogDocuments.get(documentUri); } - private setChangelogFeedback(documentUri: string, context: AIResultContext): void { - this._changelogFeedbacks.set(documentUri, context); + private setChangelogDocument(documentUri: string, context: AIResultContext): void { + this._changelogDocuments.set(documentUri, context); } - private deleteChangelogFeedback(documentUri: string): void { - this._changelogFeedbacks.delete(documentUri); + private deleteChangelogDocument(documentUri: string): void { + this._changelogDocuments.delete(documentUri); + } + + // Storage for AI feedback responses by URI + private readonly _uriResponses = new UriMap(); + private _updateFeedbackContextDebounced: Deferrable<() => void> | undefined; + private updateFeedbackContext(): void { + this._updateFeedbackContextDebounced ??= debounce(() => { + void setContext('gitlens:tabs:ai:helpful', [ + ...filterMap(this._uriResponses, ([uri, sentiment]) => (sentiment === 'helpful' ? uri : undefined)), + ]); + void setContext('gitlens:tabs:ai:unhelpful', [ + ...filterMap(this._uriResponses, ([uri, sentiment]) => (sentiment === 'unhelpful' ? uri : undefined)), + ]); + }, 100); + this._updateFeedbackContextDebounced(); + } + setFeedbackResponse(uri: Uri, sentiment: AIFeedbackEvent['sentiment']): void { + const previous = this._uriResponses.get(uri); + if (sentiment === previous) return; + + this._uriResponses.set(uri, sentiment); + this.updateFeedbackContext(); + } + getFeedbackResponse(uri: Uri): AIFeedbackEvent['sentiment'] | undefined { + return this._uriResponses.get(uri); } }