Add FastAPI service to web_app and refactor Streamlit to consume it - #22
Open
will-fawcett wants to merge 25 commits into
Open
Add FastAPI service to web_app and refactor Streamlit to consume it#22will-fawcett wants to merge 25 commits into
will-fawcett wants to merge 25 commits into
Conversation
Captures the agreed design: split web_app/ into core/, api/, ui/; add a FastAPI service that owns the model and time index; refactor Streamlit to consume the API for predictions while keeping direct Zarr access for AIA images. Pulls auth/batch/image-streaming out of scope. The spec is the input to writing-plans; it locks the contract (3 endpoints + /health), file layout, deps pinning policy, and known risks (cold-cache startup, empty DATA_TEST fixture) before any code moves.
…o parquet - Tests assume a configured data backend on the running machine (S3 or real LOCAL_DATA_ROOT). No checked-in DATA_TEST fixture; tests run on the deployment server where data already lives. - Switch the time-index cache from CSV to parquet. The cache becomes a shared file (API writes, UI reads) and parquet round-trips dtypes cleanly. pyarrow is already a transitive streamlit dep, so it's free on the UI side and a small explicit add on the API side. - Parameterize the docker-compose data path so the same compose file works locally and on the server with an env override.
Translates the design spec into 15 bite-sized tasks: file restructure, parquet cache swap, Pydantic schemas, FastAPI app + lifespan + /health, /info, /predict, /predict-range, api Dockerfile, ui api_client, Streamlit refactor, slimmer ui Dockerfile, two-service docker-compose, end-to-end smoke test. Each task ships with TDD test code, expected output, and a commit step.
Move inference, data access, model, and checkpoints into core/. Move Streamlit + assets + Dockerfile into ui/. Add empty api/ that later tasks populate. Imports and the data_access cache path are updated to match the new layout. Streamlit asset paths are prefixed with ui/. The pickle-time sys.modules aliasing in inference.py is preserved unchanged so the existing checkpoint still unpickles.
Pytest rootdir is anchored at web_app/ so 'from core import ...' and 'from api import ...' resolve in test files. The fixture in api/tests/conftest.py prints a soft warning if the data backend is not configured, since lifespan-exercising tests need real data.
The cache becomes a shared artefact between the API (writer) and the UI (reader) once the FastAPI service lands. Parquet preserves dtypes without a parse_dates hint and round-trips the DatetimeIndex cleanly. pyarrow is the engine and is already a transitive streamlit dep, so the UI gets parquet support for free. Old CSV caches are not migrated; first run rebuilds the index.
The /predict endpoint needs to return the snapped timestamp it actually ran inference on, not the raw request. This helper centralises the 36-minute rounding and the in-range bounds check, returning None when the request falls outside the indexed range so the endpoint can map that to a 422.
Covers /info, /predict, /predict-range, and /health. The PredictRangeRequest validator rejects inverted ranges with a 422 at the framework boundary so endpoint code can assume a valid range. Equal endpoints are accepted (a zero-width range is valid; it just matches one or zero timestamps).
InfoResponse.model_name conflicts with Pydantic's reserved 'model_' prefix. The field name is part of the API contract, so opt out of the namespace check rather than rename.
Lifespan loads the model + time index once on startup and stores them on app.state; endpoints read state from there. The _state['ready'] flag flips to true only after the load completes, so /health returns 503 during startup and 200 once the API is ready to serve.
Returns model name, AIA wavelength list, EVE ion list, and the inclusive bounds of the indexed date range. Clients use this to discover the queryable surface without trial and error.
The endpoint snaps the request to the nearest indexed timestamp via core.data_access.find_nearest_indexed_timestamp and returns the snapped value alongside the 38 ion predictions. Out-of-range and unparseable timestamps return 422; valid in-range requests always succeed because every indexed timestamp has corresponding data.
Filters indexed timestamps to the inclusive [start, end] window and runs inference on each. Returns count + records; an empty window is HTTP 200 with count=0 (not an error). Inverted ranges are rejected by the schema validator with 422.
Two-stage build mirroring the existing ui Dockerfile. New deps for the service layer use compatible-release pins (~=) so patches flow but minor-version surprises are blocked. Shared deps stay on >= matching the existing style. HEALTHCHECK runs curl against /health every 10s with a 60s start grace period for model load.
predict_range returns a DataFrame indexed by timestamp so the existing Streamlit plot code works unchanged. The other methods return raw dicts. health() does not raise on 503 since the UI uses the body to distinguish 'starting' from 'ready'.
The UI no longer loads the model or imports core.inference. Date bounds, model name, and ion list come from /info; range predictions come from /predict-range via the new APIClient. The AIA image panel still reads Zarr directly (out of scope for the API per the design spec). API_URL is configurable via env var; defaults to http://localhost:8000.
The UI no longer loads the model so torch/torchvision/pytorch-lightning come out, shrinking the image. httpx is added for the api_client. pyarrow is explicit so parquet-cache reads are obvious. Dockerfile paths updated for the new web_app/core + web_app/ui layout; build context is web_app/ root.
api + ui services with parameterised HOST_DATA_PATH so the same compose file works locally and on the deployment server. The ui service waits for the api healthcheck to pass before starting, so Streamlit's first request never races the API's model load.
The showcase default 2017-09-06 falls outside the indexed range on deployments with limited data (e.g. test fixtures covering only 2011), crashing st.date_input. Clamp the default into [date_min, date_max] so the widget renders cleanly regardless of the data window.
…string - README.md was still describing the pre-refactor flat layout, with outdated install/run instructions and references to the deleted top-level Dockerfile and main.py. Rewrite it for the api+ui split, including endpoint surface, configuration table, and test command. - Drop unused 'import io' and 'import numpy as np' from ui/main.py (leftovers from the inference path that moved to the API). - Fix the test_api.py module docstring: it incorrectly told readers to skip with -k 'not requires_data' but no tests are marked. Replace with the actual recommendation.
The FastAPI lifespan builds the AIA time index on first run by reading
T_OBS from every year/wavelength .zattrs. It used to slurp all ~126
timestamp lists into memory at once and parse each with
pd.to_datetime(format="mixed") — on a ~3 GiB deployment box that
transient spike, on top of an already-resident torch, got the container
OOM-killed (exit 137) during startup.
Now process one wavelength at a time: read its years, parse, fold into
the running inner-join, then drop the raw lists before the next
wavelength. Peak memory is ~1/9th of before. Also:
- parse with format="ISO8601" (the actual T_OBS format) instead of the
slow per-element "mixed" path; errors="coerce" + dropna so one bad
value doesn't abort startup
- raise a clear error if no year dirs are found or the join is empty,
instead of a confusing pd.concat([]) / silent-empty failure
Once the parquet cache is written the whole scan is skipped, so this
only matters for the first start (or after a cache wipe).
Introduce a MODEL_CADENCE constant in core.data_access with a comment explaining why everything is snapped to a 36-minute grid: AIA records its 9 wavelengths a few seconds apart so they need a shared bin to be joined into one row, and 36 min is the cadence the checkpoint was trained at. Use the constant in build_time_index, find_nearest_indexed_ timestamp and get_aia_image instead of the bare "36min" literal, and add matching notes to the Streamlit sidebar and the README.
… kill When the api container is SIGKILL'd during the lifespan, block-buffered stdout means the build_time_index progress lines never appear, hiding where it died. Unbuffered output makes the failure point visible.
…kage `streamlit run ui/main.py` inserts the script's directory (/app/ui) onto sys.path rather than the working directory, so `from core.data_access import …` raised ModuleNotFoundError in the ui container. The api container only avoided this by accident — uvicorn adds its cwd. Set PYTHONPATH=/app explicitly in both images so the layout no longer depends on the launcher's path quirks.
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
web_app/api/) exposing/health,/info,/predict,/predict-range. The service owns the model and the parquet-backed time-index cache; lifespan loads them once on startup.web_app/into three subpackages:core/(shared inference + data access),api/(new service),ui/(existing Streamlit, refactored). The UI no longer loads the model — predictions go through the API; AIA images are still read from Zarr directly per the design.docker-compose.ymlwith healthcheck-driven startup so the UI never races the API's model load. The UI image drops torch (and friends) and shrinks substantially.The design spec and implementation plan are checked into
docs/superpowers/specs/anddocs/superpowers/plans/for context.Test Plan
cd web_app && DATA_BACKEND=local LOCAL_DATA_ROOT=/path/to/sdomlv2a pytest→ 26 passing.HOST_DATA_PATH=/path/to/sdomlv2a docker compose up --build, then verify:curl localhost:8000/health→{"status":"ready"}curl localhost:8000/info | jq '.eve_ions | length'→ 38curl -X POST localhost:8000/predict -d '{"timestamp":"<min>"}'→ 38 ion predictionscurl -X POST localhost:8000/predict-range -d '{"start":"<min>","end":"<min+1h>"}'→ count > 0