Skip to content

Feat/deterministic network audit final - #79

Merged
qian-harvard merged 10 commits into
Power-Agent:mainfrom
BurhanAbdullah:feat/deterministic-network-audit-final
Sep 24, 2026
Merged

qian-harvard merged 10 commits into
Power-Agent:mainfrom
BurhanAbdullah:feat/deterministic-network-audit-final

Conversation

@BurhanAbdullah

Copy link
Copy Markdown
Contributor

Summary

Adds a deterministic, solver-independent structural audit layer for the pandapower MCP integration.

Changes

  • Add pandapower/audit.py with deterministic network pre-flight checks.
  • Detect invalid bus voltage limits, invalid line parameters/ratings, transformer rating issues, and unsupplied buses.
  • Use pandapower topology analysis for disconnected/unsupplied bus detection.
  • Expose the audit through the existing pandapower MCP server as audit_network.
  • Add regression tests covering clean networks, invalid parameters, islanded buses, non-finite limits, report stability, and non-mutation.
  • No power-flow solver is executed by the audit.

Scope

This PR intentionally contains only the audit implementation, MCP exposure, and tests.

@qian-harvard qian-harvard left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This addresses all three blockers from #68, and I checked each one against the network that disproved the original rather than taking the description's word for it. Thank you for the rework — the diff is noticeably tighter than the first attempt.

The three blockers, verified

DISCONNECTED_BUS now fires. unsupplied_buses(net) & in_service_buses is the right primitive. Re-run of the exact case that killed the first version, plus the ones around it:

two islands, slack at b0      status=error  DISCONNECTED_BUS: [2, 3]
orphan bus, no lines at all   status=error  DISCONNECTED_BUS: [1]
no source anywhere            status=error  DISCONNECTED_BUS: [0, 1]
ext_grid out of service       status=error  DISCONNECTED_BUS: [0, 1]

The last one is a nice property you may not have aimed for: a source that exists but is out of service correctly supplies nothing. And no false positives where it matters — case9, case118 and mv_oberrhein all come back ok.

The module is in the right package. pandapower/audit.py, next to the server that uses it. I checked the import hazard that comes with that, since the repo directory shares a name with the installed library: with only the server directory on sys.path (how powermcp/runner.py launches it), import pandapower inside panda_mcp.py still resolves to site-packages, not the repo folder. No shadowing.

It is reachable. audit_network registers as a tool and works end to end through the server — case9 returns ok, the islanded network returns two DISCONNECTED_BUS findings, and the payload round-trips through json.dumps. The non-mutation claim holds at the tool boundary too: bus table, line table and converged all unchanged after a call.

Also good: except ImportError instead of the catch-all, and the arbitrary 0.0..1.1 / 0.9..2.0 voltage windows are gone in favour of finite-and-signed checks, which is a much more defensible thing to assert.

Merged onto current main it is clean — no conflicts, 455 passed, and the server entrypoint tests still pass.

One thing worth fixing: status is overloaded

status: "error" means two different things, and a caller cannot tell them apart:

A) {"status": "error", "message": "No pandapower network is currently loaded. ..."}
B) {"status": "error", "counts": {"errors": 2, "warnings": 0, "info": 0}, "findings": [...]}

A is "the audit could not run". B is "the audit ran and the network has two unsupplied buses". A model branching on status will report a structural fault to the user when in fact nothing was audited — the opposite of what a pre-flight check is for, since the failure mode is silent and confident.

The AuditReport half of the contract is good precisely because it is machine-readable; the error half throws that away. Something like status: "failed" for the could-not-run case, or an explicit "ok" | "warning" | "error" | "failed" set, would keep the two separable. Either way it is worth stating the full set in the tool docstring, since that is what the model reads.

This is the same shape as the issue on #78, where a native failure dropped the structured fields and left only prose — worth a look at how that one landed.

Smaller notes, none blocking

  • counts["info"] is always 0. No check emits "info", so the key is decorative. Fine to keep for shape stability, just noting it carried over from the first version.
  • The test bypasses the server's import path. tests/test_audit.py loads audit.py through spec_from_file_location, which is a reasonable way to avoid the sys.path question — but it means the from audit import audit_network line in panda_mcp.py is not covered by any test. I verified it by hand and it works; a test that imports the server module would keep it that way.
  • audit is a very generic top-level module name. Nothing in the current dependency tree provides one, so there is no collision today. The existing from core import ... in OpenDSS is the same pattern, so this is consistent with the repo rather than new risk.
  • Cost is fine. iterrows had me expecting trouble on a large case; 1354 buses audits in 54 ms, so it is not worth restructuring.

Happy to merge once status distinguishes "could not run" from "ran and found faults" — everything else here is in good shape.

Checked against pandapower's bundled networks, the audit graded eight
solvable benchmark grids as structurally broken -- case145, case1888rte,
case2848rte, case3120sp, case6470rte, case6495rte, case6515rte and
case9241pegase all audited as "error" while run_power_flow converged
on each. They carry negative line resistance or negative transformer
vk_percent, as converted network equivalents often do.

Audit changes:
- Negative line r and negative trafo vk_percent are warnings.
  Non-finite r, and vk of zero or non-finite, stay errors: runpp raises
  FloatingPointError on those.
- NaN voltage limits count as unset, which is how pandapower stores
  them, rather than as invalid.
- Parameter faults on out-of-service elements are capped at warning,
  since they cannot break a solve.
- An area fed only through a dcline is now flagged DISCONNECTED_BUS. A
  dcline is two PV generators, not a slack, so it supplies nothing;
  runpp leaves NaN voltages there. Every in-service bus is added to the
  graph so a dangling element reference cannot hide an isolated bus.

Across all 62 zero-argument pandapower.networks constructors, exactly
the eight grids above change, from error to warning.

Tool contract: audit_network now reports like its sibling tools --
status "success" when the audit ran, "error" with a message when it
could not -- and carries the verdict in audit_status (ok, warning or
error). Across the pandapower server "error" means "could not run"
nineteen times; a verdict in the top-level status gave it the opposite
meaning in one tool.

The server also puts its own directory on sys.path before importing
audit, so a direct launch under PYTHONSAFEPATH no longer crashes; main
starts fine there and this branch did not.

Tests were written first and cover each change, the envelope, tool
registration and the safe-path launch: 10 failed at the previous head,
19 pass now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@qian-harvard

Copy link
Copy Markdown
Contributor

I pushed a round of fixes as 3a2dc0f (maintainer edit) and am merging. Thank you for the rework after the last review — and apologies, because one of these changes reverses advice I gave you.

The status vocabulary. I suggested ok | warning | error | failed, and you implemented exactly that, with a clear docstring. Looking at the whole server instead of the one tool, that suggestion was wrong: across pandapower/panda_mcp.py, "error" means "could not run" nineteen times, and in audit_network it meant the opposite. A model that learned the convention from run_power_flow would read a completed fault report as a tool failure. So audit_network now reports like its siblings — status: "success" when the audit ran, status: "error" plus a message when it couldn't — and the verdict lives in audit_status (ok | warning | error). That's my mistake to own, not yours. AuditReport.to_dict() is unchanged.

The audit against real networks. Across all 62 zero-argument pandapower.networks constructors, eight solvable benchmark grids audited as structurally broken — case145, case1888rte, case2848rte, case3120sp, case6470rte, case6495rte, case6515rte, case9241pegase — while run_power_flow converged on each. They carry negative line resistance or negative transformer vk_percent, as converted equivalents often do. Those are now warnings. Non-finite resistance, and vk of zero or non-finite, stay errors — runpp raises FloatingPointError on them. Exactly those eight grids change, and nothing else does.

Three smaller corrections:

  • NaN voltage limits are treated as unset, which is how pandapower stores them, not as invalid.
  • Parameter faults on out-of-service elements cap at warning, since they can't break a solve.
  • An area fed only through a dcline is flagged DISCONNECTED_BUS. A dcline is two PV generators, not a slack, so it supplies nothing, and runpp leaves NaN voltages there. Every in-service bus is added to the graph so a dangling element reference can't hide an isolated bus.

Packaging. The server now puts its own directory on sys.path before from audit import ..., so a direct launch under PYTHONSAFEPATH no longer crashes. The installed wheel ships audit.py correctly, since it lives in the force-included pandapower/ directory.

Tests were written first and cover every change, the envelope, tool registration and the safe-path launch: 10 failed at your previous head and 19 pass now. Merged with current main the full suite is 475 passed, and CI is green on all four interpreters.

Left as follow-ups, none blocking: counts["info"] is still never populated; the structural checks could catch a few more runpp failures (dangling references, vkr > vk); and the non-mutation test compares only net.bus.

Your #86 is next. It needs the same envelope and has one packaging blocker; I'll comment there.

@qian-harvard
qian-harvard merged commit 0cb26d1 into Power-Agent:main Sep 24, 2026
4 checks passed
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.

3 participants