Add logging for autocompletion interactions Release-Notes: skip Change-Id: I6e5508e98c2c40667c7c233800f28d2a30b35224
diff --git a/polygerrit-ui/app/api/embed.ts b/polygerrit-ui/app/api/embed.ts index 2faeeda..d6425be 100644 --- a/polygerrit-ui/app/api/embed.ts +++ b/polygerrit-ui/app/api/embed.ts
@@ -45,6 +45,7 @@ /** <gr-textarea> event when showing a hint */ export declare interface HintShownEventDetail { hint: string; + oldValue: string; } /** <gr-textarea> event when a hint was dismissed */
diff --git a/polygerrit-ui/app/api/suggestions.ts b/polygerrit-ui/app/api/suggestions.ts index c3089a9..b952180 100644 --- a/polygerrit-ui/app/api/suggestions.ts +++ b/polygerrit-ui/app/api/suggestions.ts
@@ -72,6 +72,7 @@ export declare interface AutocompleteCommentResponse { responseCode: ResponseCode; completion?: string; + modelVersion?: string; } export declare interface SuggestCodeResponse {
diff --git a/polygerrit-ui/app/constants/reporting.ts b/polygerrit-ui/app/constants/reporting.ts index 37a17ba..643f8aa 100644 --- a/polygerrit-ui/app/constants/reporting.ts +++ b/polygerrit-ui/app/constants/reporting.ts
@@ -102,6 +102,8 @@ APPLY_FIX_LOAD = 'ApplyFixLoad', // Time to copy target to clipboard COPY_TO_CLIPBOARD = 'CopyToClipboard', + // Time to autocomplete a comment + COMMENT_COMPLETION = 'CommentCompletion', } export enum Interaction { @@ -156,4 +158,9 @@ // The very first reporting event with `ChangeId` set when visiting a change // related page. Can be used as a starting point for user journeys. CHANGE_ID_CHANGED = 'change-id-changed', + + COMMENT_COMPLETION_SUGGESTION_SHOWN = 'comment-completion-suggestion-shown', + COMMENT_COMPLETION_SUGGESTION_ACCEPTED = 'comment-completion-suggestion-accepted', + COMMENT_COMPLETION_SAVE_DRAFT = 'comment-completion-save-draft', + COMMENT_COMPLETION_SUGGESTION_FETCHED = 'comment-completion-suggestion-fetched', }
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts index a2fd3d6..f9bfbde 100644 --- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts +++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -79,8 +79,12 @@ commentModelToken, } from '../gr-comment-model/gr-comment-model'; import {formStyles} from '../../../styles/form-styles'; -import {Interaction} from '../../../constants/reporting'; -import {Suggestion, SuggestionsProvider} from '../../../api/suggestions'; +import {Interaction, Timing} from '../../../constants/reporting'; +import { + AutocompleteCommentResponse, + Suggestion, + SuggestionsProvider, +} from '../../../api/suggestions'; import {when} from 'lit/directives/when.js'; import {getDocUrl} from '../../../utils/url-util'; import {configModelToken} from '../../../models/config/config-model'; @@ -89,7 +93,12 @@ import {deepEqual} from '../../../utils/deep-util'; import {GrSuggestionDiffPreview} from '../gr-suggestion-diff-preview/gr-suggestion-diff-preview'; import {waitUntil} from '../../../utils/async-util'; -import {AutocompleteCache} from '../../../utils/autocomplete-cache'; +import { + AutocompleteCache, + AutocompletionContext, +} from '../../../utils/autocomplete-cache'; +import {HintAppliedEventDetail, HintShownEventDetail} from '../../../api/embed'; +import {levenshteinDistance} from '../../../utils/string-util'; // visible for testing export const AUTO_SAVE_DEBOUNCE_DELAY_MS = 2000; @@ -225,7 +234,9 @@ * An hint for autocompleting the comment message from plugin suggestion * providers. */ - @state() autocompleteHint = ''; + @state() autocompleteHint?: AutocompletionContext; + + private autocompleteAcceptedHints: string[] = []; /** Based on user preferences. */ @state() autocompleteEnabled = true; @@ -900,12 +911,70 @@ rows="4" .placeholder=${this.messagePlaceholder} text=${this.messageText} - autocompleteHint=${this.autocompleteHint} + autocompleteHint=${this.autocompleteHint?.commentCompletion ?? ''} @text-changed=${this.handleTextChanged} + @hintShown=${this.handleHintShown} + @hintApplied=${this.handleHintApplied} ></gr-suggestion-textarea> `; } + private handleHintShown(e: CustomEvent<HintShownEventDetail>) { + const context = this.autocompleteCache.get(e.detail.oldValue); + if (context?.commentCompletion !== e.detail.hint) return; + + this.reportHintInteraction( + Interaction.COMMENT_COMPLETION_SUGGESTION_SHOWN, + context + ); + } + + private handleHintApplied(e: CustomEvent<HintAppliedEventDetail>) { + const context = this.autocompleteCache.get(e.detail.oldValue); + if (context?.commentCompletion !== e.detail.hint) return; + + this.autocompleteAcceptedHints.push(e.detail.hint); + this.reportHintInteraction( + Interaction.COMMENT_COMPLETION_SUGGESTION_ACCEPTED, + context + ); + } + + private reportHintInteractionSaved() { + const content = this.messageText.trimEnd(); + const acceptedHintsConcatenated = this.autocompleteAcceptedHints.join(''); + const numExtraCharacters = + content.length - acceptedHintsConcatenated.length; + let distance = levenshteinDistance(acceptedHintsConcatenated, content); + if (numExtraCharacters > 0) { + distance -= numExtraCharacters; + } + const context = { + ...this.createAutocompletionBaseContext(), + similarCharacters: acceptedHintsConcatenated.length - distance, + maxSimilarCharacters: acceptedHintsConcatenated.length, + acceptedSuggestionsCount: this.autocompleteAcceptedHints.length, + totalAcceptedCharacters: acceptedHintsConcatenated.length, + savedDraftLength: content.length, + }; + this.reportHintInteraction( + Interaction.COMMENT_COMPLETION_SAVE_DRAFT, + context + ); + } + + private reportHintInteraction( + interaction: Interaction, + context: Partial<AutocompletionContext> + ) { + context = { + ...context, + draftContent: '[REDACTED]', + commentCompletion: '[REDACTED]', + }; + this.reporting.reportInteraction(interaction, context); + } + private handleTextChanged(e: ValueChangedEvent) { const oldValue = this.messageText; const newValue = e.detail.value; @@ -927,7 +996,7 @@ if (cachedHint) { this.autocompleteHint = cachedHint; } else { - this.autocompleteHint = ''; + this.autocompleteHint = undefined; this.autocompleteTrigger$.next(); } } @@ -1353,6 +1422,7 @@ return; } const commentText = this.messageText; + this.reporting.time(Timing.COMMENT_COMPLETION); const response = await suggestionsProvider.autocompleteComment({ id: id(this.comment), commentText, @@ -1362,10 +1432,51 @@ range: this.comment.range, lineNumber: this.comment.line, }); + const elapsed = this.reporting.timeEnd(Timing.COMMENT_COMPLETION); + const context = this.createAutocompletionContext( + commentText, + response, + elapsed + ); + this.reportHintInteraction( + Interaction.COMMENT_COMPLETION_SUGGESTION_FETCHED, + context + ); if (!response?.completion) return; - // Note that we are setting for `commentText` and getting for `this.messageText`. - this.autocompleteCache.set(commentText, response.completion); - this.autocompleteHint = this.autocompleteCache.get(this.messageText) ?? ''; + // Note that we are setting the cache value for `commentText` and getting the value + // for `this.messageText`. + this.autocompleteCache.set(context); + this.autocompleteHint = this.autocompleteCache.get(this.messageText); + } + + private createAutocompletionBaseContext(): Partial<AutocompletionContext> { + return { + commentId: id(this.comment!), + commentNumber: this.comments?.length ?? 0, + filePath: this.comment!.path, + fileExtension: getFileExtension(this.comment!.path ?? ''), + }; + } + + private createAutocompletionContext( + draftContent: string, + response: AutocompleteCommentResponse, + requestDurationMs: number + ): AutocompletionContext { + const commentCompletion = response.completion ?? ''; + return { + ...this.createAutocompletionBaseContext(), + + draftContent, + draftContentLength: draftContent.length, + commentCompletion, + commentCompletionLength: commentCompletion.length, + + isFullCommentPrediction: draftContent.length === 0, + draftInSyncWithSuggestionLength: 0, + modelVersion: response.modelVersion ?? '', + requestDurationMs, + }; } private renderRobotActions() { @@ -1794,6 +1905,7 @@ if (this.isFixSuggestionChanged()) { draft.fix_suggestions = this.getFixSuggestions(); } + this.reportHintInteractionSaved(); return this.getCommentsModel().saveDraft(draft, options.showToast); }
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts index 5e1d825..444a43f 100644 --- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts +++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -898,35 +898,39 @@ suite('handleTextChangedForAutocomplete', () => { test('foo -> foo with asdf', async () => { - element.autocompleteHint = 'asdf'; - element.autocompleteCache.set('foo', 'asdf'); + const ctx = {draftContent: 'foo', commentCompletion: 'asdf'}; + element.autocompleteHint = ctx; + element.autocompleteCache.set(ctx); element.messageText = 'foo'; element.handleTextChangedForAutocomplete(); - assert.equal(element.autocompleteHint, 'asdf'); + assert.equal(element.autocompleteHint.commentCompletion, 'asdf'); }); test('foo -> bar with asdf', async () => { - element.autocompleteHint = 'asdf'; - element.autocompleteCache.set('foo', 'asdf'); + const ctx = {draftContent: 'foo', commentCompletion: 'asdf'}; + element.autocompleteHint = ctx; + element.autocompleteCache.set(ctx); element.messageText = 'bar'; element.handleTextChangedForAutocomplete(); - assert.equal(element.autocompleteHint, ''); + assert.isUndefined(element.autocompleteHint); }); test('foo -> foofoo with asdf', async () => { - element.autocompleteHint = 'asdf'; - element.autocompleteCache.set('foo', 'asdf'); + const ctx = {draftContent: 'foo', commentCompletion: 'asdf'}; + element.autocompleteHint = ctx; + element.autocompleteCache.set(ctx); element.messageText = 'foofoo'; element.handleTextChangedForAutocomplete(); - assert.equal(element.autocompleteHint, ''); + assert.isUndefined(element.autocompleteHint); }); test('foo -> foofoo with foomore', async () => { - element.autocompleteHint = 'foomore'; - element.autocompleteCache.set('foo', 'foomore'); + const ctx = {draftContent: 'foo', commentCompletion: 'foomore'}; + element.autocompleteHint = ctx; + element.autocompleteCache.set(ctx); element.messageText = 'foofoo'; element.handleTextChangedForAutocomplete(); - assert.equal(element.autocompleteHint, 'more'); + assert.equal(element.autocompleteHint.commentCompletion, 'more'); }); });
diff --git a/polygerrit-ui/app/embed/gr-textarea.ts b/polygerrit-ui/app/embed/gr-textarea.ts index 628b65b..9b88e9c 100644 --- a/polygerrit-ui/app/embed/gr-textarea.ts +++ b/polygerrit-ui/app/embed/gr-textarea.ts
@@ -456,13 +456,7 @@ const value = await this.getValue(); this.innerValue = value; - this.dispatchEvent( - new CustomEvent('input', { - detail: { - value: this.value, - }, - }) - ); + this.fire('input', {value: this.value}); } private onFocus(event: Event) { @@ -492,7 +486,7 @@ (event.ctrlKey || event.metaKey) ) { event.preventDefault(); - this.dispatchEvent(new CustomEvent('saveShortcut')); + this.fire('saveShortcut'); } await this.toggleHintVisibilityIfAny(); } @@ -507,7 +501,13 @@ } private handleScroll() { - this.dispatchEvent(new CustomEvent('scroll')); + this.fire('scroll'); + } + + private fire<T>(type: string, detail?: T) { + this.dispatchEvent( + new CustomEvent(type, {detail, bubbles: true, composed: true}) + ); } private async handleTabKeyPress(event: KeyboardEvent) { @@ -529,14 +529,7 @@ await this.putCursorAtEnd(); await this.onInput(event); - this.dispatchEvent( - new CustomEvent('hintApplied', { - detail: { - hint, - oldValue, - }, - }) - ); + this.fire('hintApplied', {hint, oldValue}); } private async toggleHintVisibilityIfAny() { @@ -572,6 +565,7 @@ } private addHintSpanAtEndOfContent(editableDivElement: Node, hint: string) { + const oldValue = this.value ?? ''; const hintSpan = document.createElement('span'); hintSpan.classList.add(AUTOCOMPLETE_HINT_CLASS); hintSpan.setAttribute('role', 'alert'); @@ -581,26 +575,16 @@ ); hintSpan.dataset['hint'] = hint; editableDivElement.appendChild(hintSpan); - this.dispatchEvent( - new CustomEvent('hintShown', { - detail: { - hint, - }, - }) - ); + this.fire('hintShown', {hint, oldValue}); } private removeHintSpanIfShown() { const hintSpan = this.hintSpan(); if (hintSpan) { hintSpan.remove(); - this.dispatchEvent( - new CustomEvent('hintDismissed', { - detail: { - hint: (hintSpan as HTMLElement).dataset['hint'], - }, - }) - ); + this.fire('hintDismissed', { + hint: (hintSpan as HTMLElement).dataset['hint'], + }); } } @@ -616,13 +600,7 @@ event?.preventDefault(); event?.stopImmediatePropagation(); - this.dispatchEvent( - new CustomEvent('cursorPositionChange', { - detail: { - position: this.getCursorPosition(), - }, - }) - ); + this.fire('cursorPositionChange', {position: this.getCursorPosition()}); } private async updateValueInDom() {
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts index 6df2c67..e175228 100644 --- a/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts +++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
@@ -55,7 +55,7 @@ /** * Finish named timer and report it to server. */ - timeEnd(name: Timing, eventDetails?: EventDetails): void; + timeEnd(name: Timing, eventDetails?: EventDetails): number; /** * Get a timer object for reporting a user timing. The start time will be * the time that the object has been created, and the end time will be the
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts index 1eb3bc2..781f370 100644 --- a/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts +++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
@@ -770,23 +770,26 @@ /** * Finish named timer and report it to server. */ - timeEnd(name: Timing, eventDetails?: EventDetails) { + timeEnd(name: Timing, eventDetails?: EventDetails): number { if (!hasOwnProperty(this._baselines, name)) { - return; + return 0; } - const baseTime = this._baselines[name]; + const begin = this._baselines[name]; delete this._baselines[name]; - this._reportTiming(name, now() - baseTime, eventDetails); + const end = now(); + const elapsed = end - begin; + this._reportTiming(name, elapsed, eventDetails); // Finalize the interval. Either from a registered start mark or // the navigation start time (if baseTime is 0). - if (baseTime !== 0) { + if (begin !== 0) { window.performance.measure(name, `${name}-start`); } else { // Microsoft Edge does not handle the 2nd param correctly // (if undefined). window.performance.measure(name); } + return elapsed; } /**
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting_mock.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting_mock.ts index fb1f0c3..7cb777a 100644 --- a/polygerrit-ui/app/services/gr-reporting/gr-reporting_mock.ts +++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting_mock.ts
@@ -71,5 +71,5 @@ setRepoName: () => {}, setChangeId: () => {}, time: () => {}, - timeEnd: () => {}, + timeEnd: () => 0, };
diff --git a/polygerrit-ui/app/utils/autocomplete-cache.ts b/polygerrit-ui/app/utils/autocomplete-cache.ts index 8eea65d..c8077ab 100644 --- a/polygerrit-ui/app/utils/autocomplete-cache.ts +++ b/polygerrit-ui/app/utils/autocomplete-cache.ts
@@ -3,9 +3,27 @@ * Copyright 2024 Google LLC * SPDX-License-Identifier: Apache-2.0 */ -export interface Autocompletion { - completionContent: string; - completionHint: string; +export interface AutocompletionContext { + draftContent: string; + draftContentLength?: number; + commentCompletion: string; + commentCompletionLength?: number; + + isFullCommentPrediction?: boolean; + draftInSyncWithSuggestionLength?: number; + modelVersion?: string; + requestDurationMs?: number; + + commentId?: string; + commentNumber?: number; + filePath?: string; + fileExtension?: string; + + similarCharacters?: number; + maxSimilarCharacters?: number; + acceptedSuggestionsCount?: number; + totalAcceptedCharacters?: number; + savedDraftLength?: number; } /** @@ -21,31 +39,41 @@ * entries, if the capacity is exceeded. And we want to prefer newer entries over older * entries, if both match the criteria for being reused. */ - private cache: Autocompletion[] = []; + private cache: AutocompletionContext[] = []; constructor(private readonly capacity = 10) {} - get(content: string): string | undefined { + get(content: string): AutocompletionContext | undefined { if (content === '') return undefined; for (let i = this.cache.length - 1; i >= 0; i--) { - const {completionContent, completionHint} = this.cache[i]; + const cachedContext = this.cache[i]; + const completionContent = cachedContext.draftContent; + const completionHint = cachedContext.commentCompletion; const completionFull = completionContent + completionHint; if (completionContent.length > content.length) continue; if (!completionFull.startsWith(content)) continue; if (completionFull === content) continue; - return completionFull.substring(content.length); + const hint = completionFull.substring(content.length); + return { + ...cachedContext, + draftContent: content, + commentCompletion: hint, + draftInSyncWithSuggestionLength: + content.length - completionContent.length, + }; } return undefined; } - set(content: string, hint: string) { - const completion = {completionContent: content, completionHint: hint}; - const index = this.cache.findIndex(c => c.completionContent === content); + set(context: AutocompletionContext) { + const index = this.cache.findIndex( + c => c.draftContent === context.draftContent + ); if (index !== -1) { this.cache.splice(index, 1); } else if (this.cache.length >= this.capacity) { this.cache.shift(); } - this.cache.push(completion); + this.cache.push(context); } }
diff --git a/polygerrit-ui/app/utils/autocomplete-cache_test.ts b/polygerrit-ui/app/utils/autocomplete-cache_test.ts index 851737c..970436b 100644 --- a/polygerrit-ui/app/utils/autocomplete-cache_test.ts +++ b/polygerrit-ui/app/utils/autocomplete-cache_test.ts
@@ -13,63 +13,77 @@ cache = new AutocompleteCache(); }); + const cacheSet = (draftContent: string, commentCompletion: string) => { + cache.set({draftContent, commentCompletion}); + }; + + const assertCacheEqual = ( + draftContent: string, + expectedCommentCompletion?: string + ) => { + assert.equal( + cache.get(draftContent)?.commentCompletion, + expectedCommentCompletion + ); + }; + test('should get and set values', () => { - cache.set('foo', 'bar'); - assert.equal(cache.get('foo'), 'bar'); + cacheSet('foo', 'bar'); + assertCacheEqual('foo', 'bar'); }); test('should return undefined for empty content string', () => { - cache.set('foo', 'bar'); - assert.equal(cache.get(''), undefined); + cacheSet('foo', 'bar'); + assertCacheEqual('', undefined); }); test('should return a value, if completion content+hint start with content', () => { - cache.set('foo', 'bar'); - assert.equal(cache.get('foo'), 'bar'); - assert.equal(cache.get('foob'), 'ar'); - assert.equal(cache.get('fooba'), 'r'); - assert.equal(cache.get('foobar'), undefined); + cacheSet('foo', 'bar'); + assertCacheEqual('foo', 'bar'); + assertCacheEqual('foob', 'ar'); + assertCacheEqual('fooba', 'r'); + assertCacheEqual('foobar', undefined); }); test('should not return a value, if content is shorter than completion content', () => { - cache.set('foo', 'bar'); - assert.equal(cache.get('f'), undefined); - assert.equal(cache.get('fo'), undefined); + cacheSet('foo', 'bar'); + assertCacheEqual('f', undefined); + assertCacheEqual('fo', undefined); }); test('should not get values that are not set', () => { - assert.equal(cache.get('foo'), undefined); + assertCacheEqual('foo', undefined); }); test('should not return an empty completion, if content equals completion content+hint', () => { - cache.set('foo', 'bar'); - assert.equal(cache.get('foobar'), undefined); + cacheSet('foo', 'bar'); + assertCacheEqual('foobar', undefined); }); test('skips over the first entry, but returns the second entry', () => { - cache.set('foobar', 'bang'); - cache.set('foo', 'bar'); - assert.equal(cache.get('foobar'), 'bang'); + cacheSet('foobar', 'bang'); + cacheSet('foo', 'bar'); + assertCacheEqual('foobar', 'bang'); }); test('replaces entries', () => { - cache.set('foo', 'bar'); - cache.set('foo', 'baz'); - assert.equal(cache.get('foo'), 'baz'); + cacheSet('foo', 'bar'); + cacheSet('foo', 'baz'); + assertCacheEqual('foo', 'baz'); }); test('prefers newer entries, but also returns older entries', () => { - cache.set('foo', 'bar'); - assert.equal(cache.get('foob'), 'ar'); - cache.set('foob', 'arg'); - assert.equal(cache.get('foob'), 'arg'); - assert.equal(cache.get('foo'), 'bar'); + cacheSet('foo', 'bar'); + assertCacheEqual('foob', 'ar'); + cacheSet('foob', 'arg'); + assertCacheEqual('foob', 'arg'); + assertCacheEqual('foo', 'bar'); }); test('capacity', () => { cache = new AutocompleteCache(1); - cache.set('foo', 'bar'); - cache.set('boom', 'bang'); - assert.equal(cache.get('foo'), undefined); + cacheSet('foo', 'bar'); + cacheSet('boom', 'bang'); + assertCacheEqual('foo', undefined); }); });
diff --git a/polygerrit-ui/app/utils/string-util.ts b/polygerrit-ui/app/utils/string-util.ts index 81dcde1..abc5529 100644 --- a/polygerrit-ui/app/utils/string-util.ts +++ b/polygerrit-ui/app/utils/string-util.ts
@@ -115,3 +115,42 @@ fileName: fileNameSection, }; } + +/** + * Computes the Levenshtein edit distance between two strings. + */ +export function levenshteinDistance(str1: string, str2: string): number { + const m = str1.length; + const n = str2.length; + + // Create a matrix to store edit distances + const dp: number[][] = Array.from({length: m + 1}, () => + Array(n + 1).fill(0) + ); + + // Initialize first row and column with base cases + for (let i = 0; i <= m; i++) { + dp[i][0] = i; + } + for (let j = 0; j <= n; j++) { + dp[0][j] = j; + } + + // Calculate edit distances for all substrings + for (let i = 1; i <= m; i++) { + for (let j = 1; j <= n; j++) { + if (str1[i - 1] === str2[j - 1]) { + dp[i][j] = dp[i - 1][j - 1]; + } else { + dp[i][j] = Math.min( + dp[i - 1][j] + 1, // Deletion + dp[i][j - 1] + 1, // Insertion + dp[i - 1][j - 1] + 1 // Substitution + ); + } + } + } + + // Return the final edit distance + return dp[m][n]; +}