fix(polymarket): bind legacy health freshness to writes - #419
Conversation
|
Warning Review limit reached
Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe legacy shadow-gate path now captures health-file write times, validates strict timestamp and ordering constraints, records the evidence in ChangesLegacy health freshness
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ShadowGate as Shadow gate
participant HealthFile as Legacy health file
participant Policy as jq policy
participant GateJSON as gate.json
ShadowGate->>HealthFile: Capture snapshot and filesystem write time
ShadowGate->>Policy: Validate freshness and timestamp ordering
Policy-->>ShadowGate: Accept validated observation
ShadowGate->>GateJSON: Record baseline write-time evidence
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: b8994eb9bf
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
deployment/aliyun/test-polymarket-raw-ops-control-plane.sh (1)
1939-1949: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDisambiguate line-number greps for robustness.
snapshot_line,completion_snapshot_line,gate_start_line, andshadow_start_linedon't pin down a single match the waydaemon_reload_linedoes withtail -1. If any of these literal markers were ever duplicated in$GATEin the future,cut -d: -f1would yield a multi-line value and the subsequent((...))comparison would likely error out with an unclear message rather than a clean assertion failure.♻️ Proposed hardening
-snapshot_line=$(grep -nF 'baseline_health_observation=$(fresh_legacy_health_observation' \ - "$GATE" | cut -d: -f1) +snapshot_line=$(grep -nF 'baseline_health_observation=$(fresh_legacy_health_observation' \ + "$GATE" | head -1 | cut -d: -f1)(similarly for
completion_snapshot_line,gate_start_line,shadow_start_line)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deployment/aliyun/test-polymarket-raw-ops-control-plane.sh` around lines 1939 - 1949, Update the line-number lookups for snapshot_line, completion_snapshot_line, gate_start_line, and shadow_start_line to select a single matching line, consistent with daemon_reload_line using tail -1. Keep the existing ordering assertion unchanged so duplicate markers produce a deterministic comparison and clean failure.deployment/aliyun/polymarket-shadow-gate-policy.jq (1)
44-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated
utc_iso8601_unixcomputation forstarted_at/completed_at.
.started_at | utc_iso8601_unixis computed independently 4 times (lines 44, 65, 66-67, 78) and.completed_at | utc_iso8601_unixtwice (48, 80). Binding these once (e.g.(.started_at | utc_iso8601_unix) as $started_at_unix) would avoid duplicated logic and reduce the risk of one occurrence silently drifting from the others in future edits to this security-relevant policy.♻️ Proposed refactor sketch
+and ((.started_at | utc_iso8601_unix) as $started_at_unix | $started_at_unix | type == "number") +and ((.completed_at | utc_iso8601_unix) as $completed_at_unix | $completed_at_unix | type == "number") -and (.started_at | utc_iso8601_unix | type == "number") and (.parity_window_started_at_unix | positive_integer) and (.parity_window_ended_at_unix | positive_integer) and (.parity_window_ended_at_unix - .parity_window_started_at_unix >= 601) -and (.completed_at | utc_iso8601_unix | type == "number")(actual binding would need to be threaded through the surrounding
andchain, e.g. via a top-level. as $root | ($root.started_at|utc_iso8601_unix) as $started_at_unix | ($root.completed_at|utc_iso8601_unix) as $completed_at_unix | ...)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deployment/aliyun/polymarket-shadow-gate-policy.jq` around lines 44 - 80, Refactor the policy around its top-level predicate to bind the converted started_at and completed_at values once, using scoped variables such as $started_at_unix and $completed_at_unix, while preserving the existing root object context. Replace every repeated utc_iso8601_unix use for these fields throughout the baseline validation branch and surrounding checks with the corresponding bindings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deployment/aliyun/polymarket-raw-ops-shadow-gate.sh`:
- Around line 396-422: Document the freshness contract introduced around
fresh_legacy_health_observation: legacy health observations use the file rewrite
mtime, bounded by MAX_HEALTH_SILENCE_SECONDS, as the recency signal, while
updated_at and last_success_at only need to be non-empty and not future-dated.
Add this clarification where the gate freshness contract is defined, without
changing the existing validation logic.
In `@deployment/aliyun/polymarket-shadow-gate-policy.jq`:
- Around line 4-15: Update the deployment policy around utc_iso8601_unix to
explicitly require jq 1.7 or newer, documenting and enforcing that version
requirement wherever this policy is executed. Keep the existing UTC timestamp
validation and conversion behavior unchanged rather than relying on pre-1.7
timezone-dependent date builtins.
---
Nitpick comments:
In `@deployment/aliyun/polymarket-shadow-gate-policy.jq`:
- Around line 44-80: Refactor the policy around its top-level predicate to bind
the converted started_at and completed_at values once, using scoped variables
such as $started_at_unix and $completed_at_unix, while preserving the existing
root object context. Replace every repeated utc_iso8601_unix use for these
fields throughout the baseline validation branch and surrounding checks with the
corresponding bindings.
In `@deployment/aliyun/test-polymarket-raw-ops-control-plane.sh`:
- Around line 1939-1949: Update the line-number lookups for snapshot_line,
completion_snapshot_line, gate_start_line, and shadow_start_line to select a
single matching line, consistent with daemon_reload_line using tail -1. Keep the
existing ordering assertion unchanged so duplicate markers produce a
deterministic comparison and clean failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e1f30fff-309b-4c10-bfd1-f9698f035973
📒 Files selected for processing (3)
deployment/aliyun/polymarket-raw-ops-shadow-gate.shdeployment/aliyun/polymarket-shadow-gate-policy.jqdeployment/aliyun/test-polymarket-raw-ops-control-plane.sh
1 participant
Change contract
Bind legacy Python Gate freshness to a stable atomic health-file write while preserving exact payload timestamps and strict immutable evidence.
Out of scope
Collector/snapshot/research logic, #235, parity semantics, observation duration, PID/restart identity, OSS readback behavior, and production cutover.
Dependency / merge order
Base exact-main
b199e1c87cf9a547c3d0ca605810ee2b26c2437c(PR #417). Merge this before rebuilding the next exact-main release artifact.Focused validation
bash deployment/aliyun/test-polymarket-raw-ops-control-plane.shgit diff --checkRollout / rollback impact
Rebuild a new digest-addressed exact-main artifact and run a fresh 3600+601 second Gate. Revert this PR to restore the prior payload-age freshness rule; production remains on Python until the Gate and cutover both pass.
Closes #418
Summary by CodeRabbit
New Features
Bug Fixes
Tests