Skip to content

Fail closed on deferred OpenDSS geometry - #484

Closed
samtalki wants to merge 1 commit into
stack/pr-480-dss-geometry-safetyfrom
agent/dss-geometry-implementation
Closed

Fail closed on deferred OpenDSS geometry#484
samtalki wants to merge 1 commit into
stack/pr-480-dss-geometry-safetyfrom
agent/dss-geometry-implementation

Conversation

@samtalki

@samtalki samtalki commented Sep 4, 2026

Copy link
Copy Markdown
Member

Why

OpenDSS lines whose impedance comes from LineGeometry, LineSpacing,
WireData, CNData, or TSData currently fall through to the Line class
factory impedance. That can turn a real geometry into plausible but wrong
electrical parameters and can turn a one-conductor SWER line into three
conductors without a parse diagnostic (#479).

This implementation is intentionally stacked on Burhan Abdullah's regression
and safety-contract PR #480. The temporary base branch is an exact mirror of
#480's head so this PR contains only the follow-up implementation commit. Merge
#480 first; this PR can then be retargeted to main without copying the
contributor's work.

What changed

  • Represents deferred line impedance explicitly as DistLine.linecode: None.
  • Replays phases, linecode, geometry, spacing, and conductor/cable-list
    assignments in source order so terminal maps retain the stated conductor
    shape.
  • Emits a parse-time error diagnostic when geometry impedance has not been
    calculated.
  • Preserves exact same-format DSS output from retained source.
  • Omits unresolved lines from semantic DSS, PMD, and BMOPF output with an error
    diagnostic instead of inventing a linecode or electrical values.
  • Keeps explicit linecode and inline-impedance behavior unchanged.

Validation

  • cargo test -p powerio-dist
  • cargo check -p powerio-dist --all-features
  • cargo clippy -p powerio-dist --all-targets -- -D warnings
  • cargo fmt --all --check
  • git diff --check

The regression coverage includes four-conductor geometry, single-conductor
SWER, spacing plus conductor lists, parse severity, exact DSS echo, and every
semantic distribution writer.

Depends on #480. Addresses #479 without implementing the separate Carson/line
constants work tracked in #84.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Conversion Matrix

Matrix generation failed before writing the report. See the workflow logs.

@samtalki samtalki closed this Sep 4, 2026
@samtalki
samtalki deleted the agent/dss-geometry-implementation branch September 4, 2026 03:00
@samtalki
samtalki restored the agent/dss-geometry-implementation branch September 4, 2026 03:01

Copy link
Copy Markdown
Contributor

I traced the workspace compile failure after DistLine.linecode became Option<String>. The core direction in this PR is right; the remaining issue is propagation of the new unresolved state into downstream consumers.

The current failures are in facade code that still assumes linecode: String, notably powerio/src/stored/legacy09/document.rs and powerio/src/transform.rs (calls such as net.linecode(&line.linecode), eq_ignore_ascii_case(&line.linecode), and formatting line.linecode with {}). These should not unwrap/default None, because that would reintroduce the silent-fabrication hazard this PR is fixing.

Suggested invariant for each consumer:

let Some(linecode) = line.linecode.as_deref() else {
    // record/return the existing unresolved-impedance error for this semantic path
    // and do not lower/serialize/analyze this line
    continue; // or return the appropriate readiness/lowering error
};

Then use linecode for lookup/diagnostic formatting. In readiness/lowering code, None should make the network not electrically lowerable rather than appearing as an unknown named linecode.

I also searched the tree for remaining direct line.linecode assumptions. Besides the two compile-failing facade paths, powerio-matrix/src/matrix/multiconductor.rs and some tests still use the old mandatory-string shape on current main. Worth auditing those as part of the same API propagation so a future all-workspace build cannot accidentally accept unresolved geometry.

Happy to keep validating the propagation against #480's fail-closed contract; the important point is preserving None as a first-class unresolved electrical state, never coercing it to a placeholder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants