fix: Refresh artifact under corrected methodology, fix silently-ignor… - #7
Merged
Merged
Conversation
…ed CSVs Completes verification of the temporal-validation and walk-forward fixes against real data, per review checklist. **Real-data verification (all confirmed on the full 9.5M-row dataset):** - Retrained via the corrected train_model.py: test-set metrics are byte-identical to before (PR-AUC 0.9859, ROC-AUC 0.9998) as expected -- the trained model/calibrator don't change, only where the threshold is selected from. Decision threshold moved from 0.001075 (old, selected from the calibration split) to 0.001160 (new, selected from the true held-out validation split). Calibration diagnostics are now honestly slightly worse (calibrated Brier 0.000078 vs 0.000043 previously) since they're now measured on data the calibrator never saw, rather than on its own fitting set. - Walk-forward backtest: 4 real windows spanning the full timeline, PR-AUC 0.938 -> 0.990 as the training window expands (previously: 0 windows, RuntimeError). Added to README as a new Temporal Stability table. - Unseen-entity evaluation re-run against the fresh artifact; added the fixed-production-threshold table to docs/research/unseen_entity_generalization.md -- one threshold (0.001160) applied to both subgroups lands each within noise of the intended 0.5% alert budget independently, recall is indistinguishable (99.3% vs 99.2%), precision differs 11x consistent with the subgroups' 12x prevalence gap. - `python scripts/run_experiments.py --generalization` run end-to-end against a real 284K-row sample of SAML-D (fast mode) to verify the full 6-stage orchestration and --features/--artifact passthrough work correctly; real committed reports/ were untouched (sample used isolated /tmp paths) and diffed byte-identical against docs/assets/ afterward. **Bug found while verifying the above:** docs/assets/ablation_results.csv and docs/assets/typology_results.csv (and their reports/ counterparts) were never actually committed, despite prior commit messages claiming otherwise -- a blanket `*.csv` rule in .gitignore (meant for large raw datasets) was silently swallowing them since the first commit that tried to add them. This meant /api/results would return empty ablation/typology tables on any truly fresh clone, even though the PNG figures and JSON files (unaffected by the csv rule) worked fine -- the fallback-to-docs/assets resilience only half-worked. Fixed: `!docs/assets/*.csv` exception added, both files committed for the first time. All 63 tests pass, ruff clean, API smoke-tested against the refreshed artifact (health/model-info/results all correct, decision_threshold and PR-AUC match the retrained artifact). 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.
…ed CSVs
Completes verification of the temporal-validation and walk-forward fixes against real data, per review checklist.
Real-data verification (all confirmed on the full 9.5M-row dataset):
python scripts/run_experiments.py --generalizationrun end-to-end against a real 284K-row sample of SAML-D (fast mode) to verify the full 6-stage orchestration and --features/--artifact passthrough work correctly; real committed reports/ were untouched (sample used isolated /tmp paths) and diffed byte-identical against docs/assets/ afterward.Bug found while verifying the above: docs/assets/ablation_results.csv and docs/assets/typology_results.csv (and their reports/ counterparts) were never actually committed, despite prior commit messages claiming otherwise -- a blanket
*.csvrule in .gitignore (meant for large raw datasets) was silently swallowing them since the first commit that tried to add them. This meant /api/results would return empty ablation/typology tables on any truly fresh clone, even though the PNG figures and JSON files (unaffected by the csv rule) worked fine -- the fallback-to-docs/assets resilience only half-worked. Fixed:!docs/assets/*.csvexception added, both files committed for the first time.All 63 tests pass, ruff clean, API smoke-tested against the refreshed artifact (health/model-info/results all correct, decision_threshold and PR-AUC match the retrained artifact).