Treat untracked component categories as transparent pass-throughs by shsms · Pull Request #35 · frequenz-floss/frequenz-microgrid-component-graph-rs

shsms · 2026-04-29T14:49:40Z

This PR updates the component-graph traversal and validation/modeling layers to treat certain “unhandled” component categories as pass-through (transparent) nodes, so validation rules and formula generation operate on the effective topology rather than the raw graph.

Changes:

Introduce effective neighbor traversal that skips pass-through categories, while adding raw_predecessors / raw_successors for graph-direct traversal.
Update connectedness validation to ignore pass-through nodes, and add pass-through-focused formula/validation tests.
Add pass-through classification via ComponentCategory::is_passthrough() and emit operator warnings for pass-through components during graph construction.

Captures the four desired behaviors when a pass-through category (one without specific handling -- `PowerTransformer` here) sits in the topology between handled categories. Each test asserts the contract and is `#[ignore]`d until the corresponding fix lands; `cargo test -- --ignored` runs them. The four cases: * Validation accepts `Grid → PT → Meter → Inverter → Battery`: neighbor rules consult the effective predecessors / successors, so the chain through the pass-through reads as if it weren't there. * `grid_formula` skips a PT sitting directly below the GCP and uses the meter beneath it as the measurement source. * `meter_fallback` walks past a PT between meter and inverter, summing the meter's effective successors rather than including the transformer (which has no measurement). * `component_fallback` for an inverter beneath a meter-PT-inverter chain finds the meter through the pass-through and falls back to it. Adds a `power_transformer()` helper to the test builder. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>

The illustrative protobuf-to-cg-enum mapping in the `Node` trait's doc-comment referenced four `gr::ComponentCategory` (or `gr::InverterType`) variants that don't exist in this crate's public API: * `gr::ComponentCategory::Grid` — should be `GridConnectionPoint` * `gr::InverterType::Solar` — should be `Pv` * `gr::ComponentCategory::Fuse` — no Rust variant; arm dropped * `gr::ComponentCategory::VoltageTransformer` — should be `PowerTransformer` The example carries an `///ignore` rustdoc tag so the typos didn't break the build, but anyone copy-pasting from the docs hit non-compiling code immediately. The protobuf (`pb::*`) names stay unchanged — they're for whichever upstream API version the user is on, not this crate. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>

The illustrative protobuf-to-cg-enum example mapped a hypothetical `pb::ComponentCategory::Relay` to `gr::ComponentCategory::Relay` — which doesn't exist in this crate's public enum. The Rust enum has `Breaker`, and the upstream Python client treats relays and breakers as the same protobuf value (`ELECTRICAL_COMPONENT_CATEGORY_BREAKER`), so map across the gap. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>

`ComponentGraph::try_new` now logs a `tracing::warn!` for each pass-through node in the validated graph, telling the operator that the component will be transparent to validators and formula generators. Logged once per node, after validation succeeds, so we only warn for components that actually end up in the graph. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>

A *pass-through* category is one without specific handling in validators or formula generators — the graph treats it as transparent when checking neighbor relationships and when generating formulas. The set: Converter, Breaker, Precharger, Electrolyzer, PowerTransformer, Hvac, Plc, CryptoMiner, StaticTransferSwitch, UninterruptiblePowerSupply, CapacitorBank. `Unspecified` is intentionally not pass-through — it's rejected at graph-construction time before either system sees it. `WindTurbine` isn't either — it has a dedicated formula generator and is a real measurement source. The classification is locked by a unit test that asserts every `ComponentCategory` variant lands on the right side. The method is `#[allow(dead_code)]` until the next commits wire it into `predecessors`/`successors` and the construction-time warning. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>

Introduces `raw_predecessors` and `raw_successors` as the graph-direct iterators over a component's neighbors — the bodies that `predecessors` and `successors` currently inline. Their docs are explicit that these include pass-through nodes, unlike the soon-to-be-pass-through-aware `predecessors` / `successors`. The two new methods are near-duplicates of `predecessors` / `successors`; the next commit pulls the shared body out into a private helper. `predecessors` / `successors` themselves stay unchanged here — they keep their inline bodies until the commit that flips them to walk past pass-throughs. Renames the existing `Neighbors` iterator to `RawNeighbors` (and updates its docstring) to reflect that it's now the raw view, returned by the new `raw_*` methods. The `Neighbors` name is freed up for a soon-to-be-added pass-through-aware iterator returned by `predecessors` / `successors`. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>

…rieval `raw_predecessors` and `raw_successors` had nearly identical bodies that differed only in the `petgraph::Direction` they passed. Pull the common body into a private `raw_neighbors(component_id, direction)` helper that both methods now delegate to. Pure refactor; no behavior or API change. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>

The public `predecessors` and `successors` methods now walk transparently past pass-through nodes, yielding only their non-pass-through ancestors / descendants. The raw graph view stays available via the new `raw_predecessors` / `raw_successors`. Implementation: a private `collect_effective_neighbors` helper does a breadth-first walk through pass-through nodes in the chosen direction, dedup'd via a `HashSet<u64>` and yielded as the new `Neighbors` iterator type in `iterators.rs` (the prior graph-direct iterator was renamed to `RawNeighbors` two commits ago). `Siblings` switches its inner iterator type to match. Validators are split between the two views: * `validate_acyclicity` walks via `raw_successors` so cycles composed entirely of pass-through nodes (which are invisible to the effective view) are still detected. * `validate_connected_graph` walks the effective view but exempts pass-through components from the unvisited check: PTs are transparent and never marked visited, but we don't require them to be reachable -- even when `allow_unconnected_components` is `false`. * `ensure_root` stays on the effective view: a pass-through sitting upstream of the GCP is tolerated, since the effective predecessors view walks past it. `find_all` skips pass-through nodes when checking the predicate, so they don't appear in callers' result sets. It walks the raw graph without a visited set; acyclicity guarantees termination. The four pass-through tests added three commits ago drop their `#[ignore]` and pass -- both the validation case (rules use `predecessors` / `successors` so they auto-pick up the effective view) and all three formula cases (`grid_formula`, `meter_fallback`, `component_fallback` likewise inherit the new behavior through the public methods they already call). Drops the `#[allow(dead_code)]` marker on `ComponentCategory::is_passthrough` now that it has its first non-test caller. Existing `validate_neighbors` tests used `Electrolyzer` as a "wrong-category" sentinel for successor checks. Electrolyzer is now pass-through, so the validator interprets it differently (the inverter then has zero *effective* successors, failing `ensure_not_leaf` rather than the matcher). Substituted with `WindTurbine`, which has no validation rules and isn't pass-through -- the original assertions hold with that swap. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>

Copilot

Pull request overview

This PR updates the component-graph traversal and validation/modeling layers to treat certain “unhandled” component categories as pass-through (transparent) nodes, so validation rules and formula generation operate on the effective topology rather than the raw graph.

Changes:

Introduce effective neighbor traversal that skips pass-through categories, while adding raw_predecessors / raw_successors for graph-direct traversal.
Update connectedness validation to ignore pass-through nodes, and add pass-through-focused formula/validation tests.
Add pass-through classification via ComponentCategory::is_passthrough() and emit operator warnings for pass-through components during graph construction.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`src/graph_traits.rs`	Updates example category/type mappings (e.g., Grid → GridConnectionPoint, Solar → Pv, Relay → Breaker, VoltageTransformer → PowerTransformer).
`src/graph/validation/validate_neighbors.rs`	Adjusts neighbor-validation tests to use a non-pass-through category (`WindTurbine`) for expected failures.
`src/graph/validation/validate_graph.rs`	Connectedness validation now excludes pass-through categories from the “must be reachable” set.
`src/graph/test_utils.rs`	Adds builder helper for `PowerTransformer` to support pass-through scenarios in tests.
`src/graph/retrieval.rs`	Adds `raw_*` neighbor APIs and changes `predecessors`/`successors` to return pass-through-aware effective neighbors.
`src/graph/iterators.rs`	Introduces `EffectiveNeighbors` and updates `Siblings` to use effective neighbor iteration.
`src/graph/formulas/fallback.rs`	Adds tests asserting validation/formula behavior when pass-through nodes exist in the topology.
`src/graph/creation.rs`	Emits a `tracing::warn!` for each pass-through node accepted into a graph.
`src/component_category.rs`	Adds `ComponentCategory::is_passthrough()` plus tests locking the classification.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>

The single-argument `concat!` macro was just a string literal; clippy's `useless_concat` (new in 1.95) flags it. Drive-by from running clippy on this branch -- the call has been there since the test was first written and isn't related to the pass-through work in the rest of this branch. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Rename the local variable to `first_occurrence` (correct spelling). Drive-by while reviewing the cycle-detection code; predates this branch. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>

simonvoelcker · 2026-05-04T09:45:44Z

@shsms code looks very good - Indeed needs a rebase now to account for steam boilers.

I was wondering what we need this for though. Can you point me to any ticket / slack thread or so with a bit of context?

shsms · 2026-05-04T14:43:43Z

@simonvoelcker was too lazy to make an issue, but sent you a link to what prompted this. Also rebased.

simonvoelcker · 2026-05-05T09:10:43Z

+
+        let mut queue: VecDeque<NodeIndex> =
+            self.graph.neighbors_directed(start, direction).collect();
+        let mut visited: HashSet<NodeIndex> = HashSet::new();


I'm wondering if de-duplicating nodes will change behavior in some cases.

Before the change, nodes that have multiple parents will appear multiple times when iterating over all nodes - The simplest case is a diamond pattern. Now, if the intermediate nodes in the diamond are pass-through, the leaf node will appear only once instead of twice. Is that always desired or could it make sense to offer control over this behavior to the caller, via a flag?

I don't think there are any cases where we want the leaf node to appear twice. For example,

gcp -> two transformers -> both connected to one meter

We want the meter only once.

I also think that should be the common case. I was just wondering whether we're sure that we never want the leaf node to appear twice, given that this is a change to an existing function rather than a new one.

simonvoelcker

Very nice!

shsms added 8 commits April 28, 2026 10:28

shsms requested a review from Copilot April 29, 2026 14:49

Copilot started reviewing on behalf of shsms April 29, 2026 14:50 View session

shsms mentioned this pull request Apr 29, 2026

Add steam boiler #34

Merged

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Comment thread src/graph/retrieval.rs

Comment thread src/graph/retrieval.rs

Comment thread src/graph/retrieval.rs

Comment thread src/graph/creation.rs

Update release notes

a3f9ebe

Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>

shsms force-pushed the passthrough-categories branch 2 times, most recently from 3579204 to d30c41a Compare April 29, 2026 16:30

shsms force-pushed the passthrough-categories branch from d30c41a to 7bc6234 Compare April 30, 2026 08:04

shsms requested a review from Copilot April 30, 2026 08:28

Copilot started reviewing on behalf of shsms April 30, 2026 08:29 View session

Copilot AI reviewed Apr 30, 2026

View reviewed changes

Comment thread src/graph/validation/validate_graph.rs Outdated

Comment thread src/graph/formulas/fallback.rs

Comment thread src/graph/retrieval.rs

Comment thread RELEASE_NOTES.md

Fix first_occurance typo in validate_acyclicity

a683ed2

Rename the local variable to `first_occurrence` (correct spelling). Drive-by while reviewing the cycle-detection code; predates this branch. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>

shsms mentioned this pull request Apr 30, 2026

Pass-through component support in the Python bindings frequenz-floss/frequenz-microgrid-component-graph-python#49

Draft

shsms force-pushed the passthrough-categories branch from 09a0e5f to d88dfdc Compare May 4, 2026 14:39

shsms requested a review from simonvoelcker May 4, 2026 14:42

simonvoelcker reviewed May 5, 2026

View reviewed changes

Comment thread RELEASE_NOTES.md

simonvoelcker approved these changes May 5, 2026

View reviewed changes

shsms enabled auto-merge May 5, 2026 11:20

shsms force-pushed the passthrough-categories branch from d88dfdc to a683ed2 Compare May 5, 2026 11:21

shsms added this pull request to the merge queue May 5, 2026

Merged via the queue into frequenz-floss:v0.x.x with commit a9d51a2 May 5, 2026
3 checks passed

shsms deleted the passthrough-categories branch May 5, 2026 11:22

Conversation

shsms commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

simonvoelcker commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shsms commented May 4, 2026

Uh oh!

simonvoelcker May 5, 2026

Choose a reason for hiding this comment

Uh oh!

shsms May 5, 2026

Choose a reason for hiding this comment

Uh oh!

simonvoelcker May 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

simonvoelcker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shsms commented Apr 29, 2026 •

edited

Loading

simonvoelcker commented May 4, 2026 •

edited

Loading