Conversation
Co-Authored-By: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63d324f518
ℹ️ 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".
| def _read_snapshot_members(output: Path) -> dict[str, bytes]: | ||
| if not hasattr(os, "O_NOFOLLOW") or not hasattr(os, "O_DIRECTORY") or os.open not in os.supports_dir_fd: | ||
| _invalid("descriptor-anchored snapshot readback is unavailable") |
There was a problem hiding this comment.
Preserve verification on supported non-POSIX platforms
On Windows, O_NOFOLLOW, O_DIRECTORY, and directory-relative os.open are unavailable, so this guard rejects every snapshot before reading it; because materialize_tqqq_r1_snapshot invokes the verifier internally, publication also becomes unusable there. The package declares no OS restriction and the previous pathlib-based implementation worked cross-platform, while the only CI job runs on Ubuntu, so either retain a safe fallback for platforms without these primitives or explicitly constrain the package/API to supported operating systems.
Useful? React with 👍 / 👎.
| if integer is not None and str(integer) == value: | ||
| if integer <= 0: | ||
| _invalid("adjusted_close must be positive finite") | ||
| return integer |
There was a problem hiding this comment.
Reject integers that overflow the downstream finiteness check
When a tampered but otherwise parseable snapshot contains a canonical positive integer of roughly 310–4300 digits, this returns the unbounded Python integer and the later map(math.isfinite) raises OverflowError while converting it to a float. An attacker can reach this path even with the trusted manifest unchanged by updating prices.csv and sha256sums.json, because the manifest's prices_sha256 is compared only after CSV normalization; callers expecting invalid snapshots to raise SnapshotValidationError therefore receive an unexpected crash instead.
Useful? React with 👍 / 👎.
Co-Authored-By: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80b4cbb40a
ℹ️ 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".
| rename.argtypes = (ctypes.c_char_p, ctypes.c_char_p, ctypes.c_uint) | ||
| rename.restype = ctypes.c_int | ||
| result = rename(source_bytes, destination_bytes, rename_excl) | ||
| elif sys.platform.startswith("linux") and hasattr(libc, "renameat2"): |
There was a problem hiding this comment.
Add a fallback when libc lacks renameat2
On Linux systems with the required descriptor features but without a libc-exported renameat2 wrapper—such as glibc versions before 2.28—_require_supported_filesystem_runtime() accepts the environment, but this condition falls through to the unsupported-publication error, so every materialization fails despite the newly documented Linux support. Invoke the syscall through a fallback or include this capability in the runtime gate and narrow the stated contract.
Useful? React with 👍 / 👎.
Summary
adjusted_closeinputs before numeric coercion or artifact generationBase and scope
38e4f1e523458784a4e1d05e2d093bcabdbc455963d324f518dd0db6ce6933147dd69e2210806908src/us_equity_snapshot_pipelines/tqqq_r1_snapshot.pytests/test_tqqq_r1_snapshot.pyTests-first evidence
1;3 failed, 1 passed; output SHA-2568c9eefec29b34043d4a5c0ea9ddb4cfce5048b9b3e41f81878ee99522348b27a1;1 failed; output SHA-2564ed41b38c8ee67a501fd34d40c174833c50338483b6d0d49ddc3d4ce59d497650;4 passed; output SHA-256dbd214fdd81dacefd23d75edac6a1fdf45be9a0af8d3f9f84408f17dd10ffe0bpython3 -m ruff check .: exit0; output SHA-25682b3e6a6c090a57601d22943bd23fca9218d1031dbe5a7b754092f9a156b4f18PYTHONPATH=src python3 -m pytest -q: exit0;710 passed, 4 skipped; output SHA-256a3bed4180988309e5cd7002938b22d7cbc29ffadb16070ea65967fff1afc2afcgit diff --check: exit0Non-goals / safety boundary
No provider or price-data acquisition, replay, optimization, plugin execution, broker/account/order/capital/position/leverage action, cloud mutation, deployment, or live action.
size=0remains enforced. This PR must not be auto-merged.