feat: add strict latest signal linkage primitive#39
Conversation
Co-Authored-By: Codex <noreply@openai.com>
🤖 Codex PR Review🚫 Merge blocked: 1 serious issue(s) found in high-risk files ⚖️ Codex Review Arbitration🚫 block: The current finding remains valid. In 🚫 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: cddb661244
ℹ️ 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".
| if snapshot.get("as_of") != payload.get("as_of"): | ||
| raise SignalValidationError("signal and theme snapshot as_of must match") |
There was a problem hiding this comment.
When replaying or backfilling a saved signal, a theme snapshot with the same as_of but a generated_at later than the signal's generated_at still passes here because the linkage check only compares as_of. That admits a source artifact that did not exist when the signal was generated, which breaks the point-in-time guarantee and can introduce look-ahead into experiments; parse both timestamps and reject snapshots generated after the signal.
Useful? React with 👍 / 👎.
| if expected is None: | ||
| return |
There was a problem hiding this comment.
When source_hashes is present but keyed with a different spelling of the same source path, this branch returns successfully instead of treating the missing digest for the sole linked source as a validation failure. That silently disables the integrity check even though the artifact declared hashes, so a tampered snapshot can pass if the hash key drifts; reject a supplied hash map that does not cover the declared/resolved source.
Useful? React with 👍 / 👎.
| if snapshot.get("mode") != "theme_momentum_snapshot": | ||
| raise SignalValidationError("source must declare theme_momentum_snapshot") |
There was a problem hiding this comment.
When a source JSON keeps mode as theme_momentum_snapshot but sets another artifact_type, it passes here because the strict linkage only checks mode; the existing snapshot validator only checks that artifact_type is present. That lets a non-medium-horizon artifact be linked to a latest signal and pollute replay context; compare artifact_type to medium_horizon_theme_context before accepting the source.
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: 9703b54b17
ℹ️ 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".
| try: | ||
| parsed = dt.datetime.fromisoformat(normalized) |
There was a problem hiding this comment.
When either the signal or snapshot supplies generated_at as a bare date such as 2026-06-27, datetime.fromisoformat() accepts it and this strict linkage check treats it as midnight UTC. That lets artifacts without an actual generation time pass the point-in-time ordering check, so same-day replay can be accepted even though the metadata cannot prove the snapshot existed when the signal was generated; require a time component before parsing.
Useful? React with 👍 / 👎.
| if base is None: | ||
| fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW) |
There was a problem hiding this comment.
When callers omit signal_base_dir for an absolute evidence.sources entry, path has already been produced by Path.resolve(), so a declared source that is a symlink is dereferenced before this O_NOFOLLOW open. In that absolute-path mode the validator accepts and hashes the symlink target instead of rejecting the declared source, which can link a latest signal to a swapped or future snapshot when hashes are omitted; keep the declared path for the no-follow open.
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: 12849f7df0
ℹ️ 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".
| _validate_source_hash(evidence, payload, declared_source, source_path, raw) | ||
| snapshot = _decode_declaration(raw) | ||
| try: | ||
| validate_theme_momentum_snapshot(snapshot) |
There was a problem hiding this comment.
When the linked v2 snapshot sets policy.execution_allowed or policy.portfolio_allocation_allowed to true, this still accepts it because validate_theme_momentum_snapshot() only checks that policy exists and the strict linkage adds no policy validation, even though the generated theme snapshot contract emits those flags as false for research-only context. That lets a latest signal validate against a source artifact that advertises execution/allocation permission, weakening the strict linkage guarantee for downstream consumers that trust validated artifacts.
Useful? React with 👍 / 👎.
| if _parse_datetime(snapshot["generated_at"], "theme snapshot generated_at") > _parse_datetime( | ||
| payload["generated_at"], "signal generated_at" | ||
| ): | ||
| raise SignalValidationError("theme snapshot generated_at must not be later than signal generated_at") |
There was a problem hiding this comment.
When the snapshot claims as_of data from a future date relative to its own generated_at (for example as_of=2026-06-26 with snapshot generated_at=2026-06-25T00:00:00Z), this still passes as long as the signal uses the same as_of and was generated later. That permits a point-in-time replay source to claim data that could not have been available when the snapshot was generated, so strict linkage should compare the parsed as_of date against the generated timestamps, not only snapshot-vs-signal ordering.
Useful? React with 👍 / 👎.
| if _parse_datetime(snapshot["generated_at"], "theme snapshot generated_at") > _parse_datetime( | ||
| payload["generated_at"], "signal generated_at" | ||
| ): | ||
| raise SignalValidationError("theme snapshot generated_at must not be later than signal generated_at") |
There was a problem hiding this comment.
When a v2 snapshot has expires_at after as_of but before the signal's generated_at (for example a 2026-06-26 snapshot expiring 2026-06-28 linked to a signal generated 2026-07-27), this still validates because strict linkage only checks matching as_of and generation ordering. That allows a latest signal to be built from stale medium-horizon context even though the snapshot declares it was no longer valid at signal generation time.
Useful? React with 👍 / 👎.
|
Final user-authorized bounded remediation is exhausted and the required review remains blocking. Per the rollout contract, this PR is closed without merge; no further code changes or reslices will be attempted automatically. |
1 participant
Summary
validate_latest_signalas a strict v2 linkage primitiveas_ofValidation
python3 -m pytest -q(51 passed)git diff --checkNo merge requested.