Repository navigation
fix(clusters): keep the chosen region when a second region is removed on a single-region tier - #1842
Draft
dawsontoth wants to merge 2 commits into
Draft
dawsontoth wants to merge 2 commits into
dawsontoth wants to merge 2 commits into
Conversation
…ects it A tier with allowedRegionIds (the free tier) takes a single region, and the form corrects that region whenever it is the only one left. The correction always jumped to the default US region, so removing the second region, which is what the tier's "only one region" error asks the user to do, also threw away the first region and its latency. Prefer a latency the tier allows in the region the user chose, and fall back to the default region only when the tier offers nothing there. Refs #1275 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… regions Removing the first row shifts the survivor to index 0 before the correction runs, so pin that it keeps its region there too. Refs #1275 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the region correction logic in ClusterForm to prioritize keeping the user's chosen region when correcting regions on a tier that limits allowed regions, falling back to 'US' or the first available region only if the chosen region is not offered. It also adds corresponding documentation in DESIGN.md and a comprehensive suite of unit tests in ClusterForm.test.tsx to verify this behavior. There are no review comments to address.
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||
This was referenced Oct 11, 2026
Draft
dawsontoth
added this pull request to stack #1907
October 11, 2026 21:26
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.
⊙ Problem
On a tier that allows only one region (the free tier, which carries
allowedRegionIds), removing the second region of a new cluster threw away the first region too: it snapped back to the default US region and its smallest latency (#1275). The free tier's own validation error on the second row ("You can only select one region with this performance tier!") is what asks the user to remove it, so the natural fix-up step is the one that loses their choice.Reproduced on current
stagein a real browser (Playwright against a local dev server with mocked API responses): pick Medium, choose Europe 45ms for region 1, add a region, switch to Free, remove region 2. Region 1 readUS / 90ms, small distributionafterwards.The cause is
autoSelectRegionBasedOnAllowedRegionIdsinClusterForm.tsx. It corrects the only remaining region whenever that region is not one the tier allows, and it always corrected to US. Removing the second row is what drops the count to one and fires it. The triage suspect,autoPickLatencyDescriptioninRegionFormInputs.tsx, is not involved: rows are keyed byfield.idand removing a row leaves the other row's region and latency untouched on tiers without a region allow-list (pinned by a test below).💡 Solution
The correction now prefers an allowed entry in the region the user already chose, and falls back to US, then the first allowed region, only when the tier offers nothing there.
🔧 Changes
src/features/clusters/upsert/ClusterForm.tsx— the one-line change inautoSelectRegionBasedOnAllowedRegionIds: afindon the surviving entry'sregionNameruns before the existing US and first-allowed fallbacks. Nothing else in the effect, including when it runs, changed.src/features/clusters/DESIGN.md— records when the correction runs (including on removal) and the same-region-first rule.PR #1780 (custom regions) rewrites this effect to work on
regionId; it keeps the US-first selection, so it will need the same preference expressed as "an allowed id whose catalog entry has the current entry's name" when it rebases. The two do not otherwise touch the same lines.✅ Verification
Route: new component test driving the real
ClusterForm(react-hook-form, zod resolver, Radix selects, the real Remove button) in jsdom, plus a live browser repro.src/features/clusters/upsert/ClusterForm.test.tsx(new) — five cases, reading each row's value from the hidden native selects Radix renders: removing a row on an unrestricted tier leaves the survivor untouched; on the free tier the survivor keeps its region whether row 2 or row 1 was removed ([Create Cluster] Deleting the second region will also revert the first chosen region and latency #1275); an already-allowed survivor is untouched; and a survivor in a region the tier does not offer still falls back to US.expected [ 'US / 90ms, small distribution' ] to deeply equal [ 'Europe / 60ms, small distribution' ]), the other three stay green.npx vitest run src/features/clusters/upsert/ClusterForm.test.tsx→ 5 passed (exit 0). Full gate via the pre-commit hook: vitest 396 files / 3668 tests passed,oxlintclean,dprint check --stagedclean, commitlint ok;npx tsc -bexit 0.Europe / 60ms, small distribution; the four unrestricted-tier removal scenarios were unchanged before and after.Cross-model review: two rounds, codex (graded) and gemini both ran. Their findings were rejected on evidence: a crash if
firstRegionis undefined (the effect only runs when exactly one region exists, and the Remove button only renders with two or more), and explicit import extensions (this repo's imports are extensionless throughout). The Cursor leg could not fetch (1Password SSH agent refused to sign) and the Harper domain adjudicator failed authentication, so the outside findings were triaged by hand rather than adjudicated.Closes #1275
🤖 Generated by Anthropic Claude Code (Claude Opus 5.5); posted via @dawsontoth.
🤖 Generated with Claude Code
Related PRs: #1780 overlaps (rewrites the same correction effect for region ids)
Complexity: easy
Review-Coverage: authored=claude; ran=codex,gemini; blocked=cursor-composer(no-receipt),domain(auth); declined=cursor-grok,cursor-kimi,cursor-muse; rounds=2; full=1 @ fe4bbe8
Review-Attention: skim ~2m (raised: degraded review) @ fe4bbe8