fix: Topology Spec with the switch hierarchy

[ravisoundar]

No description provided.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.46%. Comparing base (35d7bee) to head (5578857).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #246   +/-   ##
=======================================
  Coverage   68.46%   68.46%           
=======================================
  Files          82       82           
  Lines        4842     4842           
=======================================
  Hits         3315     3315           
  Misses       1395     1395           
  Partials      132      132           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[greptile-apps]

Greptile Summary

This PR fixes topology spec generation for tree-based topologies by replacing the immediate-parent-only switch reference with the full ancestor switch hierarchy. Previously, nodeInfo.switchName held a single switch ID; now nodeInfo.switches holds the complete ordered path from the tree root down to the leaf's parent (e.g., default:S1:S2 instead of default:S2), and the same full-path approach is applied to per-partition topologies via a new parents map on TreeTopo/BlockTopo.

The lookup strategy in getTopologySpec was also improved from an O(N·M) linear scan (iterating switches and expanding node lists) to an O(depth) map lookup, which is a meaningful performance improvement.

Confidence Score: 5/5

Safe to merge — logic is correct, tests pass for all code paths including cluster-wide and per-partition topologies.

No P0/P1 issues found. The ancestor-chain construction in both initTree and getTreeTopologyUnit is correctly ordered and handles the empty-root-ID convention. The O(depth) map lookup in getTopologySpec is a strict improvement. All updated test expectations align with the traced execution paths.

No files require special attention.

Important Files Changed

Filename	Overview
pkg/translate/topology.go	Changes nodeInfo.switchName string to nodeInfo.switches []string and builds the full ancestor chain in initTree via a map[string][]string parentMap; logic is correct for both rooted (ID="") and non-rooted BFS traversal.
pkg/translate/yaml.go	Adds unexported parents maps to TreeTopo and BlockTopo (yaml:"-"), populates them during topology unit construction, and replaces the O(N·M) switch/node scan in getTopologySpec with O(depth) map lookups; copySkeleton intentionally omits these maps as they are not serialised.
pkg/translate/topology_test.go	Expected spec strings updated to reflect full switch paths (e.g., topo1:IB2:S1:S3 instead of topo1:S3); tests correctly exercise both cluster-wide (default:) and per-partition (topo:) cases.
pkg/engines/slinky/engine_test.go	Test expectations updated to include three-level switch hierarchy paths (e.g., topo-0:sw3:sw21:sw11), consistent with the medium.yaml test data.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[BFS over topology.Vertex tree] -->|leaf node found| B["nodeInfo.switches = parentMap[v.ID]"]
    A -->|non-leaf vertex v| C{"len(v.ID) != 0?"}
    C -->|yes| D["parentMap[w.ID] = parentMap[v.ID] + [v.ID]"]
    C -->|no root vertex| E[skip — root excluded from path]
    D --> A

    F[getTreeTopologyUnit / getBlockTopologyUnit] -->|build partition tree| G["tu.Tree.parents[key] = ancestors + [v.ID]"]
    G -->|leaf uses w.Name as key| H["parents['Node201'] = ['IB2','S1','S3']"]
    G -->|switch uses w.ID as key| I["parents['S3'] = ['IB2','S1']"]

    J[GetNodeTopologySpec] -->|cluster-wide tree| K["strings.Join(nodeInfo.switches, ':')"]
    J -->|per-partition| L["getTopologySpec → parents map lookup O(depth)"]
    K --> M["default:S1:S2"]
    L --> N["topo1:IB2:S1:S3"]

Loading

_{Reviews (2): Last reviewed commit: "fix: Topology Spec with the switch hiera..." | Re-trigger Greptile}

[dmitsh]

LGTM