Skip to content

[Server] Filter list completions in one pass - #649

Merged
chr-hertel merged 1 commit into
modelcontextprotocol:mainfrom
andreybolonin:perf/reduce-formatter-completion-allocations
Nov 2, 2026
Merged

chr-hertel merged 1 commit into
modelcontextprotocol:mainfrom
andreybolonin:perf/reduce-formatter-completion-allocations

Conversation

@andreybolonin

@andreybolonin andreybolonin commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Same as #652 for ListCompletionProvider. The tool formatting part is gone, #656 made that change on main.

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.

🟡 Changes recommended

The by-value loop breaks references in nested content arrays, causing stale objects to be returned if a later JsonSerializable item reassigns them.

1 open issue
What changed in this PR

This PR reduces intermediate array allocations in the PHP MCP SDK’s tool-result formatting and prefix completion paths.

Changes:

  • Replaces repeated content-array merges with direct appends and keyed assignments.
  • Builds prefix completion results in a single pass.
  • Adds tests for nested content keys, object identity, completion order, duplicates, and edge cases.
File Description
tests/​Unit/​Capability/​Formatter/​ToolResultFormatterTest.php Tests mixed nested content, key handling, and object identity.
tests/​Unit/​Capability/​Completion/​ListCompletionProviderTest.php Tests prefix matches, duplicates, missing matches, and zero-prefix behavior.
src/​Capability/​Formatter/​ToolResultFormatter.php Replaces repeated merges with direct insertion.
src/​Capability/​Completion/​ListCompletionProvider.php Replaces filtering and reindexing with one-pass collection.

Review effort: Balanced

@chr-hertel chr-hertel changed the title Avoid intermediate arrays in tool formatting and completions [Server] Filter list completions in one pass Nov 2, 2026
@chr-hertel
chr-hertel force-pushed the perf/reduce-formatter-completion-allocations branch from 1085001 to 839d40b Compare November 2, 2026 22:30
@chr-hertel
chr-hertel requested a balanced review from Copilot November 2, 2026 22:30

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

No active review issues were identified for the single-pass completion filtering change.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@chr-hertel
chr-hertel merged commit 579819f into modelcontextprotocol:main Nov 2, 2026
28 checks passed
@chr-hertel chr-hertel added the Server Issues & PRs related to the Server component label Nov 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Server Issues & PRs related to the Server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants