feat: add SOXL/TQQQ clean-cutover snapshots - #199
Conversation
Co-Authored-By: Codex <noreply@openai.com>
🤖 Codex PR Review🚫 Merge blocked: 1 serious issue(s) found in high-risk files 🚫 Blocking IssuesThese issues must be fixed before this PR can be merged: 1. 🟠 [HIGH] Logic in
|
![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: 7d303231aa
ℹ️ 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".
| "properties": { | ||
| "schema": {"const": "soxl_tqqq_clean_cutover_snapshot.v1"}, | ||
| "pair_id": {"enum": ["QQQ_TQQQ", "SOXX_SOXL"]}, | ||
| "symbols": {"type": "array", "items": {"type": "string"}, "minItems": 2, "maxItems": 2, "uniqueItems": true}, |
There was a problem hiding this comment.
When a downstream consumer validates a manifest only against this new schema, pair_id: "QQQ_TQQQ" can be paired with any two unique strings, including SOXX/SOXL or unrelated symbols, even though the Python verifier requires the exact PAIR_SYMBOLS mapping. This allows a schema-valid manifest to misidentify its dataset; add pair-specific conditional constraints so each pair_id requires its canonical ordered symbol list.
Useful? React with 👍 / 👎.
| or manifest["plugin"] != PLUGIN | ||
| or manifest["offline_fixture"] is not True | ||
| or manifest["compatibility"] != _compatibility() | ||
| or manifest["size"] != SIZE |
There was a problem hiding this comment.
When verifying an externally produced manifest containing "size": false, this comparison accepts it because Python considers False == 0; the same equality behavior also lets one-session per_symbol_counts use booleans and lets the publication's complete field be 1. The verifier then reports a schema-invalid artifact as valid despite promising strict readback, so these contract fields need exact type checks rather than value equality alone.
Useful? React with 👍 / 👎.
| _invalid("session must be a canonical ISO date") | ||
| if type(symbol) is not str or symbol not in symbols: | ||
| _invalid("each session must contain exactly the pair symbols") | ||
| if type(adjusted_close) not in (int, float) or isinstance(adjusted_close, bool) or not math.isfinite(adjusted_close) or adjusted_close <= 0: |
There was a problem hiding this comment.
If an offline fixture contains an arbitrarily large integer such as 10**1000 for adjusted_close, math.isfinite(adjusted_close) raises OverflowError while converting it to a float. This affects both materialization and authenticated readback and bypasses SnapshotValidationError/INVALID_EVIDENCE, so callers that classify invalid fixtures by that documented exception instead crash; catch the overflow or validate integers without converting them to float.
Useful? React with 👍 / 👎.
| if not _is_iso_date(session): | ||
| _invalid("session must be a canonical ISO date") |
There was a problem hiding this comment.
When rows contain a syntactically valid non-trading date such as Saturday 2026-01-03, both materialization and verification accept it as a completed session even when the caller supplies the expected calendar digest, because the date is checked only for ISO syntax and the digest is never related to the rows. This permits impossible market sessions into the price snapshot and can corrupt downstream returns; validate every session against the calendar represented by calendar_sha256 (or at minimum reject weekends).
Useful? React with 👍 / 👎.
Co-Authored-By: Codex <noreply@openai.com>
|
Closing unmerged per the bounded Phase 1 authority: natural review remained FAILURE after the sole consolidated remediation (budget 1/1). Evidence is preserved; no further remediation, rerun, or merge is authorized. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be4995f537
ℹ️ 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".
| (temporary / "payload.json").write_bytes(payload_bytes) | ||
| (temporary / "manifest.json").write_bytes(manifest_bytes) | ||
| (temporary / "publication.json").write_bytes(_canonical_json(publication)) |
There was a problem hiding this comment.
When valid inputs serialize beyond MAX_MEMBER_BYTES—for example, a source_identity longer than 1 MiB or a sufficiently long row history—the materializer writes the files and returns success, but verify_clean_cutover_snapshot always rejects the resulting snapshot as oversized. Check each serialized member's size before renaming the temporary directory so every successfully materialized snapshot can pass readback.
Useful? React with 👍 / 👎.
| except (UnicodeDecodeError, ValueError, json.JSONDecodeError) as exc: | ||
| raise SnapshotValidationError(f"{INVALID_EVIDENCE}: invalid {name}") from exc |
There was a problem hiding this comment.
When an externally supplied member contains deeply nested JSON, json.loads can raise RecursionError while the member is still well below the 1 MiB limit; a roughly 200 KB payload with 100,000 nested arrays reproduces this. Because the exception is not caught here, authenticated readback escapes without the documented SnapshotValidationError/INVALID_EVIDENCE classification instead of treating the malformed fixture as invalid.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,100 @@ | |||
| { | |||
| "$schema": "https://json-schema.org/draft/2020-12/schema", | |||
| "$vocabulary": {"https://json-schema.org/draft/2020-12/vocab/format-assertion": true}, | |||
There was a problem hiding this comment.
When a downstream consumer uses an ordinary Draft 2020-12 validator without separately installing and enabling a format checker, format remains annotation-only because $schema selects the standard meta-schema; declaring $vocabulary in this instance schema does not activate assertion behavior. Dates such as 2026-02-30 therefore satisfy the regex and pass schema validation, while the test masks this by explicitly supplying FORMAT_CHECKER; select a meta-schema that declares the assertion vocabulary or otherwise make the validator requirement enforceable.
Useful? React with 👍 / 👎.
| "first_available_session": {"type": "string", "format": "date", "pattern": "^[0-9]{4}-[0-9]{2}-[0-9]{2}$"}, | ||
| "last_available_session": {"type": "string", "format": "date", "pattern": "^[0-9]{4}-[0-9]{2}-[0-9]{2}$"}, | ||
| "completed_sessions": {"type": "array", "items": {"type": "string", "format": "date", "pattern": "^[0-9]{4}-[0-9]{2}-[0-9]{2}$"}, "minItems": 1, "uniqueItems": true}, | ||
| "row_count": {"type": "integer", "minimum": 2}, |
There was a problem hiding this comment.
For either supported pair, every completed session contains exactly two symbols, but this schema accepts an odd row_count such as 3 alongside one completed session and one count per symbol. A schema-only consumer can therefore accept impossible coverage that the Python verifier rejects; require at least multipleOf: 2 so the static pair cardinality is represented in the schema.
Useful? React with 👍 / 👎.
| if type(value) is not str or not value.endswith("Z"): | ||
| return False | ||
| try: | ||
| return datetime.fromisoformat(value[:-1] + "+00:00").isoformat().replace("+00:00", "Z") == value |
There was a problem hiding this comment.
When materialized_at contains canonical nonzero six-digit fractional seconds, such as 2026-07-26T00:00:00.123456Z, this equality succeeds, so both materialization and authenticated readback accept the value. The bundled manifest schema's timestamp pattern permits seconds only, however, causing a successfully produced snapshot to fail schema validation; either reject fractional seconds here or allow the same canonical form in the schema.
Useful? React with 👍 / 👎.
1 participant
Summary
Validation
python3 -m pytest -q tests/test_soxl_tqqq_clean_cutover_snapshot.pypython3 -m ruff check src/us_equity_snapshot_pipelines/soxl_tqqq_clean_cutover_snapshot.py tests/test_soxl_tqqq_clean_cutover_snapshot.pypython3 -m compileall -q src/us_equity_snapshot_pipelines/soxl_tqqq_clean_cutover_snapshot.pyNo provider, real-data, replay, optimization, plugin L2/L3, or live behavior is included; size remains zero.