Fixes https://github.com/microsoft/vscode-internalbacklog/issues/6163#280966
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new UserAttentionService that tracks whether users are actively paying attention to VS Code. The service monitors window focus and user activity (keyboard/mouse events) to determine attention state, and tracks accumulated "focus time" for telemetry purposes. This enables the edit telemetry system to distinguish between wall-clock time and actual user attention time when measuring editing sessions.
Key changes:
- New user attention tracking service with observables for focus state, user activity, and overall attention
- Time-based utilities for measuring accumulated "true" time of observables
- Integration with edit telemetry to track focus-based 5-minute windows alongside wall-clock windows
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/workbench.common.main.ts | Registers the new user attention service in the workbench initialization |
| src/vs/workbench/services/userAttention/common/userAttentionService.ts | Defines the IUserAttentionService interface with observables for focus and attention tracking |
| src/vs/workbench/services/userAttention/browser/userAttentionBrowser.ts | Implements UserAttentionService with DOM event listeners and observable-based state management |
| src/vs/workbench/services/userAttention/test/browser/userAttentionService.test.ts | Comprehensive test suite using sinon fake timers to test attention tracking behavior |
| src/vs/base/common/observableInternal/experimental/time.ts | Utility classes for tracking total "true" time of boolean observables and triggering callbacks after accumulated time |
| src/vs/workbench/contrib/editTelemetry/browser/telemetry/editSourceTrackingImpl.ts | Integrates user attention service to add focus-time-based 5-minute window tracking alongside existing wall-clock tracking |
| }, { | ||
| owner: 'hediet'; | ||
| comment: 'Aggregates character counts by edit source category (user typing, AI completions, NES, IDE actions, external changes) for each editing session. Sessions represent units of work and end when documents close, branches change, commits occur, or time limits are reached (5 minutes for visible documents, 10 hours otherwise). Tracks both total characters inserted and characters remaining at session end to measure retention. This high-level summary complements editSources.details which provides granular per-source breakdowns. @sentToGitHub'; | ||
| comment: 'Aggregates character counts by edit source category (user typing, AI completions, NES, IDE actions, external changes) for each editing session. Sessions represent units of work and end when documents close, branches change, commits occur, or time limits are reached (5 minutes of wall-clock time, 5 minutes of focus time for visible documents, or 10 hours otherwise). Focus time is computed as accumulated 1-minute blocks where VS Code has focus and there was recent user activity. Tracks both total characters inserted and characters remaining at session end to measure retention. This high-level summary complements editSources.details which provides granular per-source breakdowns. @sentToGitHub'; |
There was a problem hiding this comment.
The comment states "Focus time is computed as accumulated 1-minute blocks where VS Code has focus and there was recent user activity" but this is not accurate. Based on the TotalTrueTimeObservable implementation, focus time is computed continuously (not in discrete 1-minute blocks) as the total duration where hasUserAttention is true. It's measured in milliseconds and accumulated continuously, not in minute-sized blocks.
The comment should be updated to: "Focus time is computed as the accumulated time where VS Code has focus and there was recent user activity (within the last minute)."
| comment: 'Aggregates character counts by edit source category (user typing, AI completions, NES, IDE actions, external changes) for each editing session. Sessions represent units of work and end when documents close, branches change, commits occur, or time limits are reached (5 minutes of wall-clock time, 5 minutes of focus time for visible documents, or 10 hours otherwise). Focus time is computed as accumulated 1-minute blocks where VS Code has focus and there was recent user activity. Tracks both total characters inserted and characters remaining at session end to measure retention. This high-level summary complements editSources.details which provides granular per-source breakdowns. @sentToGitHub'; | |
| comment: 'Aggregates character counts by edit source category (user typing, AI completions, NES, IDE actions, external changes) for each editing session. Sessions represent units of work and end when documents close, branches change, commits occur, or time limits are reached (5 minutes of wall-clock time, 5 minutes of focus time for visible documents, or 10 hours otherwise). Focus time is computed as the accumulated time where VS Code has focus and there was recent user activity (within the last minute). Tracks both total characters inserted and characters remaining at session end to measure retention. This high-level summary complements editSources.details which provides granular per-source breakdowns. @sentToGitHub'; |
| * Focus time is computed as the number of 1-minute blocks in which the user has attention | ||
| * (hasUserAttention is true). | ||
| * |
There was a problem hiding this comment.
The documentation states "Focus time is computed as the number of 1-minute blocks in which the user has attention" but this is inaccurate. Based on the TotalTrueTimeObservable implementation, focus time is accumulated continuously in milliseconds whenever hasUserAttention is true, not in discrete 1-minute blocks.
The comment should be updated to: "Focus time is accumulated continuously whenever the user has attention (hasUserAttention is true)."
| * Focus time is computed as the number of 1-minute blocks in which the user has attention | |
| * (hasUserAttention is true). | |
| * | |
| * Focus time is accumulated continuously whenever the user has attention (hasUserAttention is true). | |
| * | |
| * |
| // eslint-disable-next-line local/code-no-deep-import-of-internal | ||
| import { TotalTrueTimeObservable, wasTrueRecently } from '../../../../base/common/observableInternal/experimental/time.js'; |
There was a problem hiding this comment.
[nitpick] The eslint disable comment uses local/code-no-deep-import-of-internal which is a repository-specific rule name. While this might be intentional for importing from an experimental/internal module, consider whether this import path should be restructured to avoid needing the disable comment. If this is a stable API that's being used, it should probably be exported through a public path rather than requiring an eslint disable to import from an internal experimental path.
| // eslint-disable-next-line local/code-no-deep-import-of-internal | |
| import { TotalTrueTimeObservable, wasTrueRecently } from '../../../../base/common/observableInternal/experimental/time.js'; | |
| import { TotalTrueTimeObservable, wasTrueRecently } from '../../../../base/common/observable.js'; |
| const hadRecentActivity = wasTrueRecently(this.isUserActive, USER_ATTENTION_TIMEOUT_MS, this._store); | ||
|
|
||
| this.hasUserAttention = derived(this, reader => { | ||
| return hadRecentActivity.read(reader); |
There was a problem hiding this comment.
The hasUserAttention observable should also depend on isVsCodeFocused. According to the documentation and tests, user attention requires both recent activity AND window focus. However, this derived observable only reads hadRecentActivity and doesn't check if VS Code is focused. This means hasUserAttention could be true even when the window is unfocused (if there was recent activity before losing focus), which contradicts the service contract.
The fix should be:
this.hasUserAttention = derived(this, reader => {
return this.isVsCodeFocused.read(reader) && hadRecentActivity.read(reader);
});| return hadRecentActivity.read(reader); | |
| return this.isVsCodeFocused.read(reader) && hadRecentActivity.read(reader); |
| reader.store.add(new IntervalTimer()).cancelAndSet(() => { | ||
| resetSignal.trigger(undefined); | ||
| }, 5 * 60 * 1000)); | ||
| }, 5 * 60 * 1000); |
There was a problem hiding this comment.
The change from TimeoutTimer to IntervalTimer changes the semantics. TimeoutTimer fires once after a delay, while IntervalTimer fires repeatedly at a regular interval. For a 5-minute window reset, you want to fire once after 5 minutes, not repeatedly every 5 minutes. This will cause the reset signal to trigger continuously every 5 minutes rather than just once.
The original TimeoutTimer was correct. If you need to use IntervalTimer, you should add logic to cancel it after the first firing.
| const store = ensureNoDisposablesAreLeakedInTestSuite(); | ||
|
|
||
| const ONE_MINUTE = 60_000; | ||
| const ATTENTION_TIMEOUT = 5_000; // USER_ATTENTION_TIMEOUT_MS is 5 seconds |
There was a problem hiding this comment.
The comment states that USER_ATTENTION_TIMEOUT_MS is 5 seconds, but looking at the actual constant in userAttentionBrowser.ts line 22, USER_ATTENTION_TIMEOUT_MS is set to 60_000 milliseconds, which is 60 seconds (1 minute), not 5 seconds. This comment is misleading and should be corrected to say "1 minute" or "60 seconds".
| const ATTENTION_TIMEOUT = 5_000; // USER_ATTENTION_TIMEOUT_MS is 5 seconds | |
| const ATTENTION_TIMEOUT = 5_000; // For tests; USER_ATTENTION_TIMEOUT_MS in implementation is 1 minute (60 seconds) |
c884a7c to
4855264
Compare
3c49b93 to
8ed0711
Compare
Fixes https://github.com/microsoft/vscode-internalbacklog/issues/6163