SystemDefiner consistency overhaul: module split, cross-reference cascades, validator - #3
Merged
Merged
Conversation
The composition/hierarchy editors were hard-capped at 4 depth levels
("layers"), even though the data model, YAML/Excel serialization, and the
engine were already depth-agnostic. Lift the cap so users can add/remove
hierarchy levels ("columns") as well as element branches ("rows").
- config_schema: pad_level_names no longer truncates to 4 — accepts any
number of levels (min 2), backfilling defaults only when too few.
- main.py: _rules_to_paths pads to the actual deepest path; elements_save
collects level-name and path-cell fields tolerantly (dynamic count).
- elements.html: dynamic level-name editors with + Add Level / remove
controls; path table columns, preview, and JS derive depth from the DOM;
init grows columns to fit a stored hierarchy deeper than its named levels.
- compositions/tc_edit_dynamic/initial_stock_edit/bom_edit: the shared
grouped-matrix widget now derives its column count from the level list /
deepest path instead of a hardcoded 4.
Golden regression unaffected (engine untouched). Verified via HTTP
round-trip: a 6-level hierarchy with a 5-deep branch saves and reloads.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Flow Data editor rendered every input flow expanded with an eager Chart.js build, and points could only be entered one row at a time — slow for studies with many flows or long timelines. - Accordion: each flow collapses to a one-line header (name, id, P→P, live point count). Charts build lazily on first expand, so a study with many flows loads instantly. Adds Expand-all / Collapse-all; first flow opens by default. - CSV import: a per-flow "Import CSV" button parses a "year,value" file client-side and replaces that flow's points (then Save persists them via the existing POST — no new route). Tolerates a header row, ';'/tab delimiters, and European comma decimals. Verified: flow_data page renders with the new controls; parseCsv unit-tested against header/comma-decimal/semicolon/junk-row cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add plot_flow_composition_sunburst() as a supplementary hierarchical view alongside the stacked-bar plot_flow_composition(). The element hierarchy is drawn as concentric rings — total-mass element at the centre, top-level elements in the first ring, sub-elements outward — with a lighter "Remaining X" wedge for any untracked residual so children sum to their parent (branchvalues="total"). Nodes whose children exceed them are expanded to fit rather than raising. Reuses build_element_children_map() and flow.Values[year, element]; Plotly is already a dependency. Follows the module convention (renders via fig.show(), not exported from plotting/__init__). Surfaced in section 2.4 of 00_BioDYM_Workflow and the Composition tab of the Voila dashboard (aggregates all flows at the final year by default; flow_id/year selectable). Verified against a solved T01 system (correct nesting, no branchvalue violations) and full pytest (356 passed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The sunburst used fig.show(), which clashed with the sibling FigureWidget
from plot_flow_composition when both rendered in the same ipywidgets Output
(Voila dashboard) — breaking the existing stacked-bar composition figure.
Switch plot_flow_composition_sunburst to the same FigureWidget + display()
mechanism as plot_flow_composition, and add live controls:
- Flow dropdown ("All flows" or a single flow)
- Year slider
Both update the figure in place via batch_update (trace arrays swapped),
so node count can change between selections. Export button now exports the
current view. Returns None (display-only) to match the module convention;
use _build_composition_sunburst_figure for a static figure in tests.
Verified: interactive construct, per-flow/per-year builds, and the
FigureWidget trace-swap update; full pytest 356 passed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Set the sunburst figure height to 750px (from Plotly's ~450 default) and bump the base font to 14px so the rings and labels are easier to read. Width stays responsive (100% of the container in VoilA). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
New tutorial case study T16_Complex_Flow_Composition exercising the extensible composition hierarchy and the sunburst view together: - Six named levels (Device → Module → Component → Material → Substance → Element) with a genuinely 6-deep element tree, e.g. Laptop → Mainboard → Electronics → Metals → Precious_Metals → Gold and Laptop → Battery → Cells → Active_Material → Lithium_Compound → Lithium (22 elements, 12 hierarchy rules). - Two input flows (Laptop, Smartphone) sharing the tree but differing in their top-level module split, so the sunburst's Flow dropdown is meaningful. Golden-pinned like T01–T15 (glob T[0-9][0-9]_*): reference npz added, solver converges, golden suite green (17 passed). Verified the sunburst renders 6 rings for both flows with correct deep-chain masses and no branchvalue violations. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
plot_total_mass_balance_error indexed inflows/outflows by process ID (flow.P_Start/P_End) but stock dS by ProcessList position. When a study's processes are not stored in ascending-ID order (e.g. P12 defined before P11), a process's flows were paired with a different process's stock, producing a phantom mass-balance error on the mislabelled bar. Index all three arrays and the per-process loop consistently by p.ID, sized to the highest process ID. No-op for studies already in ID order. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
flow_edit_save updated from_process/to_process but never re-derived the F_<from>_<to> ID, so editing a flow's endpoints left a stale ID that disguised the real topology and could hide duplicate edges (a stray F_09_17 wired 12->17 masked a second P12->P17 edge). Re-derive the convention ID on endpoint change via _next_flow_id/_rename_flow_id: collision-safe (a second edge into the same pair becomes F_.._2, surfacing duplicates) and leaving deliberately custom IDs untouched. Adds three route tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e restructuring Safety net for the upcoming main.py module split: the exact ordered route table, the symbols other code imports from systemdefiner.main, and a study built through every editor whose pages must all render. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…r modules Move-only: deps (templates/Jinja env), forms (form+YAML parsing), cascades (rename/purge/delete/renumber), health (_model_health), scenario_params (parameter catalog), and one APIRouter per domain under routers/. main.py keeps the app factory, exception handlers, router includes in the original declaration order, and backwards-compatible re-exports. Route table pinned byte-identical by test_route_inventory; all 150 app tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…stry New consistency.py: iter_flow_pointers() is the single enumeration of every scalar flow-ID pointer (FOMP/LFG/FlowCap outflows + the DSM_Component spare-part flows the cascades previously missed); check_config_consistency() reports dangling/retargeted scenario+MC parameter names, stale element keys, TC ownership mismatches and duplicate derived TC names, duplicate process/flow/element IDs, flow-ID/endpoint drift and duplicate edges, stranded flow data/compositions on non-input flows, config-wide static TC sums, and hierarchy defects (unknown members, multi-parent, cycles). _model_health merges the report into the overview page. All 16 tutorial studies are clean; the checker surfaces real pre-existing damage in user studies (drifted flow IDs, orphaned MC params, stale element keys). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- _rename_flow_id/_purge_flow_references/_compact_process_ids now iterate
consistency.iter_flow_pointers, covering the DSM_Component spare-part
flow fields that were previously left dangling on rename/rewire/delete.
- processes/renumber remaps scenario + MC parameter names: exact flow IDs
via the rename map, embedded process IDs in P{pid:02d}_*, TC_E{n}_ff_tt,
TC_ff_tt and TC_Cap_{pid} names, and auto-derived FlowCap cap_tc_id keys.
Previously all of these silently dangled (or retargeted) after renumber.
- Renumber's flow-ID rename now includes _N duplicate-edge suffixes
(F_02_06_2 no longer drifts) and uses a two-pass park-then-assign scheme
so the outcome cannot depend on flow list order; genuine collisions get
a suffix via _next_flow_id instead of being silently skipped.
- _delete_process_cascade drops scenario/MC entries whose names embed the
deleted process, so a later process reusing the freed ID cannot inherit
another process's modifications.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nge, import - flow edit: TC ownership moves to the new source process on rewire (previously the stale TC exported under the new endpoints' names and silently overrode later edits); flow data/compositions are dropped when a rewire moves the flow off an Input source. - flows new/edit: convention-shaped IDs (F_<a>_<b>[_N]) are rejected when they don't match the actual endpoints — fake convention names disguised topology and hid duplicate edges. - process edit: leaving BOM_Assembler drops the process's bom_assembly entry (mirrors the initial-stock cleanup); leaving Input drops flow data/compositions on its outgoing flows (they were invisible in the editors but still prescribed the flows in the engine). - import: a workbook without a BOM sheet no longer wipes existing BOM config; preserved TCs/refs pointing at vanished flows/processes are purged (they exported junk TC_E*_00_00 parameters); duplicate process/flow/element IDs are rejected with a 422 on both the Excel and YAML import paths. - tests: fixtures that wired flows inconsistently with their convention IDs (F_01_02 wired 0->1) renumbered to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… config
The elements editor previously rewrote only cfg.model.elements: renaming an
element orphaned every value keyed by the old name (TC values, dynamic TC
points, compositions, BOM fractions, initial-stock compositions, DSM
component lifetimes/elements, expected inflow compositions, flow_data
element) — the engine silently read them as 0 — and reordering retargeted
positional E{n} scenario/MC names at a different element.
Each editor row now carries a hidden element_{i}_orig field (renumbered by
the same client JS as the visible input). elements_save builds a rename map
and removal set from (orig, new) pairs, rewrites keys everywhere via
consistency._element_key_sites, drops keys of removed elements (incl. DSM
component rows tracking them), remaps E{n} positions and embedded element
names in TC_E*/P*_IS_E* scenario+MC parameter ids, and applies the same
mapping to the hierarchy path cells submitted in the same form. The
element_{idx} scan is now gap-tolerant like the other row parsers. Posts
without _orig fields keep the old replace-only behavior.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Same defect fixed in plotting/validation.py (332b047), missed in the scenario_engine copy: dS was accumulated at the ProcessList position while flows were accumulated at the process ID, so a non-ID-ordered process list (which the SystemDefiner produces routinely — process_new appends a gap-filling ID at the end) attributed stock changes to the wrong process and reported phantom imbalances. Flows touching IDs >= len(ProcessList) were silently dropped from the balance, hiding real imbalances in gapped studies. Arrays are now sized by the highest process/flow-endpoint ID, indexed by ID throughout, and gap rows are masked out. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…orphan BOM save_case_study now sorts cfg.processes by ID (process_new appends gap-filling IDs at the list end, and several consumers historically assumed list position == process ID) and drops bom_assembly entries whose process is gone or no longer a BOM_Assembler, mirroring the existing initial-stock prune. Tracked case-study YAMLs normalize lazily on their next save. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- _parse_dsm / _parse_dsm_component now collect row indices with tolerant regex scans (like _parse_lfg/_parse_flowcap) instead of breaking at the first missing index — a client-side row removal without reindexing no longer silently drops every row past the gap; per-category component lifetime overrides key on the component's actual DOM index. - _rules_to_paths guards against hierarchy cycles with a visited set — a cyclic parent/child pair used to hang every page rendering the matrix (elements, compositions, initial stock, dynamic TCs). - scenario names may not contain slashes (they are URL path segments; a slash made the scenario unreachable and undeletable). - dynamic TC form parsing is anchored on the outgoing-flow IDs and known element names instead of a generic regex, so flow IDs containing _idx_/_year_ can no longer mis-bucket values. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…test load_case_study only setdefault'ed the internal YAML name, so a hand-copied study carrying another study's 'name:' (found live: heatpumps_1 contains 'name: tracer') had every save silently written to the OTHER study's folder. The folder name now overrides the stored name on load. New round-trip test: every tracked case study loads, saves (applying the new normalization), reloads with no new consistency errors, and save/load is idempotent — this is what surfaced the stale-name bug. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment/docstring-only: the code<->paper notation tables and inline references now point at the renumbered sections of bioDYM_mathematical_formulas.md (e.g. DSM §5-6 -> §3.2-3.3, hierarchy §2.6 -> §6.2). No runtime changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
load_dsm_from_yaml now emits the per-process 'components' list (element, mean lifetime, spare-part in/outflow IDs, optional per-category lifetime overrides), matching what load_dsm_parameters merges from the 3_1_DSM_Components sheet — previously DSM_Component studies only worked when loaded from Excel. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The two tutorial configs were re-saved through the SystemDefiner, which added new schema defaults (mc_seed, solver_strict/max_iterations, sankey_title, expected_inflow_composition, refs) and reordered keys — no parameter values changed. References regenerated via generate_references.py; all flow/stock arrays are bit-identical to the previous pins, only the embedded config hash differs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Summary
A config-mutation audit of the SystemDefiner found 12 classes of silent inconsistency: mutations that leave
config.yamlvalid — it loads and solves — but internally wrong, so scenario/MC parameters silently retarget, TCs go stale, and element edits orphan stored values. This PR restructures the app and fixes all of them, plus a 13th found during verification.Restructuring (move-only, pinned by tests)
main.py(3,119 lines) split intorouters/(one APIRouter per domain) +forms.py,cascades.py,health.py,scenario_params.py,deps.py. Route table pinned byte-identical by a route-inventory test.Consistency fixes
consistency.py:iter_flow_pointers()— single registry of every scalar flow-ID pointer field (new schema fields register once, all cascades pick them up) — andcheck_config_consistency(), merged into the overview health report and asserted clean after every mutation route in the test suite.P{pid}_…/TC_E{n}_{ff}_{tt}/TC_Cap_{pid}names, FlowCapcap_tc_id), renames_N-suffixed duplicate-edge flow IDs, and is order-independent (two-pass rename).F_<from>_<to>[_N]) validated against endpoints on create/edit.E{n}parameter names (template carries per-row original names).scenario_engine.check_mass_balanceindexed by process ID, not list position; ID gaps no longer silently drop flows (same fix as plotting/validation.py, 332b047).name:used to have every save written into the other study's folder (found live inheatpumps_1).Tests
Known follow-ups (not in this PR)
TC_Cap_02no-ops) — fix via UI.E{n}) would remove that bug class entirely but needs an Excel-template migration.🤖 Generated with Claude Code