openapi json export - #30
Merged
Merged
Conversation
jd-lara
force-pushed
the
jd/openapi-json-export
branch
from
July 31, 2026 23:43
9f43901 to
35be103
Compare
jd-lara
marked this pull request as ready for review
August 10, 2026 23:36
m-bossart
reviewed
Aug 12, 2026
| @@ -0,0 +1,55 @@ | |||
| @!IC,SBASE,REV,XFRRAT,NXFRAT,BASFRQ | |||
There was a problem hiding this comment.
If we are adding the casedata artifact directly, should all of the fixtures just go into casedata?
| it is safe to ignore rather than a tracked gap. The only sanctioned form of silent skip. | ||
| """ | ||
| const KNOWN_UNCONSUMED_PM_SECTIONS = Dict( | ||
| "substation" => |
There was a problem hiding this comment.
Why is substation in the unconsumed section here?
| `OpenAPISystem` API across parsers, but stays permanently empty here — PSS/E and | ||
| Matpower carry no time series. | ||
| """ | ||
| struct OpenAPISystem |
There was a problem hiding this comment.
This gets defined both here and in PowerTableDataParser which doesn't seem ideal. Its not clear to me where it belongs though
| ) | ||
| end | ||
|
|
||
| """Curtailment cost for a hydro generator: PSCB never derives one from pm data.""" |
There was a problem hiding this comment.
This is a small thing, but there are many comments referencing PSCB throughout because a lot of this code was ported from there. Which won't make sense in the long run
| @@ -0,0 +1,33 @@ | |||
| # Parsing code ignores type=null. | |||
There was a problem hiding this comment.
Why is the generator mapping included as a yaml file?
| @@ -0,0 +1,30 @@ | |||
| # scripts | |||
There was a problem hiding this comment.
If I run this validation script pointing to inspection_output/modified_14bus_system.NATURAL_UNITS.json or inspection_output/modified_14bus_system.DEVICE_BASE.json I get a bunch of failures. Is that expected?
A BRANCH record whose CKT is prefixed '@' or '*' is a breaker or switch, and _split_breakers_and_branches! routes it to SWITCHES_AS_BRANCHES. The RAW System Switching Device record carries only a reactance below the zero-impedance threshold -- it has no GI/BI/GJ/BJ columns at all -- and a breaker or disconnect has no line-connected reactor or capacitor to describe. A switching-device row carrying line shunts is therefore malformed input. That admittance was previously dropped without a trace. Each non-zero end now becomes a shunt at the bus it was declared on, and the relocation is warned about rather than silent: a branch with real line shunts is a transmission line, not a breaker or switch, so the source data is worth checking. GI/BI/GJ/BJ are pu on the system base -- a BRANCH record declares no MVA base of its own, unlike a transformer winding with its SBASE1-2 -- while a FIXED SHUNT's GL/BL are MW/MVAr at unity voltage and _make_per_unit! divides every shunt gs/bs by the case base. The value is scaled by baseMVA so that division lands back on the declared pu value. The shunt inherits the device's ST because the RAW defines these shunts as "connected to and switched with the line". The salvage runs before _build_switch_breaker_sub_data, which pops I and J.
These are Claude Code working notes, not project documentation, and should never be committed.
The 12 copies of this file across the psy6 workspace had drifted into 6 distinct versions. Its content now lives in the user-scope `sienna-psy6` skill, which loads on invocation instead of being read in full every session.
The lowercase name only loaded because macOS is case-insensitive; on Linux and in CI the file was silently ignored. Pure rename, no content change.
Mirrors PowerTableDataParser's completed refactor (8cbf8a4): OpenAPISystem now composes PC.SystemDocument + IdRegistry(document) + an always-empty time_series slot instead of duplicating the document's state, IdRegistry delegates id allocation to PC.next_id!(document), and serialize.jl's hand-rolled envelope is replaced by PC.validate_document + PC.write_document. Adds src/openapi/topology.jl (ACBus, Area, LoadZone, Arc-minting scaffolding) and src/openapi/build.jl (build_openapi_system entry point), porting PowerSystemCaseBuilder's read_bus!/read_loadzones! oracle with unit conversion at the set_value! boundary: PowerModelsData's system per-unit power quantities are multiplied back by baseMVA before every set_value! that touches MW/MVAr, since the schemas' ActivePower/ReactivePower have no pu row. Voltage magnitude/ limits and angle need no conversion (already pu-on-base_kv and radians respectively). Later reader stages (load, generation, branch, ...) are same-named stubs that error on demand; build_openapi_system calls only the implemented topology stage.
build_openapi_system was silently dropping every non-topology pm dict section
(gen, load, branch, shunt, facts, dcline, ...) with no warning or error - the
silent-partial-output pattern this effort exists to kill. Add
_warn_unconsumed_sections, called at the end of build_openapi_system: walks the
pm dict, skips scalar/metadata keys and the sections in _CONSUMED_PM_SECTIONS
("bus" only today), and emits one @warn naming every remaining non-empty
section by name and row count. As later sub-tasks add real readers, each
consumed section joins the tuple and leaves the warning; once every section is
accounted for, this becomes a hard error for genuinely unknown pm-dict shapes.
Also fix test_openapi_units.jl's two failing tests, which turned out to be
fallout of this same effort's PowerOpenAPIModels regen (f460cc7) rather than
unrelated drift: TwoTerminalLCCLine.parameter_units dropped "SYSTEM_BASE" down
to a 2-value enum (DEVICE_BASE now plays that role), and ConstantReserve was
replaced by OnlineReserve/OfflineReserve/GroupReserve.
Ports PSCB power_models_data.jl's load, generator (thermal, hydro, renewable, synchronous condenser, storage), and cost-curve readers into the openapi/ pipeline, bug-compatible with the three known oracle defects (renewable rating double base_conversion, generic battery's unconverted thermal_rating, hydro reservoir emitted as HydroDispatch). Wires read_loads!/read_generation! into build_openapi_system in place of their prior stubs.
…it layer Ports PSCB's read_branch!/read_3w_transformer!/read_dcline!/read_vscline!/ read_shunt!/read_switched_shunt!/read_facts! plus the interarea_transfer AreaInterchange block into PFFP's emit layer: Line, TwoWindingTransformer + TransformerCircuit, ThreeWindingTransformer, TwoTerminalLCCLine/ TwoTerminalGenericHVDCLine, TwoTerminalVSCLine, AreaInterchange (bug-compatible with the oracle's D5 #4 unscaled power_transfer), FixedAdmittance, SwitchedAdmittance, and FACTSControlDevice. Wires all four stages into build_openapi_system and extends _CONSUMED_PM_SECTIONS accordingly.
…LTAGE arithmetic Converts the 16 ternary operators flagged in review (branch.jl, dc_branch.jl) to if/else, since the oracle itself uses if/else at those sites and they don't qualify for the verbatim-port exception. Inline-marks the 5 remaining ternaries that ARE verbatim oracle ports (including 2 more found beyond the review's list in read_area_interchanges!). Also removes the dimensionally-nonsensical sys_mbase-scaled dc_setpoint_from/to computation in the unreachable DC_VOLTAGE branch, leaving the loud set_value! error path as the only outcome there.
…lip warn to error Task 13d (final PFFP-readers sub-task): read_switch_breaker! (switch/breaker/ generic_connector -> DiscreteControlledACBranch) and read_attributes! (impedance correction tables -> shared ImpedanceCorrectionData supplemental attributes, deduplicated per (table, winding) to match the oracle's object sharing). Also consumes area_interchange (Area.ext), found while auditing every remaining pm dict section; the rest (substation, areas, owner, zone) are recorded on KNOWN_UNCONSUMED_PM_SECTIONS with reasons. _warn_unconsumed_sections is now _check_unconsumed_sections and errors on any section neither consumed nor allow-listed. Adds the mandatory CZ/CW/CM/MDC discriminator fixture (device base != system base, closing the untestable gap flagged in test_openapi_branch.jl) and regenerates inspect_14bus_json.jl's output via the now-complete emit layer. Full report: SiennaSchemas .superpowers/sdd/.../reports/task-13d-report.md
…/pu discriminator debt
…ries DEVICE_BASE; dual-mode canary; AC-side gap pointers
Trim task-narration and paragraph-padding from the src/openapi/ headers and docstrings, keeping only non-obvious WHY (units conventions, oracle deviations, bug-compatible markers, the VSC RECORDED GAP). Structural cleanups: - attach make_vscline!'s docstring to make_vscline! instead of the helper above it - merge the duplicated dc/ac voltage-control conditions in make_vscline! - drop the redundant validate_document call in to_json (write_document validates) - inline the single-call-site impedance-correction association and cost slope helpers - hoist the six copy-pasted 14-bus fixture consts and _matches_nt into runtests.jl
unit_system = "DEVICE_BASE" previously stamped the flag but left every value in natural units, identical to NATURAL_UNITS. Add a post-build conversion pass (src/openapi/device_base.jl) that walks the built document and divides each power-family field by the component's own device base (or the document's system base, for the few types with none of their own) whenever the document is DEVICE_BASE, mirroring the inverse of PowerSystems' own NaturalUnit importer. Classification is mechanical (PowerCoreOpenAPIModels' declared unit/quantity metadata), with two hand-diffed exceptions (Area/LoadZone peak fields are schema-fixed-natural; FACTSControlDevice has no base_power of its own) and a loud error for anything the pass cannot classify. Extend generation/load/branch tests with DEVICE_BASE assertions for a generator with mbase != sys_mbase, a load, a line rating, and a transformer circuit, hand-deriving the expected per-unit values.
The PSY equivalence oracle (from_openapi round-trip comparison) caught ThermalStandard/HydroDispatch/EnergyReservoirStorage's ramp_limits (quantity ActivePowerChangeRate, MW/min) missing from the power-family quantity set, leaving it in natural units in a DEVICE_BASE document while every sibling MW/MVAr/MVA field converted correctly. Add a regression assertion cross-checked against the oracle's finding.
EnergyReservoirStorage.storage_capacity was silently left in natural units under DEVICE_BASE: its quantity (ElectricalEnergy) is only resolvable through energy_units' instance-level discriminator, and the previous rule treated every instance-dispatched field as :skip, conflating a genuine pu-vs-natural representation switch (correctly untouched) with a natural-unit-choice discriminator that still needs conversion (PSY's own converter divides storage_capacity by device base_power regardless of which energy_units branch is active). Replace the blanket skip with an explicit registry (_DEVICEBASE_INSTANCE_DISPATCHED) that every instance-dispatched (key, prop) must appear in, erroring by name otherwise -- falling through silently is no longer possible by construction. Populated by empirically auditing every real (unit, quantity) pair PFFP's readers produce, which also caught two more previously-mis-skipped fields: TransformerCircuit.controlled_quantity_limits and TwoTerminalLCCLine.transfer_setpoint both genuinely switch quantity with their own discriminator (control_objective / power_mode) and are resolved per component via a second registry (_DEVICEBASE_DYNAMIC_QUANTITIES). Add a storage DEVICE_BASE test (device base != system base, math in comments) and a test pinning the loud-error guarantee on an unregistered instance-dispatched field.
…:dynamic Round 1 classified this field :dynamic, converting it by the circuit's own base_power whenever control_objective resolves to a power-flow quantity (ActivePower/ReactivePower). Wrong: PowerSystems' own to_openapi calls the identical, unscaled _minmax_po(get_controlled_quantity_limits(circuit)) in both DeviceBaseUnit and NaturalUnit (export_handwritten.jl:166-167, :195-196) -- this field never scales with the document convention, regardless of control_objective. Invisible on the 14-bus fixture because every circuit there is control_objective = "FIXED" (already :skip either way). Static :skip now, matching control_limits, with the PSY citation inlined. Mark TwoTerminalLCCLine.transfer_setpoint's kept :dynamic disposition as explicitly parked: PSY has no converter for that type to check against yet, unlike every other registry entry. Add a regression test: a synthetic ACTIVE_POWER_FLOW-objective transformer circuit (base_power != sys_mbase) asserting controlled_quantity_limits passes through unscaled while its sibling power fields still convert.
…two-terminal HVDC, family convention governs
…ue helpers, dedup tests
…plementalAttributeAssociation Service membership moved back to its own specialized table on the schema side (SiennaSchemas un-consolidated the single association table). add_service_association! now constructs PO.ServiceAssociation rows and drops the attribute_type argument, which no longer applies. Drop the stale group_index/role docstring on add_supplemental_attribute! and the test assertions that checked those now-removed fields.
…mapping
SiennaSchemas made base_power required on both types; neither emitted it, so
every document failed validation against the generated Python models. Both
carry the SYSTEM base ("in lieu of a system-level table"), the same value
_DEVICEBASE_SYSTEM_BASE_TYPES already scaled by, so the device_base
classification is unchanged — but its comment claimed FACTSControlDevice has
no base_power field, which is now false and was the stated reason for the
fallback.
validate_with_python.py validated every supplemental attribute as
GeographicInfo. The document does record each attribute's class, in
supplemental_attribute_associations; dispatch on it, and fail loudly for an
attribute that has no association rather than guessing. 13 failures -> 0.
The generator mapping was a YAML file read at runtime from the only YAML use
in the package; inline it as a literal table and drop the dependency, keeping
the duplicate-key check so a collision still errors instead of overwriting.
Strip the PSCB porting-provenance comments, keeping the ones that document a
deliberate divergence or bug-for-bug compatibility.
…on gap `data["substation"]` was allow-listed as deliberately unread because the only target was PSCB's GeographicInfo bus attachment, which needs `ext` threaded through `read_bus!`. SiennaSchemas now defines a `Substation` supplemental attribute carrying the identity and grounding resistance directly, so the section is consumable as-is: one attribute per SUBSTATION record, associated with every bus its node list places inside it. A substation spanning several buses gets one attribute and an association per bus, the sharing shape `_attach_impedance_correction!` already uses. Node, switching-device and terminal detail stays in the pm dict — `read_switch_breaker!` is what turns switching devices into components. The node/device/terminal detail and the RAW's latitude/longitude have no schema target, so they stay in the pm dict rather than being reshaped into geo JSON no reader consumes.
jd-lara
force-pushed
the
jd/openapi-json-export
branch
from
August 15, 2026 02:06
829707e to
f6d9cac
Compare
From the review flagged in #8, applied to the files that came across to this package (psse.jl, pti.jl, node_breaker.jl): - Drop the `cond ? true : false` forms; the comparison is already Boolean. - Name the PSS/E bus type codes and the micro-unit scale factor, replacing the bare 4/2/1e-6 literals. `pm_io/data.jl` keeps its literals: it is vendored PowerModels code and was not part of the review. - Match the switched-shunt step columns as `^N[1-8]$` rather than "starts with N and ends in a digit", which would also take an unrelated future column. - Say what `apply_tap_correction!`'s COD/CW guard actually tests, and compare against tuples rather than allocating a vector per call. - Derive v35's SWITCHING DEVICE position from the section it follows instead of a literal index into `_pti_sections`. - Log the FACTS shunt-only simplification once per file rather than once per device, and say what the simplification is. Closes #8
The [sources] pins pointed at sibling checkout paths, which exist only in a local workspace, so every CI job failed at resolution with "expected package PowerCoreOpenAPIModels to exist at path". Pin both by repository URL and subdir instead. Neither is registered yet, so this stays a branch rev until there is a tag. Verified by instantiating and running the suite from a clean export with no sibling checkout on disk: 2364/2364.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
- Add `add_supplemental_attribute_association!` to container.jl; both the substation and impedance-correction readers built the association row by hand with `attribute_type` as a string literal, which `add_supplemental_attribute!` already derives from the attribute. The impedance cache now holds the attribute rather than its id so the derivation works for both. - Drop FACTSControlDevice from _DEVICEBASE_SYSTEM_BASE_TYPES. It carries its own base_power now, so the generic anchor path reads the base from the document instead of asserting it from a name list; the set went back to meaning one thing, "no base_power field of its own". Same number either way today, which is exactly why the bypass was worth removing. - Rename read_attributes! to read_impedance_corrections! and make read_attributes! the stage that calls it and read_substations!. The old name was false — it early-returns when there are no correction curves, which is why substations could not live inside it. - Register Substation by name rather than allocating a bare id: it has a name, so it belongs in the by-name index and gets duplicate rejection. - Finish the bus_type migration in psse.jl, which had left bare literals in the same loop as converted ones, and drop the duplicate PM_BUS_TYPE_NAMES. - Replace the N1..N8 regex with an exact character test and use it for the B1..B8 sibling too; convert three more vector membership tests in the same transformer loop to tuples. - Delete three empty docstrings left by the earlier comment cleanup, and trim padded comments.
The Documentation job failed to resolve for the same reason CI did: the docs env never pinned the OpenAPI packages. `[sources]` is honored only for the active project's own deps, so the docs project has to declare both rather than inherit them transitively through PowerFlowFileParser. Also document GENERATOR_MAPPING_PM, which an existing docstring `@ref`s — an unresolvable cross-reference fails the build.
The docs job pinned 1.10, which predates `[sources]` support, so the OpenAPI pins the docs environment now carries were ignored and resolution fell through to the registry. Every other workflow already tracks '1'.
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.
This is a first pass at populating the OpenAPIModels from the PowerFlowParser
Closes #8