fix: Remove dead Flask-era code, fix lint errors, add docs/assets fal… - #4
Merged
Merged
Conversation
…lback An external review of main flagged several potential integration issues. Verified against the actual codebase: temporal_split, fit_model(config=...), exact top-K alert ranking, currency-aware behavioral windows, and the FastAPI/model feature schema were already consistent (test_leakage.py already covers top-K-on-ties and currency-aware windows). The following were real issues and are fixed here: **Dead code removal:** - Deleted src/api/app.py (orphaned Flask app, unreachable, not imported anywhere), src/api/schemas.py (Marshmallow, Flask-only), src/api/openapi.yaml (hand-written spec superseded by FastAPI's auto-generated schema), src/api/templates/ (Flask Jinja template superseded by public/index.html), and src/models/inference.py (duplicate of src/api/inference.py, only consumer was the deleted app.py) - Removed dead threshold_for_alert_rate() from metrics.py (quantile-based, unused everywhere -- top_k_alert_mask is the only path actually used) **Lint (ruff check was failing with 23 errors -- CI would have failed):** - Fixed import ordering and removed unused imports across affected files - Removed f-strings without placeholders in train_model.py **Dashboard resilience:** - results_loader.py now falls back to the committed docs/assets/ snapshot when reports/*.csv or reports/figures/*.png don't exist locally, so a fresh clone shows real published results without retraining first (verified by simulating a clean clone with no local reports/ output) **Docs:** - Fixed stale "Flask" mention in the tech-stack line (README already documented FastAPI correctly everywhere else, and already embedded all 6 result charts) All 60 tests pass, ruff check is clean. Co-Authored-By: Claude Sonnet 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
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.
…lback
An external review of main flagged several potential integration issues. Verified against the actual codebase: temporal_split, fit_model(config=...), exact top-K alert ranking, currency-aware behavioral windows, and the FastAPI/model feature schema were already consistent (test_leakage.py already covers top-K-on-ties and currency-aware windows). The following were real issues and are fixed here:
Dead code removal:
Lint (ruff check was failing with 23 errors -- CI would have failed):
Dashboard resilience:
Docs:
All 60 tests pass, ruff check is clean.