AI: per-series anomaly detection, rate-of-change feature, self-contained retrain scheduler, health endpoint, 41 tests - #3
Conversation
…re, drift detection, cooldown, fallback detector, health endpoint, self-contained retrain scheduler, 41 tests
There was a problem hiding this comment.
🟡 Changes recommended
Critical credential exposure and invalid Compose syntax block approval; additional correctness and reliability findings remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR rebuilds monitex-ai with per-series anomaly models, retraining, fallback detection, health monitoring, and expanded testing.
Changes:
- Adds rate-of-change features, cooldowns, drift detection, and scheduled retraining.
- Adds fallback z-score detection and health metrics.
- Adds evaluation tooling, documentation, Docker/Compose configuration, dependencies, and CI.
Review findings:
docker-compose.yml— critical, 3 votes: hardcoded InfluxDB token requires secret-based configuration and rotation.docker-compose.yml— moderate, 3 votes: service indentation makes the Compose file invalid.monitex-ai/evaluate_model.py— moderate, 3 votes: aggregated jump metrics do not match the report label.monitex-ai/model/README.md— nit, 2 votes: documented 41 tests versus 38 test functions.monitex-ai/model/anomaly_service.py— moderate, 3 votes: cooldown state is recorded before publish success.monitex-ai/model/anomaly_service.py— moderate, 2 votes: delayed initial retraining can leave legacy global models active.monitex-ai/train_model.py— moderate, 3 votes: skipped series can lose their existing models.monitex-ai/train_model.py— moderate, 2 votes: model writes are not atomic.
File summaries
| File | Summary |
|---|---|
monitex-ai/train_model.py |
Per-series training and model persistence |
monitex-ai/tests/test_train_model.py |
Training tests |
monitex-ai/tests/test_payloads.py |
Payload tests |
monitex-ai/tests/test_model_runtime.py |
Runtime tests |
monitex-ai/tests/test_health_server.py |
Health endpoint tests |
monitex-ai/tests/test_fallback_detector.py |
Fallback detector tests |
monitex-ai/tests/test_anomaly_service_cooldown.py |
Cooldown tests |
monitex-ai/tests/conftest.py |
Test import configuration |
monitex-ai/requirements.txt |
Runtime dependencies |
monitex-ai/requirements-dev.txt |
Development and test dependencies |
monitex-ai/model/README.md |
Usage and architecture documentation |
monitex-ai/model/rabbitmq_transport.py |
Retraining queue transport |
monitex-ai/model/payloads.py |
Series-aware anomaly payloads and severity |
monitex-ai/model/model_runtime.py |
Model loading, lookup, scoring, and reload |
monitex-ai/model/health_server.py |
/health endpoint |
monitex-ai/model/fallback_detector.py |
Running z-score fallback |
monitex-ai/model/config.py |
Service configuration |
monitex-ai/model/anomaly_service.py |
Detection, cooldowns, retraining, and health integration |
monitex-ai/LIVE_EVALUATION_REPORT.md |
Evaluation results |
monitex-ai/evaluate_model.py |
Live-data evaluation tooling |
monitex-ai/Dockerfile |
Container and health-port configuration |
docker-compose.yml |
AI service, InfluxDB configuration, and port mapping |
.gitignore |
Python and model artifact exclusions |
.github/workflows/monitex-ai-tests.yml |
AI test CI workflow |
Review details
Suppressed comments (10)
docker-compose.yml:135
- Retraining writes to
/app/esp32_anomaly_model.joblib, but this service has no volume or external persistence for that path. Recreating the container during a deployment resets the newly trained per-series bundle to the checked-in artifact and loses the backup, so scheduled retraining does not survive normal service replacement. Persist the model path in durable storage before relying on the scheduler.
ANOMALY_MODEL_PATH: /app/esp32_anomaly_model.joblib
monitex-ai/model/anomaly_service.py:287
- The retrain listener catches failures by printing them, but
_retrain_and_reload()updateslast_retrain_okonly on success and the failure update below exists only in the scheduler path. A failed on-demand retrain therefore leaves/healthreporting the previous successful result (ornull). Update the health state in the shared failure path.
try:
await self._handle_retrain_event(message.body)
except Exception:
print("[Retrain Listener] Retrain run failed:")
traceback.print_exc()
monitex-ai/model/anomaly_service.py:224
- A drift warning does not affect this unconditional reload:
train_and_save_modelhas already saved the new bundle, and this line swaps it into the live runtime even whendrift_warningsis non-empty. A bad batch can therefore still replace the working model, contrary to the PR description and README; gate the save/reload or explicitly document that drift is logging-only.
self.runtime.reload(MODEL_PATH)
monitex-ai/model/fallback_detector.py:46
- The value that was just evaluated is always added to the fallback baseline, including when it is flagged anomalous. Repeated outliers can therefore inflate the running mean/std until the same outlier no longer exceeds the z-score threshold, causing this provisional detector to self-poison. Keep flagged values out of the baseline or use a robust/decaying baseline.
stats.update(value)
monitex-ai/model/model_runtime.py:61
- When a legacy single-tuple file is loaded,
_normalize_bundlestores it underDEFAULT_KEY, and this fallback applies it to every identified unknown(device, sensorType). With the default 24-hour scheduler interval, a deployment using the existing legacy artifact will score other devices/sensors with the old global model until the first retrain, defeating per-series isolation and bypassing the new fallback detector. Bootstrap with a retrain or only use the default when identifiers are absent.
key = _model_key(device_name, sensor_type)
if key is not None and key in models:
return models[key]
return models.get(DEFAULT_KEY)
monitex-ai/model/payloads.py:79
AnomalyModelRuntime.predict()returns-1for values belowlower_boundas well as aboveupper_bound, but this severity ladder only handles upper-side bounds. A lower-bound anomaly can therefore fall through toinfounless its model score happens to be negative enough, downgrading a hard-range violation in the notification. Include the lower bound in the bound check and apply at least warning consistently.
if max_valid is not None and value > max_valid:
severity = "critical"
elif threshold is not None and value > threshold:
severity = "warning"
elif score is not None and score <= SEVERITY_SCORE_CRITICAL:
monitex-ai/model/payloads.py:81
- The score-based severity branch is unreachable for the common out-of-IQR case: this
elif threshold ... value > thresholdassignswarningbeforescoreis examined. A strongly negative score for a value betweenupper_boundandmax_valid_readingis therefore always reported as warning, contradicting the documented score-based critical/warning/info grading. Evaluate the score before the learned-range warning (while keeping the hard-cap check first).
elif threshold is not None and value > threshold:
severity = "warning"
elif score is not None and score <= SEVERITY_SCORE_CRITICAL:
severity = "critical"
elif score is not None and score <= SEVERITY_SCORE_WARNING:
monitex-ai/train_model.py:227
- Drift is only added to
drift_warnings; the drifted model remains inmodels, is saved below, and is then hot-reloaded. A bad training batch therefore still replaces the working model, contrary to the stated behavior of logging drift instead of silently replacing it. Retain/reject the affected prior entry, or explicitly change the contract to warning-only.
drift = _check_drift(key, previous_profiles, profile)
if drift:
drift_warnings.append(drift)
print(f"[DRIFT WARNING] {drift}")
monitex-ai/train_model.py:150
- The minimum-sample check is performed before IQR filtering, and this guard only rejects an empty result. A series with 50 raw rows but only one or two retained rows can therefore fit and be saved as a trained model even though it has insufficient data for a meaningful IsolationForest. Apply
TRAIN_MIN_SAMPLEStofilteredas well before fitting.
if filtered.empty:
return None
monitex-ai/train_model.py:68
- This guard disables drift detection for any constant-valued series.
fit_seriesproduces a zero-width learned range when IQR is zero, so a retrain from (for example) a constant 30 to a constant 200 is accepted without a warning. Handle the zero-width case with an absolute/relative center comparison instead of returning early.
old_width = previous["upper_bound"] - previous["lower_bound"]
if old_width <= 0:
return None
- Files reviewed: 23/24 changed files
- Comments generated: 8
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| INFLUX_URL: http://influxdb:8086 | ||
| INFLUX_ORG: monitex-org | ||
| INFLUX_BUCKET: monitex | ||
| INFLUX_TOKEN: E_0cWP_5aLZ6KQKK7y6rPVXvoI1DRF6bf0xVjMrw2JwUCu6McyG4mQe6y629aHh6Q9jNnalYkqq_HTzxDWU7nA== |
| - "5020:5020" | ||
|
|
||
| monitex-ai: | ||
| monitex-ai: |
| total_tp += r["spike_hits"] + r["jump_hits"] | ||
| total_positives += r["spike_total"] + r["jump_total"] |
| pytest tests/ -v | ||
| ``` | ||
|
|
||
| 41 tests, all synthetic (no real InfluxDB/RabbitMQ needed) - covering |
| if within_cooldown and not escalated: | ||
| return False | ||
|
|
||
| self._last_notified[key] = (now, severity) |
| while True: | ||
| await asyncio.sleep(interval_seconds) |
| models: dict[str, tuple] = {} | ||
| trained: list[dict] = [] | ||
| skipped: list[dict] = [] | ||
| drift_warnings: list[str] = [] |
| shutil.copy2(output_path, backup_path) | ||
|
|
||
| X = scaler.transform(pd.DataFrame([[value]], columns=["_value"])) | ||
| joblib.dump(models, output_path) |
Rebuilds the anomaly detection service (monitex-ai/):
and sensor type. Now trains a separate model per (device, sensorType).
normal value range get caught, not just hard out-of-range values.
loudly instead of silently replacing a working model.
trained model yet.
anomalies on real InfluxDB data, since this is unsupervised training
with no ground-truth labels. See LIVE_EVALUATION_REPORT.md for current
numbers (98% precision, 92.5% F1 on live data).
No changes to Backend/ or Front-Web/ beyond docker-compose.yml
(added the env vars the AI service needs to reach InfluxDB and expose
its health endpoint).