[Open Data] Sort instance size flexibility output ordinally so regeneration is host-independent - #2331
Open
Roland Krummenacher (RolandKrummenacher) wants to merge 2 commits into
Open
[Open Data] Sort instance size flexibility output ordinally so regeneration is host-independent#2331Roland Krummenacher (RolandKrummenacher) wants to merge 2 commits into
Roland Krummenacher (RolandKrummenacher) wants to merge 2 commits into
Conversation
…ependent The generator sorted with a bare Sort-Object, whose comparer follows the current culture. Culture-aware collation orders "Dasv4 Series" before "DCadsv5-series DedicatedHost" while an ordinal comparison puts the uppercase letter first, so the same records regenerated on a differently configured host reorder a few hundred rows without changing a value. A dispatch of the Update Instance Size Flexibility workflow after #2308 reproduced the published dataset exactly -- same 1,363 rows, same groups, same ratios -- and still pushed a 247-line diff that was pure reordering. Left alone, every scheduled run would push that diff whether or not any ISF data changed, burying real changes in review. #2308 pinned InvariantCulture on both parse sites and the write path but left the sort key culture-dependent. This applies the same treatment there, via an explicit [string]::CompareOrdinal comparer. Ordinal ordering is the order already committed, so InstanceSizeFlexibility.csv is unchanged. Adds four tests: group ordering, SKU ordering within a group, ordering held under de-DE, and an assertion that the published file is in ordinal order, which catches a file regenerated by hand on a differently configured host. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Roland Krummenacher (RolandKrummenacher)
requested review from
Brett Wilson (MSBrett) and
Michael Flanakin (flanakin)
as code owners
September 16, 2026 16:27
microsoft-github-policy-service
Bot
added
the
Needs: Review 👀
Copilot started reviewing on behalf of
Roland Krummenacher (RolandKrummenacher)
September 16, 2026 16:27
View session
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved blocking issues were identified.
Pull request overview
This pull request makes ISF CSV regeneration deterministic across host cultures using ordinal sorting, without changing the dataset.
Changes:
- Applies ordinal sorting to groups and SKUs.
- Adds culture-independent ordering and published-file validation tests.
File summaries
| File | Summary |
|---|---|
src/scripts/Update-InstanceSizeFlexibility.ps1 |
Applies deterministic ordinal sorting before export. |
src/powershell/Tests/Unit/Update-InstanceSizeFlexibility.Tests.ps1 |
Tests sorting behavior across cultures and published-file ordering. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Michael Flanakin (flanakin)
left a comment
Michael Flanakin (flanakin)
left a comment
Collaborator
There was a problem hiding this comment.
🤖 [AI][Claude] PR Review
Summary: The fix is correct and focused. The 28 tests in the ISF suite pass locally, and reverting the script to Sort-Object makes 3 of the new tests fail, so they guard the change. No blockers or should-fix items.
💡 Suggestions (2)
List.Sortis unstable, unlikeSort-Object. Ties on group + SKU could reorder between runs with-Raw.- The comparer is duplicated in the script and the published-file test. Note that they must stay in sync.
Addresses review feedback on the ISF ordinal sort: - Record why List.Sort's instability cannot surface here: $seen is keyed by flexibility group + ArmSkuName and a duplicate ArmSkuName across groups throws, so the two sort keys order every record uniquely in the normalized and the -Raw path alike. No tiebreaker is reachable. - Note that the comparer in the published-file order test is a copy of the script's and has to stay in sync with it. Comments only; no behavior change. 28/28 ISF tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
microsoft-github-policy-service
Bot
requested a review
from Michael Flanakin (flanakin)
September 21, 2026 07:08
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #2308. The ISF generator sorted with a bare
Sort-Object, whose comparer follows the current culture, so the same records regenerated on a differently configured host come out in a different order.InstanceSizeFlexibility.csvis unchanged — ordinal ordering is the order already committed ondev. This fixes only the script that produces it.How it surfaced
Dispatching Update Instance Size Flexibility after #2308 merged (run 35117329813) succeeded in 4m23s and reproduced the published dataset exactly — same 1,363 rows, same 213 groups, same ratios, verified as a set with line endings normalized. It still pushed a 247-insertion / 247-deletion diff, every line of it pure reordering.
Measured on a Windows host at
en-CH:DCadsv5-series DedicatedHostvsDasv4 SeriesDasv4firstDCadsv5firstBS Series High MemoryvsBasv2 SeriesBasv2firstBS SeriesfirstCulture-aware collation interleaves case; ordinal puts the uppercase letter first. The file committed on
devis in ordinal order while the script emitted culture-aware order, so every scheduled run would have pushed this same diff whether or not any ISF data changed — burying real changes in review, and flipping back and forth as the generator is run by hand on Windows per the README and onubuntu-latestby the workflow.The fix
#2308 pinned
InvariantCultureon both parse sites and the write path but left the sort key culture-dependent. This applies the same treatment there, with an explicit[string]::CompareOrdinalcomparer.One behavioral note:
Sort-Objectsilently tolerated a null record set, whichList[object]does not. An empty sweep is guarded explicitly — two existing tests (Queries VirtualMachines, RedisCache, and DedicatedHost but not BlockBlob by defaultandDoes not enumerate regions when -Location is supplied) caught that on the first revision.Test plan
Invoke-Pester ./src/powershell/Tests/Unit/* ./src/powershell/Tests/Lint/*— 6,023 passed, 0 failed (4 pre-existing skips), exactly +4 over the baseline of 6,019de-DE, and an assertion that the published file is in ordinal order — the last catches a file regenerated by hand on a differently configured hostInstanceSizeFlexibility.csvis byte-identical todevNo changelog entry: the ISF dataset is still under Unreleased → Added as a new dataset, and row ordering is not a documented property of it. Nothing user-visible changes.
🤖 Generated with Claude Code