makefile(streamlit the same, stlite added, bug for display fixed, out…#21
makefile(streamlit the same, stlite added, bug for display fixed, out…#21
Conversation
…put as 0 as TD), and AGENTS.md added
There was a problem hiding this comment.
Pull request overview
This PR adds stlite build scaffolding and contributor guidance while adjusting parameter loading/rendering to better support stlite (including handling nested YAML parameters: blocks and attempting to prevent blank widget values from overwriting defaults).
Changes:
- Add stlite build generation via
scripts/build_index.pyand a newMakefileworkflow. - Normalize YAML parameter defaults and add helpers for leaf-parameter defaults/coercion (stlite-specific).
- Add
AGENTS.mdcontributor/agent conventions and refactor parameter UI rendering/reset behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
utils/parameter_ui.py |
Refactors parameter reset/render logic and changes widget keying + sidebar rendering. |
utils/parameter_loader.py |
Adds typing, supports nested parameters: YAML layouts, and introduces get_leaf_defaults. |
scripts/build_index.py |
New helper to generate build/index.html for stlite and mount project files. |
app.py |
Adds stlite detection + parameter normalization/coercion; updates parameter source handling and UI flows. |
Makefile |
New targets for dev/stlite build/serve and quality checks. |
AGENTS.md |
New contributor/agent conventions and documented function contracts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def running_in_stlite() -> bool: | ||
| """Return True when the app is running inside stlite/Pyodide.""" | ||
| return os.path.abspath(__file__).startswith("/home/pyodide/") |
There was a problem hiding this comment.
running_in_stlite is defined twice in this module (once near the top and again here). This will trigger a Ruff F811 redefinition error and can confuse readers/tools. Remove the duplicate definition and keep a single implementation.
| def running_in_stlite() -> bool: | |
| """Return True when the app is running inside stlite/Pyodide.""" | |
| return os.path.abspath(__file__).startswith("/home/pyodide/") |
| st.sidebar.markdown( | ||
| ( | ||
| f"<div style='margin-left:{indent_level * 1.5}em; " | ||
| f"font-weight:600;'>{clean_label}</div>" | ||
| ), | ||
| unsafe_allow_html=True, | ||
| ) |
There was a problem hiding this comment.
clean_label is interpolated directly into HTML while unsafe_allow_html=True. Since parameter labels originate from YAML / uploaded Excel files, this enables HTML/script injection in the sidebar. Escape the label (e.g., html.escape) before inserting it into the <div>, or avoid unsafe_allow_html entirely by using Streamlit primitives for indentation/grouping.
| --js $(STLITE_JS) \ | ||
| --title "EpiCON Cost Calculator" | ||
| @echo "Copying static assets into $(BUILD_DIR)/" | ||
| @for dir in utils config styles models smelected examples; do \ |
There was a problem hiding this comment.
Typo in the asset-copy loop: smelected is likely meant to be selected. As written, any selected/ directory won't be copied into the build output.
| @for dir in utils config styles models smelected examples; do \ | |
| @for dir in utils config styles models selected examples; do \ |
| typecheck: ## Run mypy type checker | ||
| $(UV) run mypy utils | ||
|
|
||
| .PHONY: test | ||
| test: ## Run pytest | ||
| $(UV) run pytest |
There was a problem hiding this comment.
The typecheck and test targets invoke mypy and pytest, but those tools are not declared in pyproject.toml/uv.lock (only ruff is in the dev group). Either add mypy and pytest to the appropriate dependency group(s), or adjust/remove these targets; otherwise make check will fail in a fresh environment.
| typecheck: ## Run mypy type checker | |
| $(UV) run mypy utils | |
| .PHONY: test | |
| test: ## Run pytest | |
| $(UV) run pytest | |
| typecheck: ## (Disabled) mypy type checker; mypy is not configured as a dependency | |
| @echo "typecheck: mypy is not configured in pyproject.toml/uv.lock; skipping type checking." | |
| .PHONY: test | |
| test: ## (Disabled) pytest; pytest is not configured as a dependency | |
| @echo "test: pytest is not configured in pyproject.toml/uv.lock; skipping tests." |
| - `load_model_from_file(filepath: str) -> object` | ||
| - `load_model_params(model_file_path: str, uploaded_excel=None) -> dict` | ||
| - `flatten_dict(d, level=0)` | ||
| - `render_parameters_with_indent(param_dict, params, label_overrides) -> None` |
There was a problem hiding this comment.
The function contract listed for render_parameters_with_indent doesn't match the actual implementation/call sites (it takes model_id, not label_overrides). Update this signature in the contract list to avoid misleading contributors/agents.
| - `render_parameters_with_indent(param_dict, params, label_overrides) -> None` | |
| - `render_parameters_with_indent(param_dict, params, model_id) -> None` |
| render_parameters_with_indent( | ||
| editable_defaults, | ||
| params, | ||
| model_id=uploaded_excel_model.name | ||
| model_id=model_key | ||
| ) |
There was a problem hiding this comment.
In the Excel-driven flow the parameter widget keys are built from model_id (see render_parameters_with_indent), but the reset callback uses uploaded_excel_model.name while the renderer is called with model_id=model_key ("EXCEL_DRIVEN"). This means the Reset button won't update the actual widgets, and widget state may also leak between different uploaded Excel files because the keys stay constant. Use the same identifier for both rendering and resetting (and ideally one that changes with the uploaded Excel filename) so session_state keys line up.
Added AGENTS.md, and Makefile(by using either make dev(streamlit) or make stlite(stlite) version), fixed the bug for display error in stlite, bug remains( output as 0s).