crates/site-explorer/src/lib.rs (1)

3583-3615: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider consolidating the is_bf4_dpu_report cases into a single value-driven table.

These three #[test] functions exercise the same total operation (is_bf4_dpu_report → bool) with differing report shapes — precisely the pattern value_scenarios! targets. A single table (accepts BF4, rejects missing NIC topology, rejects BF3 shape) keeps each case one labeled row and localizes future additions. This is optional; the current tests are correct.

As per coding guidelines: "Use value_scenarios! for total operations (those returning a plain value, Option, or bool)."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/site-explorer/src/lib.rs` around lines 3583 - 3615, Consolidate the
three `is_bf4_dpu_report` test cases into a single `value_scenarios!`-driven
table test, since this function is a total operation returning a bool. Replace
the separate `#[test]` functions with one scenario table covering the accepted
BF4 report, the rejected zero-suffix case without BF4 NIC topology, and the
rejected BF3-shaped report, using the existing helpers like
`bf4_report_with_zero_suffix_ids`, `bf3_report_with_eth0`, and
`is_bf4_dpu_report`.

Source: Coding guidelines

crates/site-explorer/src/redfish.rs (1)

1155-1160: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Collapse the identical Err arms.

The Err(RedfishError::NotSupported(_)) => continue arm is behaviorally indistinguishable from the catch-all Err(_) => continue; the explicit variant adds no value and will trip clippy::match_same_arms, which this workspace treats as an error. Consider folding the two, and if the distinction was intended to surface diagnostics, log the swallowed error at debug before continuing rather than discarding it silently.

♻️ Proposed simplification

-        let resource = match client.get_resource(ODataId::from(path)).await {
-            Ok(resource) => resource,
-            Err(RedfishError::NotSupported(_)) => continue,
-            Err(_) => continue,
-        };
+        let resource = match client.get_resource(ODataId::from(path)).await {
+            Ok(resource) => resource,
+            Err(err) => {
+                tracing::debug!(%path, error = %err, "BF4 NDF0 resource fetch failed");
+                continue;
+            }
+        };

As per coding guidelines: "Enable all clippy lints by default and treat all warnings as errors."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/site-explorer/src/redfish.rs` around lines 1155 - 1160, The `match` on
`client.get_resource(...).await` in `redfish.rs` has two `Err` arms that both do
`continue`, which will trigger `clippy::match_same_arms`. Collapse the
`Err(RedfishError::NotSupported(_))` and `Err(_)` handling into a single branch
in the `ndf0_paths` loop, or if you need to preserve the distinction, emit a
`debug` log before continuing. Keep the behavior in `client.get_resource` and
the surrounding loop unchanged.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

crates/bmc-explorer/src/chassis.rs (2)

155-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Fragile positional path parsing for chassis/adapter/function IDs.

Splitting the hardcoded path string and indexing by fixed positions (parts.get(4), .get(6), .get(8)) silently breaks (falls through to continue) if the path format ever changes slightly (e.g., an extra/missing segment), with no diagnostic emitted. Since these IDs are already known at compile-time for each of the two BF4 topology variants, prefer expressing them directly as tuples instead of deriving them via string parsing.

♻️ Proposed refactor to avoid positional string parsing

     pub fn dpu_bf4_ndf0_permanent_mac(&self) -> Option<BaseMac> {
-        const BF4_NDF0_PATHS: [&str; 2] = [
-            "/redfish/v1/Chassis/BlueField_0/NetworkAdapters/BlueField_NIC_0/NetworkDeviceFunctions/0",
-            "/redfish/v1/Chassis/Card1/NetworkAdapters/Bluefield_NIC/NetworkDeviceFunctions/0",
-        ];
-        for path in BF4_NDF0_PATHS {
-            let parts: Vec<&str> = path.split('/').collect();
-            let (chassis_id, adapter_id, function_id) =
-                match (parts.get(4), parts.get(6), parts.get(8)) {
-                    (Some(chassis_id), Some(adapter_id), Some(function_id)) => {
-                        (*chassis_id, *adapter_id, *function_id)
-                    }
-                    _ => continue,
-                };
-
+        // BF4 NDF0 topology paths:
+        // /redfish/v1/Chassis/BlueField_0/NetworkAdapters/BlueField_NIC_0/NetworkDeviceFunctions/0
+        // /redfish/v1/Chassis/Card1/NetworkAdapters/Bluefield_NIC/NetworkDeviceFunctions/0
+        const BF4_NDF0_TARGETS: [(&str, &str, &str); 2] = [
+            ("BlueField_0", "BlueField_NIC_0", "0"),
+            ("Card1", "Bluefield_NIC", "0"),
+        ];
+        for (chassis_id, adapter_id, function_id) in BF4_NDF0_TARGETS {
             let mac = self
                 .members
                 .iter()

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/bmc-explorer/src/chassis.rs` around lines 155 - 168, In the BF4 path
handling inside the chassis parsing logic, avoid deriving chassis_id,
adapter_id, and function_id by splitting hardcoded strings and indexing fixed
positions. Replace the fragile path parsing in the BF4_NDF0_PATHS loop with
direct tuple-based data for each topology variant, and use those values directly
where the IDs are needed so the code no longer silently skips valid cases if the
path shape changes.

---

150-196: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Duplicate MAC-offset derivation logic across crates.

The BF4_NDF0_TO_BASE_MAC_OFFSET constant and the mac_to_u64/u64_to_mac conversion helpers are independently re-implemented here and in crates/site-explorer/src/redfish.rs / crates/site-explorer/src/lib.rs (per the supplied cross-file context), all performing the identical "NDF0 MAC minus 0x10" derivation. Divergence between these copies (e.g., one crate updating the offset without the other) would silently produce inconsistent PF0 base MACs. Consider hoisting this derivation logic into a shared location (e.g., carbide-network) that both bmc-explorer and site-explorer can depend on.

Also applies to: 222-232

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/bmc-explorer/src/chassis.rs` around lines 150 - 196, The BF4 PF0
base-MAC derivation is duplicated here in dpu_bf4_ndf0_permanent_mac and in the
site-explorer Redfish helpers, so consolidate the shared “NDF0 minus 0x10” logic
into one reusable helper in a common crate such as carbide-network. Move
BF4_NDF0_TO_BASE_MAC_OFFSET and the mac_to_u64/u64_to_mac-based derivation into
that shared helper, then update both callers in chassis.rs and the site-explorer
codepaths to use it so the offset and conversion behavior stay consistent.