Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 15, 2026

Summary

Toolsnaps now write JSON with consistent alphabetical key ordering at all nesting levels, eliminating diff churn from Go's randomized map iteration.

Why

Toolsnap diffs showed field reordering noise even when schemas hadn't changed. Example from a typical regeneration:

 "order": {
-  "type": "string",
-  "description": "Sort order",
+  "description": "Sort order",
   "enum": ["asc", "desc"]
+  "type": "string"
 }

What changed

  • Added sortJSONKeys() in internal/toolsnaps/toolsnaps.go: unmarshals and remarshals JSON to leverage Go's built-in alphabetical map key sorting
  • Updated writeSnap() to sort before writing
  • Regenerated all 94 toolsnap files with consistent ordering
  • Added tests for sorting behavior and idempotency

MCP impact

  • No tool or API changes - Only affects snapshot file formatting, not runtime behavior or schemas

Prompts tested (tool changes only)

N/A

Security / limits

  • No security or limits impact - Internal testing infrastructure only

Tool renaming

  • I am not renaming tools as part of this PR

Lint & tests

  • Linted locally with ./script/lint
  • Tested locally with ./script/test

Docs

  • Not needed - Internal change to test infrastructure
Original prompt

Sort the fields of toolsnaps so that we never see noop churn of them before writing them. Might even need to be recursive, https://github.com/github/github-mcp-server/pull/1776/changes is a good example


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Sort all JSON object keys alphabetically at every level in toolsnaps by
unmarshaling and remarshaling. This leverages Go's built-in behavior where
json.Marshal automatically sorts map keys alphabetically, ensuring
consistent field ordering and eliminating noop churn in diffs.

Co-authored-by: SamMorrowDrums <[email protected]>
Copilot AI changed the title [WIP] Sort toolsnaps fields to avoid noop churn Sort toolsnaps JSON keys recursively to eliminate noop churn Jan 15, 2026
Copilot AI requested a review from SamMorrowDrums January 15, 2026 15:01
@SamMorrowDrums SamMorrowDrums marked this pull request as ready for review January 15, 2026 15:03
@SamMorrowDrums SamMorrowDrums requested a review from a team as a code owner January 15, 2026 15:03
Copilot AI review requested due to automatic review settings January 15, 2026 15:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses JSON key ordering instability in toolsnap files caused by Go's randomized map iteration. The solution adds a sortJSONKeys() function that recursively sorts all JSON object keys alphabetically by unmarshaling and remarshaling JSON data, leveraging Go's built-in alphabetical key sorting for maps.

Changes:

  • Added sortJSONKeys() function to recursively sort JSON keys at all nesting levels
  • Modified writeSnap() to sort JSON before writing
  • Regenerated all 94 toolsnap files with consistent alphabetical key ordering
  • Added comprehensive tests for sorting behavior and idempotency

Reviewed changes

Copilot reviewed 96 out of 96 changed files in this pull request and generated no comments.

File Description
internal/toolsnaps/toolsnaps.go Added sortJSONKeys() function and integrated it into writeSnap()
internal/toolsnaps/toolsnaps_test.go Added comprehensive test coverage for JSON key sorting and idempotency
pkg/github/toolsnaps/*.snap Regenerated all 94 toolsnap files with alphabetically sorted keys

@SamMorrowDrums SamMorrowDrums merged commit 87a1110 into main Jan 15, 2026
22 checks passed
@SamMorrowDrums SamMorrowDrums deleted the copilot/sort-toolsnaps-fields-recursively branch January 15, 2026 15:15
Copilot AI added a commit that referenced this pull request Jan 15, 2026
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.

3 participants