fix: sort terminal type-ahead latencies numerically by yuki-nakano-520 · Pull Request #341303 · microsoft/vscode · GitHub
Skip to content

fix: sort terminal type-ahead latencies numerically - #341303

Merged
Connor Peet (connor4312) merged 1 commit into
microsoft:mainfrom
yuki-nakano-520:fix/typeahead-latency-numeric-sort
Oct 13, 2026
Merged

Connor Peet (connor4312) merged 1 commit into
microsoft:mainfrom
yuki-nakano-520:fix/typeahead-latency-numeric-sort

Conversation

@yuki-nakano-520

Copy link
Copy Markdown

Fixes #341302

Problem

PredictionStats.latency sorted an array of numbers with .sort() and no compare function, so the latencies were ordered as strings. min, median and max were wrong whenever the recorded latencies had a different number of digits. For example, latencies of 5ms, 50ms and 200ms were reported as min: 200, median: 5, max: 50.

These values are used to decide whether local echo predictions are shown (the median is compared against terminal.integrated.localEchoLatencyThreshold) and are sent in the terminalLatencyStats telemetry event.

Changes

  • Pass a numeric compare function to sort in PredictionStats.latency.
  • Add a unit test that records latencies with a different number of digits. The existing test only used 100, 300 and 500, which sort the same way as strings and as numbers.

How to test

Run the unit tests:

scripts\test.bat --grep "Terminal Typeahead"

or on macOS/Linux:

./scripts/test.sh --grep "Terminal Typeahead"

The new test sorts latencies numerically fails without the fix and passes with it. I have not verified the change in the UI, because it requires controlling the terminal latency.

Copilot AI balanced review requested due to automatic review settings October 13, 2026 13:59
@vs-code-engineering

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused fix is correct and the regression test would fail without it.

0 open findings

What changed in this PR

Fixes terminal type-ahead latency statistics by ensuring numeric ordering.

Changes:

  • Sorts recorded latencies numerically.
  • Adds regression coverage for mixed-digit latency values.
File Description
terminalTypeAheadAddon.ts Uses a numeric sort comparator.
terminalTypeAhead.test.ts Verifies correct min, median, and max values.

🧠 Review effort: Balanced


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@connor4312 Connor Peet (connor4312) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@connor4312
Connor Peet (connor4312) merged commit 2e086c5 into microsoft:main Oct 13, 2026
55 of 56 checks passed
@vs-code-engineering vs-code-engineering Bot added this to the 1.143.0 milestone Oct 13, 2026
@yuki-nakano-520
yuki nakano (yuki-nakano-520) deleted the fix/typeahead-latency-numeric-sort branch October 13, 2026 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Terminal type-ahead latency stats are sorted as strings instead of numbers

5 participants