fix: sync trusted lifecycle artifacts for quant monitor - #105
Conversation
Co-Authored-By: Codex <noreply@openai.com>
|
Reopening once to retrigger required checks; the initial PR event produced no workflow runs. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f18d2e62ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| except LifecycleArtifactError as exc: | ||
| statuses[domain] = { | ||
| "status": "error", | ||
| "code": exc.code, | ||
| "error_type": type(exc).__name__, | ||
| } |
There was a problem hiding this comment.
When a previously activated domain later fails synchronization because its artifact expired, GitHub is unavailable, or a replacement is invalid, this handler only records the error and leaves its consumer symlinks pointing at the prior data. daily_briefing.sh does not run this synchronizer, and daily_briefing_builder.py:47-98 neither reads status.json nor refreshes snapshots, so it can publish and dispatch a data_status: ready report from data that the new trust policy has rejected. Invalidate failed-domain links or gate the daily briefing on the artifact status.
AGENTS.md reference: ops/quant-monitor/AGENTS.md:L28-L32
Useful? React with 👍 / 👎.
| artifact_id = int(artifact["id"]) | ||
| version_dir = artifacts_root / "versions" / config["domain"] / str(artifact_id) |
There was a problem hiding this comment.
Whenever a producer publishes a new artifact ID, the extracted bundle is permanently stored under versions/<domain>/<artifact_id>, while cleanup only removes temporary staging paths. Because the monitor runs indefinitely across four domains and each retained archive may contain up to 256 MiB of extracted data, these immutable versions accumulate without bound and can eventually exhaust the VPS disk; retain only the active version plus a small rollback window.
Useful? React with 👍 / 👎.
| if previous_date and date_value <= previous_date: | ||
| raise LifecycleArtifactError("return matrix dates must be strictly increasing") | ||
| previous_date = date_value |
There was a problem hiding this comment.
datetime.strptime accepts non-zero-padded dates, but the ordering check compares their original strings. Consequently, a matrix containing 2026-10-01 followed by the earlier 2026-7-10 passes validation, while the chronologically increasing reverse order is rejected. Compare the parsed date objects or require a canonical zero-padded representation so out-of-order returns cannot reach drift calculations.
Useful? React with 👍 / 👎.
| for key in ("sharpe_ratio", "max_drawdown", "cagr", "volatility"): | ||
| try: | ||
| value = float(payload[key]) |
There was a problem hiding this comment.
For a malformed artifact containing a JSON boolean in sharpe_ratio, max_drawdown, cagr, or volatility, float(payload[key]) converts it to 1.0 or 0.0, and the finite-value check then accepts it as a valid metric. This can activate nonsensical backtest results and corrupt health or drift calculations; explicitly reject bool values as is already done for the integer fields.
Useful? React with 👍 / 👎.
1 participant
Summary
mainschedule/workflow-dispatch lifecycle artifacts with a successfulpreflight_backtestsjobValidation
python3 -m pytest ops/quant-monitor/tests -q(41 passed)git diff --check