Skip to content

Sort sequences in sort_maps to fix non-deterministic HashSet snapshots#876

Merged
max-sixty merged 1 commit intomitsuhiko:masterfrom
max-sixty:sorted-hashset
Feb 13, 2026
Merged

Sort sequences in sort_maps to fix non-deterministic HashSet snapshots#876
max-sixty merged 1 commit intomitsuhiko:masterfrom
max-sixty:sorted-hashset

Conversation

@max-sixty
Copy link
Copy Markdown
Collaborator

Summary

  • sort_maps only sorted Content::Map, but HashSet serializes as Content::Seq via serde, leaving snapshot output non-deterministic across recompilations
  • Extend sort_maps() to also sort Content::Seq items using the same key-based comparison
  • Extract shared cmp_as_key helper to deduplicate comparison logic between Map and Seq sorting
  • Update set_sort_maps / sort_maps doc comments to reflect the expanded scope

Closes #211

Test plan

  • Added test_sort_maps_sorts_hashset verifying both HashSet and HashMap produce deterministic sorted output
  • All existing tests pass with --all-features

This was written by Claude Code on behalf of max-sixty

…shots

`sort_maps` only sorted `Content::Map` but `HashSet` serializes as
`Content::Seq`, leaving its snapshot output non-deterministic across
recompilations. Extend `sort_maps` to also sort `Content::Seq` items.

Closes mitsuhiko#211

Co-Authored-By: Claude <noreply@anthropic.com>
@max-sixty max-sixty merged commit ebe5a6e into mitsuhiko:master Feb 13, 2026
15 checks passed
@max-sixty max-sixty mentioned this pull request Mar 27, 2026
max-sixty added a commit that referenced this pull request Mar 27, 2026
## Summary

- Bump version to 1.47.0
- Update CHANGELOG

### Changes included in this release

- Add `Comparator` trait for customizing how snapshot values are
compared. #872 (@dstu)
- Sort sequences in `sort_maps` to fix non-deterministic `HashSet`
snapshots. #876
- Improve TOML serialization error message for unsupported types. #880
- Remove unnecessary `Send + Sync` bounds from `Redaction`, allowing
non-`Send` closures in dynamic redactions. #874
- Don't use `Arc` in `Settings` unnecessarily. #873 (@dstu)
- Upgrade `console` to 0.16 and MSRV to 1.66. #885
- Upgrade `toml-edit` to 0.25. #882 (@alexanderkjall)

> _This was written by Claude Code on behalf of max-sixty_

Co-authored-by: Claude <noreply@anthropic.com>
@spinda
Copy link
Copy Markdown

spinda commented Mar 28, 2026

This isn't just scoped to HashSets, it's sorting all Seqs now, including Vecs. This is a massive breaking change that makes sort_maps unusable in many situations.

@max-sixty
Copy link
Copy Markdown
Collaborator Author

I will revert

max-sixty added a commit to max-sixty/insta that referenced this pull request Mar 29, 2026
max-sixty added a commit to max-sixty/insta that referenced this pull request Mar 29, 2026
Reverts mitsuhiko#876 (sort sequences in `sort_maps`) which was a breaking change —
it sorted all `Seq` values including `Vec`, not just `HashSet`.

Co-Authored-By: Claude <noreply@anthropic.com>
max-sixty added a commit that referenced this pull request Mar 29, 2026
Reverts #876 — the `sort_maps` change in 1.47.0 sorted all `Seq` values
(including `Vec`), not just non-deterministic collections like
`HashSet`. This was a breaking change for anyone relying on ordered
sequence output with `sort_maps` enabled.

Thanks to @spinda for reporting in #876.

> _This was written by Claude Code on behalf of max-sixty_

---------

Co-authored-by: Claude <noreply@anthropic.com>
max-sixty added a commit to max-sixty/insta that referenced this pull request Mar 29, 2026
Ensures sort_maps only sorts map keys, not sequence values.
The bug in mitsuhiko#876 sorted all Seq values (including Vec), which
broke snapshot stability for ordered collections.

Co-Authored-By: Claude <noreply@anthropic.com>
max-sixty added a commit that referenced this pull request Mar 29, 2026
Adds a regression test ensuring `sort_maps` only sorts map keys, not
sequence values. The bug in #876 sorted all `Seq` values (including
`Vec`), which broke snapshot stability for ordered collections.

The test snapshots a `HashMap` containing a `Vec` with `sort_maps`
enabled and asserts the Vec order is preserved.

> _This was written by Claude Code on behalf of max-sixty_

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

Deterministic sort order for HashSet (reopen issue #53)

2 participants