From e1f34ae81b5b50f37d30b2c3a561191b2bad9a2f Mon Sep 17 00:00:00 2001 From: Mikhail Chusavitin Date: Tue, 19 May 2026 12:36:55 +0300 Subject: [PATCH] fix: compressArticle used hard-coded indices, PSU rendered as NIC when GPU absent compressArticle assumed a fixed segment layout [model,cpu,mem,gpu,disk,net,psu,support]. Build() skips empty segments, so without GPU the PSU slot shifted to index 5. Step 2 of compression called compressNetSegment on the PSU value ("2x1kW") which returned "2xNIC". Replace positional indexing with namedSeg{group,value} tagged segments; compressArticle now looks up each segment by group name via findSegGroup(), regardless of array length. Regression test TestBuild_CompressArticle_NoGPU_PSUNotNIC reproduces the exact config from OPS-2445 (NF5280M6 + 2xPSU, no GPU) that triggered the bug. Co-Authored-By: Claude Sonnet 4.6 --- internal/article/generator.go | 102 ++++++++++++++++------------- internal/article/generator_test.go | 73 +++++++++++++++++++++ 2 files changed, 129 insertions(+), 46 deletions(-) diff --git a/internal/article/generator.go b/internal/article/generator.go index 51a34f1..1a63742 100644 --- a/internal/article/generator.go +++ b/internal/article/generator.go @@ -22,24 +22,29 @@ type BuildResult struct { } var ( - reMemGiB = regexp.MustCompile(`(?i)(\d+)\s*(GB|G)`) - reMemTiB = regexp.MustCompile(`(?i)(\d+)\s*(TB|T)`) + reMemGiB = regexp.MustCompile(`(?i)(\d+)\s*(GB|G)`) + reMemTiB = regexp.MustCompile(`(?i)(\d+)\s*(TB|T)`) reCapacityT = regexp.MustCompile(`(?i)(\d+(?:[.,]\d+)?)T`) reCapacityG = regexp.MustCompile(`(?i)(\d+(?:[.,]\d+)?)G`) rePortSpeed = regexp.MustCompile(`(?i)(\d+)p(\d+)(GbE|G)`) - rePortFC = regexp.MustCompile(`(?i)(\d+)pFC(\d+)`) - reWatts = regexp.MustCompile(`(?i)(\d{3,5})\s*W`) + rePortFC = regexp.MustCompile(`(?i)(\d+)pFC(\d+)`) + reWatts = regexp.MustCompile(`(?i)(\d{3,5})\s*W`) ) +type namedSeg struct { + group string // "MODEL","CPU","MEM","GPU","DISK","NET","PSU","SUPPORT" + value string +} + func Build(local *localdb.LocalDB, items []models.ConfigItem, opts BuildOptions) (BuildResult, error) { - segments := make([]string, 0, 8) + segs := make([]namedSeg, 0, 8) warnings := make([]string, 0) model := NormalizeServerModel(opts.ServerModel) if model == "" { return BuildResult{}, fmt.Errorf("server_model required") } - segments = append(segments, model) + segs = append(segs, namedSeg{"MODEL", model}) lotNames := make([]string, 0, len(items)) for _, it := range items { @@ -55,41 +60,39 @@ func Build(local *localdb.LocalDB, items []models.ConfigItem, opts BuildOptions) return BuildResult{}, err } - cpuSeg := buildCPUSegment(items, cats) - if cpuSeg != "" { - segments = append(segments, cpuSeg) + if cpuSeg := buildCPUSegment(items, cats); cpuSeg != "" { + segs = append(segs, namedSeg{"CPU", cpuSeg}) } memSeg, memWarn := buildMemSegment(items, cats) if memWarn != "" { warnings = append(warnings, memWarn) } if memSeg != "" { - segments = append(segments, memSeg) + segs = append(segs, namedSeg{"MEM", memSeg}) } - gpuSeg := buildGPUSegment(items, cats) - if gpuSeg != "" { - segments = append(segments, gpuSeg) + if gpuSeg := buildGPUSegment(items, cats); gpuSeg != "" { + segs = append(segs, namedSeg{"GPU", gpuSeg}) } diskSeg, diskWarn := buildDiskSegment(items, cats) if diskWarn != "" { warnings = append(warnings, diskWarn) } if diskSeg != "" { - segments = append(segments, diskSeg) + segs = append(segs, namedSeg{"DISK", diskSeg}) } netSeg, netWarn := buildNetSegment(items, cats) if netWarn != "" { warnings = append(warnings, netWarn) } if netSeg != "" { - segments = append(segments, netSeg) + segs = append(segs, namedSeg{"NET", netSeg}) } psuSeg, psuWarn := buildPSUSegment(items, cats) if psuWarn != "" { warnings = append(warnings, psuWarn) } if psuSeg != "" { - segments = append(segments, psuSeg) + segs = append(segs, namedSeg{"PSU", psuSeg}) } if strings.TrimSpace(opts.SupportCode) != "" { @@ -97,12 +100,12 @@ func Build(local *localdb.LocalDB, items []models.ConfigItem, opts BuildOptions) if !isSupportCodeValid(code) { return BuildResult{}, fmt.Errorf("invalid_support_code") } - segments = append(segments, code) + segs = append(segs, namedSeg{"SUPPORT", code}) } - article := strings.Join(segments, "-") + article := strings.Join(namedSegsValues(segs), "-") if len([]rune(article)) > 80 { - article = compressArticle(segments) + article = compressArticle(segs) warnings = append(warnings, "compressed") } if len([]rune(article)) > 80 { @@ -112,6 +115,23 @@ func Build(local *localdb.LocalDB, items []models.ConfigItem, opts BuildOptions) return BuildResult{Article: article, Warnings: warnings}, nil } +func namedSegsValues(segs []namedSeg) []string { + out := make([]string, len(segs)) + for i, s := range segs { + out[i] = s.value + } + return out +} + +func findSegGroup(segs []namedSeg, group string) int { + for i, s := range segs { + if s.group == group { + return i + } + } + return -1 +} + func isSupportCodeValid(code string) bool { if len(code) < 3 { return false @@ -484,60 +504,50 @@ func atoi(v string) int { return n } -func compressArticle(segments []string) string { - if len(segments) == 0 { +func compressArticle(segs []namedSeg) string { + if len(segs) == 0 { return "" } - normalized := make([]string, 0, len(segments)) - for _, s := range segments { - normalized = append(normalized, strings.ReplaceAll(s, "GbE", "G")) + for i, s := range segs { + segs[i].value = strings.ReplaceAll(s.value, "GbE", "G") } - segments = normalized - article := strings.Join(segments, "-") + article := strings.Join(namedSegsValues(segs), "-") if len([]rune(article)) <= 80 { return article } - // segment order: model, cpu, mem, gpu, disk, net, psu, support - index := func(i int) (int, bool) { - if i >= 0 && i < len(segments) { - return i, true - } - return -1, false - } - // 1) remove PSU - if i, ok := index(6); ok { - segments = append(segments[:i], segments[i+1:]...) - article = strings.Join(segments, "-") + if i := findSegGroup(segs, "PSU"); i >= 0 { + segs = append(segs[:i], segs[i+1:]...) + article = strings.Join(namedSegsValues(segs), "-") if len([]rune(article)) <= 80 { return article } } // 2) compress NET/HBA/HCA - if i, ok := index(5); ok { - segments[i] = compressNetSegment(segments[i]) - article = strings.Join(segments, "-") + if i := findSegGroup(segs, "NET"); i >= 0 { + segs[i].value = compressNetSegment(segs[i].value) + article = strings.Join(namedSegsValues(segs), "-") if len([]rune(article)) <= 80 { return article } } // 3) compress DISK - if i, ok := index(4); ok { - segments[i] = compressDiskSegment(segments[i]) - article = strings.Join(segments, "-") + if i := findSegGroup(segs, "DISK"); i >= 0 { + segs[i].value = compressDiskSegment(segs[i].value) + article = strings.Join(namedSegsValues(segs), "-") if len([]rune(article)) <= 80 { return article } } // 4) compress GPU to vendor only (GPU_NV) - if i, ok := index(3); ok { - segments[i] = compressGPUSegment(segments[i]) + if i := findSegGroup(segs, "GPU"); i >= 0 { + segs[i].value = compressGPUSegment(segs[i].value) } - return strings.Join(segments, "-") + return strings.Join(namedSegsValues(segs), "-") } func compressNetSegment(seg string) string { diff --git a/internal/article/generator_test.go b/internal/article/generator_test.go index 10ed01a..90a3d78 100644 --- a/internal/article/generator_test.go +++ b/internal/article/generator_test.go @@ -61,6 +61,79 @@ func TestBuild_ParsesNetAndPSU(t *testing.T) { } } +// TestBuild_CompressArticle_NoGPU_PSUNotNIC reproduces the bug where 2 PSUs produced +// "2xNIC" in the article because compressArticle used hard-coded indices that assumed +// GPU was always present. +func TestBuild_CompressArticle_NoGPU_PSUNotNIC(t *testing.T) { + local, err := localdb.New(filepath.Join(t.TempDir(), "local.db")) + if err != nil { + t.Fatalf("init local db: %v", err) + } + t.Cleanup(func() { _ = local.Close() }) + + if err := local.SaveLocalPricelist(&localdb.LocalPricelist{ + ServerID: 2, + Source: "estimate", + Version: "S-2026-05-19-001", + Name: "test", + CreatedAt: time.Now(), + SyncedAt: time.Now(), + }); err != nil { + t.Fatalf("save local pricelist: %v", err) + } + localPL, err := local.GetLocalPricelistByServerID(2) + if err != nil { + t.Fatalf("get local pricelist: %v", err) + } + + if err := local.SaveLocalPricelistItems([]localdb.LocalPricelistItem{ + {PricelistID: localPL.ID, LotName: "CPU_INTEL_8358", LotCategory: "CPU", Price: 1}, + {PricelistID: localPL.ID, LotName: "MEM_DDR4_64G_3200", LotCategory: "MEM", Price: 1}, + {PricelistID: localPL.ID, LotName: "SSD_SATA_0.4T", LotCategory: "SSD", Price: 1}, + {PricelistID: localPL.ID, LotName: "SSD_SATA_0.9T", LotCategory: "SSD", Price: 1}, + {PricelistID: localPL.ID, LotName: "HDD_SATA_16T", LotCategory: "HDD", Price: 1}, + {PricelistID: localPL.ID, LotName: "NIC_2p25G_MCX512A", LotCategory: "NIC", Price: 1}, + {PricelistID: localPL.ID, LotName: "HBA_2pFC32_Gen6", LotCategory: "HBA", Price: 1}, + {PricelistID: localPL.ID, LotName: "NIC_4p1G_I350", LotCategory: "NIC", Price: 1}, + {PricelistID: localPL.ID, LotName: "PS_1500W_Platinum", LotCategory: "PS", Price: 1}, + }); err != nil { + t.Fatalf("save local items: %v", err) + } + + // PS_1500W → "2x1.5kW" (7 chars) brings uncompressed article to 81 chars, triggering + // compressArticle. Before the fix, compressArticle used hard-coded index 5 for NET, but + // without GPU the PSU sits at index 5, so compressNetSegment("2x1.5kW") returned "2xNIC". + items := models.ConfigItems{ + {LotName: "CPU_INTEL_8358", Quantity: 2}, + {LotName: "MEM_DDR4_64G_3200", Quantity: 16}, // 1024 GiB = 1T + {LotName: "SSD_SATA_0.4T", Quantity: 2}, + {LotName: "SSD_SATA_0.9T", Quantity: 4}, + {LotName: "HDD_SATA_16T", Quantity: 6}, + {LotName: "NIC_2p25G_MCX512A", Quantity: 1}, + {LotName: "HBA_2pFC32_Gen6", Quantity: 1}, + {LotName: "NIC_4p1G_I350", Quantity: 1}, + {LotName: "PS_1500W_Platinum", Quantity: 2}, + } + result, err := Build(local, items, BuildOptions{ + ServerModel: "NF5280M6", + ServerPricelist: &localPL.ServerID, + }) + if err != nil { + t.Fatalf("build article: %v", err) + } + if len([]rune(result.Article)) > 80 { + t.Fatalf("article too long (%d): %s", len([]rune(result.Article)), result.Article) + } + // PSU segment must not be mis-labeled as NIC during compression + // The correct behaviour: PSU is dropped, NET stays as-is or compressed to HBA/NIC labels + // Before the fix: article ended with "-2xNIC" (PSU turned into NIC) + // After the fix: article must not contain a standalone "NIC" that came from PSU wattage + if strings.HasSuffix(result.Article, "-2xNIC") { + t.Fatalf("PSU mis-labeled as NIC in article: %s", result.Article) + } + t.Logf("article: %s (warnings: %v)", result.Article, result.Warnings) +} + func contains(s, sub string) bool { return strings.Contains(s, sub) }