From 2c3072cf102d902ac21381c1c6fab739cb074908 Mon Sep 17 00:00:00 2001 From: Mikhail Chusavitin Date: Mon, 17 Aug 2026 11:05:19 +0300 Subject: [PATCH] fix(exporter): fix memory dedup collapsing distinct DIMMs, disambiguate slots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dedupeMemory keyed on slot before serial, unlike dedupeStorage/dedupePSUs which both prefer serial. Some collectors (observed on MSI CG480-S6053 with older BEE-SP versions — confirmed reproducible across multiple servers, fixed in newer BEE-SP versions) report every memory module at the same slot label ("DIMM 0") even though each has a distinct real serial. Slot-first keying collapsed all of them into a single record on export — 16 real 32GB DIMMs (512GB) reduced to 1, discarding 15 physical modules' worth of data. Serial now takes priority, matching storage/PSU. Also add disambiguateMemorySlots (same pattern as the existing PSU slot fix): when multiple already-distinct DIMMs still share a slot label, renumber them to "DIMM {n}" by ascending serial order so Reanimator can track them by position and results are deterministic across re-imports. Updated the existing dedup test (was asserting the old collapse-by-slot behavior on a same-slot/different-serial case, which was exactly the bug) and added a dedicated regression test for the collision case. Co-Authored-By: Claude Sonnet 5 --- internal/exporter/reanimator_converter.go | 41 ++++++++++++++++- .../exporter/reanimator_converter_test.go | 46 ++++++++++++++++++- 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/internal/exporter/reanimator_converter.go b/internal/exporter/reanimator_converter.go index 34f736d..db9ac69 100644 --- a/internal/exporter/reanimator_converter.go +++ b/internal/exporter/reanimator_converter.go @@ -46,7 +46,7 @@ func ConvertToReanimator(result *models.AnalysisResult) (*ReanimatorExport, erro Board: convertBoard(result.Hardware.BoardInfo), Firmware: dedupeFirmware(convertFirmware(result.Hardware.Firmware)), CPUs: dedupeCPUs(convertCPUsFromDevices(devices, collectedAt, result.Hardware.BoardInfo.SerialNumber, buildCPUMicrocodeBySocket(result.Hardware.Firmware))), - Memory: dedupeMemory(convertMemoryFromDevices(devices, collectedAt)), + Memory: disambiguateMemorySlots(dedupeMemory(convertMemoryFromDevices(devices, collectedAt))), Storage: dedupeStorage(convertStorageFromDevices(devices, collectedAt)), PCIeDevices: dedupePCIe(convertPCIeFromDevices(devices, collectedAt)), PowerSupplies: disambiguatePSUSlots(dedupePSUs(convertPSUsFromDevices(devices, collectedAt))), @@ -2130,7 +2130,15 @@ func dedupeMemory(items []ReanimatorMemory) []ReanimatorMemory { seen := make(map[string]struct{}, len(items)) result := make([]ReanimatorMemory, 0, len(items)) for _, item := range items { - key := strings.ToLower(strings.TrimSpace(item.Slot)) + // Serial identifies one physical DIMM; some collectors (observed on + // MSI CG480-S6053 with older BEE-SP versions) report every module at + // the same slot label ("DIMM 0"). Keying on slot first would collapse + // all of them into one record and silently drop the rest even though + // each has a distinct, real serial number. + key := strings.ToLower(strings.TrimSpace(item.SerialNumber)) + if key == "" { + key = strings.ToLower(strings.TrimSpace(item.Slot)) + } if key == "" { key = strings.ToLower(strings.TrimSpace(item.Location)) } @@ -2215,6 +2223,35 @@ func disambiguatePSUSlots(items []ReanimatorPSU) []ReanimatorPSU { return items } +// disambiguateMemorySlots renumbers memory slots when two or more distinct +// DIMMs (different serials, already survived dedupeMemory) report the same +// slot string from the source — observed on MSI CG480-S6053 with older +// BEE-SP versions, which label every module "DIMM 0" regardless of its real +// channel/slot. As with disambiguatePSUSlots, positions within a colliding +// group are assigned by ascending serial number for a deterministic result. +func disambiguateMemorySlots(items []ReanimatorMemory) []ReanimatorMemory { + if len(items) < 2 { + return items + } + groups := make(map[string][]int, len(items)) + for i, it := range items { + key := strings.ToLower(strings.TrimSpace(it.Slot)) + groups[key] = append(groups[key], i) + } + for _, idxs := range groups { + if len(idxs) < 2 { + continue + } + sort.Slice(idxs, func(a, b int) bool { + return strings.ToLower(items[idxs[a]].SerialNumber) < strings.ToLower(items[idxs[b]].SerialNumber) + }) + for pos, idx := range idxs { + items[idx].Slot = fmt.Sprintf("DIMM %d", pos) + } + } + return items +} + func dedupePCIe(items []ReanimatorPCIe) []ReanimatorPCIe { if len(items) < 2 { return items diff --git a/internal/exporter/reanimator_converter_test.go b/internal/exporter/reanimator_converter_test.go index 8dbfc2a..13add30 100644 --- a/internal/exporter/reanimator_converter_test.go +++ b/internal/exporter/reanimator_converter_test.go @@ -946,7 +946,7 @@ func TestConvertToReanimator_DeduplicatesAllSections(t *testing.T) { }, Memory: []models.MemoryDIMM{ {Slot: "DIMM_A1", Present: true, SizeMB: 32768, SerialNumber: "MEM-1", Status: "OK"}, - {Slot: "DIMM_A1", Present: true, SizeMB: 32768, SerialNumber: "MEM-1-DUP", Status: "OK"}, + {Slot: "DIMM_A1", Present: true, SizeMB: 32768, SerialNumber: "MEM-1", Status: "OK"}, }, Storage: []models.Storage{ {Slot: "U.2-1", SerialNumber: "SSD-1", Model: "Disk1", Present: true}, @@ -979,7 +979,7 @@ func TestConvertToReanimator_DeduplicatesAllSections(t *testing.T) { t.Fatalf("expected cpus len=1 after socket dedupe, got %d", len(out.Hardware.CPUs)) } if len(out.Hardware.Memory) != 1 { - t.Fatalf("expected memory len=1 after slot dedupe, got %d", len(out.Hardware.Memory)) + t.Fatalf("expected memory len=1 after serial dedupe, got %d", len(out.Hardware.Memory)) } if len(out.Hardware.Storage) != 1 { t.Fatalf("expected deduped storage len=1, got %d", len(out.Hardware.Storage)) @@ -1002,6 +1002,48 @@ func TestConvertToReanimator_DeduplicatesAllSections(t *testing.T) { } } +// TestConvertToReanimator_MemorySlotCollisionPreservesDistinctModules covers +// the real-world case observed on MSI CG480-S6053 with older BEE-SP versions: +// the source reports every DIMM at the same slot label ("DIMM 0") even though +// each module has its own distinct serial. Keying dedup on slot first would +// have collapsed all of them into a single record. +func TestConvertToReanimator_MemorySlotCollisionPreservesDistinctModules(t *testing.T) { + input := &models.AnalysisResult{ + Filename: "dimm-slot-collision.json", + CollectedAt: time.Date(2026, 2, 10, 15, 30, 0, 0, time.UTC), + Hardware: &models.HardwareConfig{ + BoardInfo: models.BoardInfo{SerialNumber: "BOARD-001"}, + Memory: []models.MemoryDIMM{ + {Slot: "DIMM 0", Present: true, SizeMB: 32768, SerialNumber: "MEM-AAA", Status: "OK"}, + {Slot: "DIMM 0", Present: true, SizeMB: 32768, SerialNumber: "MEM-ZZZ", Status: "OK"}, + {Slot: "DIMM 0", Present: true, SizeMB: 32768, SerialNumber: "MEM-CCC", Status: "OK"}, + }, + }, + } + + out, err := ConvertToReanimator(input) + if err != nil { + t.Fatalf("ConvertToReanimator() failed: %v", err) + } + + if len(out.Hardware.Memory) != 3 { + t.Fatalf("expected all 3 distinct DIMMs preserved, got %d", len(out.Hardware.Memory)) + } + bySerial := make(map[string]string, len(out.Hardware.Memory)) + slots := make(map[string]struct{}, len(out.Hardware.Memory)) + for _, m := range out.Hardware.Memory { + bySerial[m.SerialNumber] = m.Slot + slots[m.Slot] = struct{}{} + } + if len(slots) != 3 { + t.Fatalf("expected 3 distinct disambiguated slots, got %d: %v", len(slots), bySerial) + } + // Positions assigned by ascending serial order: MEM-AAA < MEM-CCC < MEM-ZZZ + if bySerial["MEM-AAA"] != "DIMM 0" || bySerial["MEM-CCC"] != "DIMM 1" || bySerial["MEM-ZZZ"] != "DIMM 2" { + t.Fatalf("unexpected slot assignment: %v", bySerial) + } +} + func TestConvertToReanimator_StatusFallbackUsesCollectedAt(t *testing.T) { collectedAt := time.Date(2026, 2, 10, 15, 30, 0, 0, time.UTC) input := &models.AnalysisResult{