Disambiguate stacks in interactive selector#1910
Conversation
📝 WalkthroughWalkthroughThe PR refactors stack selection label generation to display provider-region information alongside stack names. A new public function Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)level=warning msg="[linters_context] running gomodguard failed: unable to read module file go.mod: current working directory must have a go.mod file: if you are not using go modules it is suggested to disable this linter" Comment |
jordanstephens
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/pkg/stacks/selector.go`:
- Around line 128-138: MakeStackSelectorLabels can drop a stack when two
different stacks produce the same formatted label; update
MakeStackSelectorLabels to detect label collisions from
formatStackLabelParts(parts) and handle them instead of overwriting: check if
labelMap already has the label, and on collision either (A) append a
disambiguating suffix using a unique field from the corresponding ListItem
(e.g., stacks[i].Name or an ID) or index, or (B) change the map value to a slice
and push both stacks' identifiers so multiple stacks are selectable; ensure you
use stackLabelParts, reduceStackLabelParts, formatStackLabelParts unchanged and
only modify labelMap population logic in MakeStackSelectorLabels to preserve
uniqueness or collect collisions.
🧹 Nitpick comments (2)
src/pkg/stacks/selector.go (1)
59-67: Inefficient O(n²) lookup to preserve stack ordering.The nested loop iterating over
labelMapfor each stack results in O(n²) complexity. Since maps in Go have non-deterministic iteration order, this approach is needed to preserve the original stack ordering, but it's inefficient.Consider building the labels in order directly:
♻️ Proposed refactor to build labels in order
- labelMap := MakeStackSelectorLabels(stackList) - stackLabels := make([]string, 0, len(stackList)+1) - stackNames := make([]string, 0, len(stackList)) - for _, stack := range stackList { - for label, name := range labelMap { - if name == stack.Name { - stackLabels = append(stackLabels, label) - stackNames = append(stackNames, name) - break - } - } - } + labelMap := MakeStackSelectorLabels(stackList) + stackLabels := make([]string, 0, len(stackList)+1) + stackNames := make([]string, 0, len(stackList)) + // Build a reverse map for O(1) lookup + nameToLabel := make(map[string]string, len(labelMap)) + for label, name := range labelMap { + nameToLabel[name] = label + } + for _, stack := range stackList { + if label, ok := nameToLabel[stack.Name]; ok { + stackLabels = append(stackLabels, label) + stackNames = append(stackNames, stack.Name) + } + }Alternatively, consider changing
MakeStackSelectorLabelsto return both the label slice (in order) and the map, avoiding the need for reverse lookup.src/pkg/stacks/selector_test.go (1)
472-539: Good test coverage, consider adding edge cases.The test cases cover the main label reduction scenarios well. Consider adding tests for:
- DeployedAt formatting: Verify that
DeployedAtis included in labels when set- Label collision: Test behavior when stacks could produce identical labels
📝 Suggested additional test cases
{ name: "includes deployed date", stacks: []ListItem{ { Parameters: Parameters{Name: "production", Provider: "aws", Region: "us-west-2"}, DeployedAt: time.Date(2026, 1, 15, 0, 0, 0, 0, time.UTC), }, }, wantLabels: []string{"production (aws, us-west-2, last deployed Jan 15 2026)"}, }, { name: "hides redundant deployed date when all same", stacks: []ListItem{ { Parameters: Parameters{Name: "prod", Provider: "aws", Region: "us-west-2"}, DeployedAt: time.Date(2026, 1, 15, 0, 0, 0, 0, time.UTC), }, { Parameters: Parameters{Name: "dev", Provider: "aws", Region: "us-east-1"}, DeployedAt: time.Date(2026, 1, 15, 0, 0, 0, 0, time.UTC), }, }, wantLabels: []string{"prod (us-west-2)", "dev (us-east-1)"}, },
2 participants
Description
I have a lot of stacks, and though I try to name them descriptively, sometimes, I forget which is which. This PR aims to add some extra info to the interactive stack selection list by printing stack provider, region and last deployed time after the name. If all of the providers or regions are the same, they won't be printed.
With many providers and regions, everything is printed
With the same provider, only region is printed
Linked Issues
Checklist
Summary by CodeRabbit
Release Notes
New Features
Tests