Refactor models and improve UI with report enhancements#35
Refactor models and improve UI with report enhancements#35olivia-banks wants to merge 32 commits intoEpiForeSITE:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates EPICC from hardcoded Python model classes (and the Excel-driven model runner) to a declarative YAML model system that is compiled at runtime, alongside a substantial UI refactor that introduces a two-column layout and a richer, block-based interactive reporting experience (including Plotly graphs and export/print support).
Changes:
- Introduces YAML-defined models compiled at runtime into
BaseSimulationModelinstances, plus a safe-ish equation evaluation/validation pipeline. - Replaces the previous sidebar-heavy UI with modular
epicc.uicomponents (state mgmt, parameter panel, report rendering, export). - Adds report blocks (markdown/table/graph/figure), Plotly graph rendering, and parameter export enhancements (YAML/XLSX w/ descriptions).
Reviewed changes
Copilot reviewed 46 out of 51 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Adds Plotly (and its transitive deps) to the locked dependency set. |
| pyproject.toml | Adds Plotly dependency; updates stlite title casing. |
| tests/epicc/test_model_loader.py | Removes tests for the old load_model path. |
| tests/epicc/test_loader.py | Adds tests for registry-based loading via get_all_models(). |
| tests/epicc/test_interpreted_model.py | Adds tests for runtime-compiled models, run-output shape, and basic report block integration. |
| tests/epicc/test_formats_yaml.py | Minor cleanup (removes module docstring header). |
| tests/epicc/test_formats_xlsx.py | Adds regression tests for no-template writes and emitting field descriptions. |
| tests/epicc/test_formats_template.py | Minor cleanup (removes section banner comments). |
| tests/epicc/test_formats.py | Minor cleanup (removes module docstring header). |
| tests/epicc/test_evaluator.py | Adds coverage for equation evaluation, dependencies, cycles, and error messages. |
| tests/epicc/test_ast_validator.py | Adds coverage for AST validation and compilation/dependency extraction. |
| src/epicc/web/sidebar.css | Updates styling for new two-column layout, Plotly UI tweaks, and print-specific rules. |
| src/epicc/utils/section_renderer.py | Removes old section-based rendering helper. |
| src/epicc/utils/parameter_ui.py | Removes old sidebar parameter rendering/reset logic. |
| src/epicc/utils/model_loader.py | Removes old built-in Python model instantiation. |
| src/epicc/utils/excel_model_runner.py | Removes deprecated Excel-driven model runner (per issue #26). |
| src/epicc/ui/styles.py | Adds centralized CSS loading for the app. |
| src/epicc/ui/state.py | Adds session-state helpers for model switching, results caching, and upload identity tracking. |
| src/epicc/ui/report.py | Adds report renderer + block renderers (markdown/table/graph/figure) using Streamlit + Plotly. |
| src/epicc/ui/parameters.py | Adds typed parameter UI rendering (widgets by schema), grouping, validation display, and upload handling. |
| src/epicc/ui/export.py | Adds parameter export modal and PDF/print trigger logic. |
| src/epicc/ui/init.py | Re-exports parameter UI helpers. |
| src/epicc/models/tb_isolation.yaml | Removes legacy default-params YAML for the old Python TB model. |
| src/epicc/models/tb_isolation.py | Removes legacy Python TB model implementation. |
| src/epicc/models/measles_outbreak.yaml | Removes legacy default-params YAML for the old Python measles model. |
| src/epicc/models/measles_outbreak.py | Removes legacy Python measles model implementation. |
| src/epicc/model/schema.py | Refactors the canonical model schema to support groups, scenarios, and report blocks (incl. graphs). |
| src/epicc/model/parameters.py | Adds formatting helper for displaying computed values. |
| src/epicc/model/models/tb_isolation.yaml | Adds new declarative TB model definition (params/equations/scenarios/report). |
| src/epicc/model/models/measles.yaml | Adds new declarative measles model definition (params/equations/scenarios/report). |
| src/epicc/model/models/init.py | Adds model registry + loader for YAML models embedded as package resources. |
| src/epicc/model/factory.py | Adds runtime model compilation (dynamic param model + dynamic BaseSimulationModel subclass). |
| src/epicc/model/evaluator.py | Adds dependency-resolving equation evaluation engine. |
| src/epicc/model/base.py | Removes section-building requirement; adds optional parameter schema/group hooks for richer UI. |
| src/epicc/model/ast_validator.py | Adds AST-based equation validation/compilation and dependency extraction. |
| src/epicc/model/init.py | Updates public API exports for the new model/evaluator/validator system. |
| src/epicc/js/print_results.js | Adds a minimal window.print() script for print/PDF export. |
| src/epicc/formats/yaml.py | Adds mime_type/label; adds support for emitting Pydantic field descriptions as YAML comments. |
| src/epicc/formats/xlsx.py | Adds mime_type/label; adds no-template XLSX writing + optional description column from Pydantic model. |
| src/epicc/formats/base.py | Adds mime_type and label fields to the BaseFormat API. |
| src/epicc/formats/init.py | Adds iter_formats() and refactors suffix iteration through it. |
| src/epicc/main.py | Replaces the monolithic UI with a two-column layout using the new epicc.ui.* modules and report renderer. |
| app.py | Switches to runpy.run_module for running epicc.__main__. |
| Makefile | Minor build target tweak (removes $(DIST_DIR) prerequisite). |
| plan.md | Removes obsolete development plan document. |
| AGENTS.md | Partial updates, but still contains outdated architecture/path guidance relative to this refactor. |
| .gitignore | Adds .ruff_cache/. |
| .devcontainer/devcontainer.json | Updates devcontainer name casing. |
| sample/~$TB Isolation.xlsx | Adds/updates sample artifact (temp Office file). |
| sample/TB Isolation.xlsx | Adds/updates sample workbook. |
| sample/Measles Outbreak.xlsx | Adds/updates sample workbook. |
Comments suppressed due to low confidence (1)
AGENTS.md:23
- AGENTS.md still describes the old architecture (Python modules under
models/, utils-based flow, built-in models undersrc/epicc/models/). This PR moves built-in models to declarative YAML undersrc/epicc/model/models/with runtime compilation, and UI helpers moved undersrc/epicc/ui/. Please update this section (and the flow/path references below) so contributor guidance matches the new codebase layout.
The current app supports Python + YAML model flows:
- A Python module in `models/` implements model logic.
- A paired YAML file provides default parameters.
- `app.py` loads the Python module, loads YAML defaults, renders parameter inputs,
runs the model, and renders sections.
Current high-level flow:
`discover_models() → load_model_from_file() / load_model_params() → render_parameters_with_indent() → run_model() → build_sections() → render_sections()`
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 47 out of 52 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 47 out of 52 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """A named visual group of parameters or nested sub-groups.""" | ||
|
|
||
| label: str | ||
| children: list["str | ParameterGroup"] = Field(default_factory=list) |
There was a problem hiding this comment.
ParameterGroup.children is annotated as list["str | ParameterGroup"]. With from __future__ import annotations, this becomes a list['str | ParameterGroup'] generic alias (inner arg is a string literal), which Pydantic is unlikely to resolve as a union type and can break parsing/validation of groups in YAML model definitions. Change this to a real union annotation (e.g., list[str | ParameterGroup] or list[str | "ParameterGroup"]) so nested group trees validate correctly.
| children: list["str | ParameterGroup"] = Field(default_factory=list) | |
| children: list[str | ParameterGroup] = Field(default_factory=list) |
| from epicc.formats import get_format, iter_formats | ||
| from epicc.formats.base import BaseFormat | ||
| from epicc.ui.state import has_results, _PRINT_REQUESTED_KEY, _PRINT_TOKEN_KEY | ||
|
|
There was a problem hiding this comment.
export.py imports _PRINT_REQUESTED_KEY / _PRINT_TOKEN_KEY from ui.state, but the leading underscores indicate these are internal implementation details. Consider exposing a small public API in ui.state (e.g. request_print() / consume_print_request()) so other modules don’t rely on private session-state key names.
| # Allowed method names on objects | ||
| SAFE_METHODS: set[str] = { | ||
| "count", | ||
| "index", | ||
| "get", | ||
| "keys", | ||
| "values", | ||
| "items", | ||
| } |
There was a problem hiding this comment.
SAFE_METHODS includes count/index, but _validate_attribute_node() only permits attributes in {'get','keys','values','items'}. Since SAFE_METHODS is exported from epicc.model, this mismatch is confusing and can lead to incorrect assumptions about what’s allowed in equations. Either enforce SAFE_METHODS in _validate_attribute_node() or update/remove entries so the constant reflects reality.
| uploaded = ct.file_uploader( | ||
| "Load parameters from file", | ||
| type=sorted(VALID_PARAMETER_SUFFIXES), | ||
| accept_multiple_files=False, | ||
| ) |
There was a problem hiding this comment.
file_uploader() is created without an explicit key. That makes it hard/impossible to reliably clear its widget state when switching models (even though sync_active_model() tries to clear related caches), and can cause the previously uploaded parameter file to persist into a different model selection. Consider giving the uploader a model-scoped key (e.g. derived from model_key) and clearing that key on model switch.
| /* Fix model selector alignment. TODO: be more specific, `:first-child`? */ | ||
| .stVerticalBlock { | ||
| justify-content: center; | ||
| } |
There was a problem hiding this comment.
The .stVerticalBlock { justify-content: center; } rule is very broad and will apply to many Streamlit layout containers, potentially causing unintended alignment changes across the app. It would be safer to target the specific model selector container with a more specific selector (e.g., a data-testid chain or a .st-key-* wrapper).
This is an overhaul of EPICC that touches pretty much every part of the codebase. The main thrust is moving from hardcoded Python model classes to a declarative YAML-based system with runtime compilation as per Wednesday's meeting. Because of the flexibility the YAML approach brings, as well as the extra information expressed therein, this required a redux of the UI/interface, along with a proper interactive reporting system. This has been a long time coming and should make the codebase way more maintainable and extensible going forward.
Oh! And the weird only-sometimes-reproducable bug was fixed. It was a caching and import error, see my comment on this GitHub issue.
YAML Models
The model system (the impetus for this PR in the first place) has been implemented. Instead of writing Python classes that inherit from
BaseSimulationModel, we now just write a YAML (or XLSX, I guess) file that declares your parameters, equations, scenarios, and report structure. The oldMeaslesOutbreakModelandTBIsolationModelPython classes are gone, replaced by clean YAML definitions insrc/epicc/model/models/that are way easier to read and edit. I've also completely removed the old Excel model runner as per #26. The YAML approach is declarative, type-safe (validated against a Pydantic schema), and doesn't require any Python knowledge to modify.To make this work, there's some pretty neat metaprogramming happening in the model factory. For each YAML file, we dynamically construct two classes at runtime using
type()andcreate_model(), those being:BaseSimulationModelsubclass with all the abstract methods implemented as closures that capture the model definition and evaluator.This means we get full type safety and validation without writing any boilerplate, since the YAML compiles directly to a working Python class.
Equation Evaluation
The equation evaluation engine is also implemented, taking some of @EddW1219's old validation code. This uses Python's
astmodule to walk the parse tree and validate that only safe operations are used (noimports, noeval(), no attribute access to dangerous methods, et cetera). After validation, we compile to Python's bytecode and extract variable dependencies by findingNamenodes withLoadcontext. These dependencies are sorted to determine execution order and detect cycles (with helpful error messages!). Evaluation happens in a restricted namespace with only safe built-ins, and as each equation executes, its result gets added to the namespace for dependent equations.The error handling is pretty good too, if you reference an undefined variable, it'll use
difflibto suggest what you probably meant.Reports
The report rendering system is now much more complicated and capable. Instead of the old "take an introduction and tack on some tables" approach, we now have a heterogeneous list of content blocks (markdown, table, graph, figure) defined in the YAML. Each block type has its own renderer class, decoupled from model execution so they can show either real results or skeleton placeholders depending on whether you've run the simulation yet. This means you can interleave explanations, tables, and charts in whatever order makes sense for your model instead of being stuck with a fixed layout.
It also means that we can display information about a model and it's various parameters before a model is run, which is a boon for accessibility and exploration.
I chose Plotly for the graphing library; we get interactive visualizations that are way nicer than static plots, and they look like they belong in Streamlit.
GraphBlocklets you specify chart type so far (bar, stacked bar, line, pie), title, caption, which scenarios to include, and which equations to plot. The renderer translates this into Plotly figure objects with proper theming and layout. For bar charts, each equation becomes a trace with scenarios on the x-axis. For line charts, each equation is a series. For pie charts, we show the breakdown across equations for a single scenario. All the charts have interactive hover tooltips and zoom controls built in. I didn't feel it was necessary to allow arbitrary Python graphing code (this would also be leagues harder to try and secure), but if I'm wrong please feel free to tell me and I will fix this.We can export PDFs of reports, either by the Command-P (Control-P?) or browser button everyone already knows, or by the built in button. I'm still trying to get Plotly to cooperate, sometimes it's overlapping with the text.
UI/Interface
Because of all the new information present in the application and models, the old sidebar-based layout was getting crowded and didn't seem like the right tool for the job. Plus, the old
__main__.pywas nearly 500 lines of spaghetti, so I split it into modules underepicc.ui. The big visual change is a two-column layout where parameters live in a sticky left column and results render in the right column. The parameter column stays visible as you scroll through results, and there's proper state management to handle file uploads, model switching, and result caching without getting into weird inconsistent states (as far as my testing goes).The parameter UI now supports rich input types instead of the old text-input-for-everything approach. Enums get proper dropdowns with human-readable labels, booleans get checkboxes, and numeric parameters get number inputs with min/max validation enforced at the widget level. There's also a grouping system where you can organize parameters into collapsible, nested sections in the YAML (top-level groups become Streamlit expanders and nested groups get bold headers). The widget rendering code walks the group tree recursively and uses parameter specs to generate the right widget type.
I added a parameter export feature (#23) that lets you save your current configuration to any of the registered formats. These files are exported with comments (originating from the model YAML), which makes it easy to understand what exactly you're looking at.
I know this is a very large PR that touches a lot of things, so please let me know how I can help aide the review process (especially when we meet later today). That's why I wrote a decent bit here and attached screenshots, so hopefully it was a little easier to navigate :)