fix(pm-gate): bound real-market preflight scan for tick-level segments - #730
Conversation
Issue #586: tick-level segments (7.7-18GB) make the preflight's full-file jq passes exceed the 1200s budget, SIGKILLing the worker. Bound the scan so cost does not scale with tick volume: - source_quote_records: capped head-window count (PREFLIGHT_SCAN_WINDOW_RECORDS + PREFLIGHT_QUOTE_COUNT_CAP), reported as a lower-bound count. - source_recorded_hours: head+tail window verification instead of full-file scan, still requiring a single recorded hour. - evidence now carries source_scan_bounded:true to keep the sampled semantics honest; the shadow-gate policy requires it. Verified: bash -n; focused scan-logic test (quote cap, single-hour window, cross-hour capture). Full test suite runs on CI (GNU date); macOS-local runs stop at an unrelated pre-existing recovery-probe date -d in the GATE script.
… extraction Issue #586 acceptance: a segment much larger than the scan window must not make preflight exceed its budget. Adds a large-case counterexample verifying the bounded scan (capped quote count, head+tail hour check) completes quickly and the evidence carries source_scan_bounded:true. Also extends the preflight verifier's sed extraction to include the new PREFLIGHT_SCAN_WINDOW_RECORDS / PREFLIGHT_QUOTE_COUNT_CAP constants so the extracted function resolves them.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 44 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 (1)
📝 WalkthroughWalkthroughThe real-market preflight bounds quote and hour scans, records bounded-scan evidence, requires that evidence in policy validation, and updates control-plane regression coverage. ChangesReal-market preflight
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 935-939: Update the quote-count pipeline in the preflight flow
around run_before_deadline to avoid piping jq into head, which can trigger
SIGPIPE under pipefail. Count at most PREFLIGHT_QUOTE_COUNT_CAP matching records
with a bounded consumer that continues reading the upstream input fully, while
preserving the existing trimmed numeric assignment and failure propagation.
🪄 Autofix
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: efa08a23-2456-492d-a154-5a1d29844755
📒 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
…IGPIPE - quote count: bounded by input-side head window (PREFLIGHT_SCAN_WINDOW_RECORDS) instead of output-side cap, eliminating the SIGPIPE risk from truncating jq output under pipefail. Removed the now-unused PREFLIGHT_QUOTE_COUNT_CAP. - counterexample: isolates the scan from the (legitimately size-scaling) upload path; a 2x-window segment must scan in bounded time with window-bounded quote count and single-hour window check. - verified locally: 35MB / 400k-record segment scans in ~1s.
Reverts the temporary removal; the counterexample is the issue #586 acceptance test. The Rust Workspace snapshot failure it was being compared against is a flaky pre-existing cutover issue (same-head runs both pass and fail).
|
CI note: the only failing check is |
1e23d2d to
e533a6a
Compare
|
Blocker update: the Rust Workspace failure at the snapshot test is a cross-impact — the preflight-scan change in this PR triggers the (code-independent) snapshot test to fail on CI, while main passes. Counterexample excluded as cause. Tracked separately in #733 (investigation) and #731 (snapshot flake, reopened). This PR's own bounded-scan logic is verified correct (counterexample passes, |
|
Root cause clarified: the Rust Workspace snapshot failure is a PRE-EXISTING test defect, confirmed by running the test on a clean main baseline locally — it fails identically. #586 is not the cause; its own logic is verified. Main CI never ran this test (Rust Workspace job is scope-skipped). This PR simply exposed it. Tracked in #731/#733. |
…ded fixture Root-cause of the snapshot flake: die() uses exit 1, which under the test harness's set -euo pipefail + subshell-function context could terminate the whole script when copied-binary-drift's die fired inside the if-condition. Running snapshot_legacy in a nested subshell confines that exit so the caller's if observes the status normally (verified locally: both absent-control and copied-binary-drift now behave correctly and the full test passes). Also add source_scan_bounded:true to the preflight gate.json fixture, required by the shadow-gate policy change (policy now demands it). Without it the policy check fails at the end of the suite. Verified locally with GNU coreutils wrappers: full test exits 0.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/test-polymarket-raw-ops-control-plane.sh`:
- Around line 1232-1245: Update the large-segment validation in the test to
assert the exact expected head-window quote count, rather than allowing any
value up to PREFLIGHT_SCAN_WINDOW_RECORDS. Add a deterministic read-limit check
or enlarge the fixture so a full-file scan cannot satisfy the test, while
preserving the existing bounded-time assertion.
🪄 Autofix
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: a1129915-f9f7-4f5f-af19-99771bd064f9
📒 Files selected for processing (1)
deployment/aliyun/test-polymarket-raw-ops-control-plane.sh
Address CodeRabbit: the counterexample's quote-count assertion was too loose (<= window). The fixture alternates quote/event records, so the head window has exactly WINDOW/2 quotes; assert that exactly. A full-file scan of the 2x fixture would return WINDOW quotes and fail — proving the scan is bounded.
1 participant
Change contract
Issue #586: tick-level segments (7.7-18GB) make the real-market preflight's
full-file jq passes exceed the 1200s budget, SIGKILLing the worker. Bound the
scan so cost does not scale with tick volume.
source_quote_records: capped head-window count (PREFLIGHT_SCAN_WINDOW_RECORDSPREFLIGHT_QUOTE_COUNT_CAP), reported as a lower-bound count.source_recorded_hours: head+tail window verification instead of full-filescan, still requiring a single recorded hour.
source_scan_bounded: true; the shadow-gate policyrequires it, keeping the sampled semantics honest.
completes preflight quickly and the evidence reflects the bounded scan.
Issue relationship
Closes #586
Out of scope
source_quote_recordsis now a bounded lower-bound count, not the exacttotal; the policy only requires
positive_integerso this is compatible.cost (head+tail window); this is the documented trade-off of the bounded-scan
approach.
Dependencies and merge order
None
Focused validation
bash -non both scripts: OKall correct
unrelated
date -din the GATE recovery probe (verified present on main)Rollout and rollback
None in production (deployment-script change). Rollback is reverting this PR.
Scope exception
None (3 files).
Summary by CodeRabbit
Bug Fixes
Tests