Restructure the formula fallback module - #43
Merged
Conversation
The resolver's claim set has behaviors that no formula-level test reaches: a subtraction seed falls back to a standalone point when its parent meter is already claimed, a substitution onto a claimed meter merges into the existing point and emits nothing, a subsumed point's node stays claimed, a target sibling that is a meter disqualifies the subtraction, and overlapping diamonds never form a group. They only show on inputs the current callers cannot produce (meter targets come only from the consumer formula, whose target collection stops at the first component chain), but a restructuring could change them without any test noticing. Pin each one at the measurement_points level, with notes on which inputs sit outside the caller contract. Also pin that the subtraction-diamond subsume is independent of component id order, as the substitution twin already does, and that a subtraction stays a subtraction when the redundant-feeder pruning empties its subtracted set (possible only with a cycle among the subtracted siblings, so only on graphs kept alive by the validation allow-flags). Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
meter_substitution and subtraction_meters were two arms of one partition: both qualified the seed through parent_meters, walked the same siblings, and ended on the same group coverage check, and subtraction_meters returned None when nothing was subtracted because the substitution covered that case. Reading them side by side hid which differences were deliberate. Replace both with one classify function that partitions the siblings once into covered targets and subtracted others, and returns a single Group (meters, covered components, subtracted). A `None` subtracted set is the substitution/diamond shape; a `Some` one is the subtraction. The shape is decided before the redundant-feeder pruning, so a subtraction whose pruning empties the list keeps its shape (a plain parent-meter sum) instead of turning into a substitution. The deliberate asymmetries now sit as guards on their branch, each with its reason: a covered sibling that is a parent meter is rejected only when nothing is subtracted (a subtracted group rejects it through the nested-target check), and a covered meter sibling disqualifies only the subtraction (with nothing subtracted the whole group merges into the parents). measurement_points now handles one Group shape: the claim guard, worklist removal, and subsumption are written once instead of once per classifier result. The Substitution and Subtraction result structs fold into Group. No behavior change; the emitted formulas and measurement points are identical. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Rename the resolver's bookkeeping set from seen to claimed and document its role in one place on measurement_points: it holds every node an emitted point accounts for and gates duplicate emission. A claim is never released: drop_subsumed keeps the retracted points' nodes claimed. No behavior change. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
fallback.rs had grown to three concerns in one file: turning targets into measurement points, turning points into expressions, and the graph predicates both sides share. Split the internals into submodules, keeping the repo's module-file style (fallback.rs next to a fallback/ directory, no mod.rs): - fallback/resolve.rs: Measurement, Group, classify, measurement_points and the claim bookkeeping; - fallback/emit.rs: measure, diamond, subtraction, the child terms and sums; - fallback/predicates.rs: parent_meters, reached_only_through, reaches_any_below, is_grid_meter, is_measurable_component; - fallback/tests.rs: the test module, unchanged; - fallback.rs: SourcePreference, aggregate and aggregate_terms, and the re-exports keeping every caller path stable. Code moved verbatim; only visibilities, imports, and reference paths (doc links, test call paths) changed. No behavior change. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Add a short resolution-pipeline map to the module doc, so a new reader sees the three stages (resolve to points, emit expressions, sum) before the details. Split child_best_effort_term into a decision and a rendering: child_term_kind returns a ChildTerm (ReadingOr0, Bare, or Recurse), each variant carrying the reasoning that was one block comment before, and child_best_effort_term maps the decision to its expression. Also note on measure's last-resort ladder that the single-child-meter arm is unreachable today: only components-first builds that ladder, and a components-first meter with a meter child always stands alone. The arm stays so the ladder is right if that ever changes. No behavior change. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Six pass-through tests lived in the fallback test module but exercise other code: neighbor validation, the acyclicity check, ensure_root, the grid formula, and the battery formula. Move each to the test module of the code it tests (invariant_checks gets its first test module). While moving, split three long doc-comment sentences into short ones, and drop the acyclicity test's unused Result return; the test logic is unchanged. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
shsms
force-pushed
the
fallback-restructure
branch
from
July 13, 2026 13:09
ad7530f to
c4dd00e
Compare
Collaborator
Author
|
based on #42 |
shsms
force-pushed
the
fallback-restructure
branch
3 times, most recently
from
July 16, 2026 15:45
c75a80b to
112d787
Compare
shsms
added
the
cmd:skip-release-notes
shsms
force-pushed
the
fallback-restructure
branch
3 times, most recently
from
July 23, 2026 11:43
bb6c4dc to
1bc142c
Compare
shsms
changed the title
shsms
marked this pull request as ready for review
July 23, 2026 11:44
shsms
force-pushed
the
fallback-restructure
branch
from
July 23, 2026 11:46
1bc142c to
556b1db
Compare
llucax
reviewed
Jul 24, 2026
llucax
left a comment
llucax
left a comment
There was a problem hiding this comment.
🤖 rAIview (AI review here!)
No actionable findings. The private fallback refactor preserves resolver classification, claim/cache semantics, formula emission, error behavior, and existing caller/API contracts while making the production module boundaries substantially clearer. I also checked invalid and cyclic graph handling, test relocation and characterization coverage, security and dependency impact, release-note treatment, and commit structure; none raised a merge-relevant issue.
llucax
approved these changes
Jul 24, 2026
shsms
added a commit
that referenced
this pull request
Jul 27, 2026
`component_formula()` returned `0.0` for a component that provides no telemetry, which says its power is zero when the reading is simply missing. This returns `None` instead, and prepares the v0.6.1 release. ## Changes - `component_formula()` returns `None` for a component that provides no telemetry, matching what `component_ac_coalesce_formula()` already did. The check reads the operational mode and never the category, so a meter is covered too. - Bump the crate version to `0.6.1` and write the release notes for the operational-mode work from #43 and #44, including the no-telemetry behaviors the notes had left out: a component measured through the meter above it, a silent meter measured through its children, the `None` a silent grid meter produces, the consumer formula's descent past a silent meter, and the formulas that can come out `0.0` or `None`. - File the `component_formula()` / `component_ac_coalesce_formula()` id check under `## Bug Fixes` rather than `## Upgrading`. Neither signature changed — both already returned `Result` — and every other formula method already validated its ids through `graph.component(id)?`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The fallback module had grown to ~2500 lines mixing three concerns in one
file, with two near-duplicate group classifiers hiding which of their
differences were deliberate. This branch pins the resolver's latent
semantics with characterization tests, then restructures the module —
with no behavior change.
Changes
any restructuring, so the refactor can't silently change them.
meter_substitution,subtraction_meters) into oneclassifypass returning a singleGroup; the deliberate asymmetries now sit as guards with theirreasons.
fallback.rsintofallback/{resolve,emit,predicates,tests}.rs;all caller paths stay stable via re-exports.
ChildTerm) and add a resolution-pipelinemap to the module doc.
they actually test.
No public API or behavior change: the module is private and the emitted
formulas are identical.