feat(polymarket): normalize typed staging evidence - #96
Conversation
|
@codex review |
📝 WalkthroughWalkthroughAdds a public Polymarket research normalizer that validates evidence, constructs canonical contract/trade/settlement/orderbook/reference rows, emits deterministic NDJSON, reports hashes and surface counts, and enforces completeness requirements. ChangesPolymarket Evidence Normalization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant normalize_polymarket_evidence
participant with_selected_research_contracts
participant normalize_raw
participant encode_rows
Caller->>normalize_polymarket_evidence: provide PolymarketEvidenceConfig
normalize_polymarket_evidence->>with_selected_research_contracts: select research contracts
with_selected_research_contracts->>normalize_raw: normalize selected evidence
normalize_raw->>encode_rows: sort and encode canonical rows
encode_rows-->>normalize_raw: NDJSON, digest, byte count, surface counts
normalize_raw-->>Caller: report and NDJSON
🚥 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 |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7413e0efb
ℹ️ 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".
| pending.line | ||
| ); | ||
| } | ||
| let trade_ts = required(&pending.update, "trade_ts")?.to_owned(); |
There was a problem hiding this comment.
When the reference hour contains polymarket_trade rows for the selected market whose trade_ts is before event_start or after event_end, reference_records has already selected them by market_id and this code emits them without a window check. The existing parity path (polymarket_parity::trade_map) explicitly keeps only trades whose trade_ts_unix is within the event window, so this normalizer can add off-window/post-expiry trades to a five-minute evidence bundle and skew downstream research.
Useful? React with 👍 / 👎.
| let winner = required(&pending.update, "winning_token_id")?.to_owned(); | ||
| let fingerprint = settlement_fingerprint(&pending)?; | ||
| if record_settlement_fingerprint(settled, market_id, &fingerprint, pending.line)? { | ||
| let prices = json_strings(market.get("outcomePrices"), "outcomePrices")?; |
There was a problem hiding this comment.
When a valid market_settlement arrives with outcomePrices as a JSON array instead of a JSON-encoded string, validate_market_settlement accepts it via the shared parse_json_array path, but this normalizer rejects it here through json_strings. The ops/control-plane fixtures already exercise array-shaped settlement payloads, so normalization can fail after the staged triplet has been accepted as canonical.
Useful? React with 👍 / 👎.
| start: DateTime<Utc>, | ||
| end: DateTime<Utc>, | ||
| ) -> Result<NormalizedPolymarketEvidence> { | ||
| let (trades, settlements) = reference_records(reference_path, contracts)?; |
There was a problem hiding this comment.
When the selected market is the last five-minute slot before an hour boundary, its metadata/trade records can be in the current reference-hour artifact while the official settlement and late trade poll are recorded after event_end in the next one. Because normalize_raw consumes only a single reference_path, picking either hour can drop part of the validated reference evidence or fail completeness for an otherwise valid boundary market.
Useful? React with 👍 / 👎.
| if required(&pending.update, "condition_id")? != metadata.condition_id | ||
| || tokens != metadata.tokens | ||
| || outcomes != metadata.outcomes |
There was a problem hiding this comment.
When a market_settlement row reuses the selected market_id/condition/tokens but its raw market carries a different symbol or five-minute time window, validate_market_settlement still accepts the update and this check only compares condition/tokens/outcomes before stamping the selected contract's event times onto the row. That lets contradictory settlement evidence satisfy completeness for the wrong governed market instead of failing closed.
Useful? React with 👍 / 👎.
Constraint: Downstream importers need content-addressed collector evidence without inheriting research snapshot authority semantics before PR #96 merges Rejected: reuse research artifact naming | it leaked snapshot-oriented identifiers and banned settlement surface semantics Directive: Keep content_sha256/content_bytes distinct from snapshot contracts and preserve the no-clobber triplet publisher as one rollout unit with the CLI Confidence: high Scope-risk: narrow Tested: cargo fmt --all; cargo test -p hft-collector manifest_semantics_disclose_content_digest_and_recording_limits --locked; cargo test -p hft-collector publish_polymarket_evidence_cli_requires_a_bounded_window_and_output_root --locked; cargo test -p hft-collector --locked; cargo clippy -p hft-collector --lib --locked -- -D warnings; git diff --check; git diff --cached --check
Constraint: Downstream importers need content-addressed collector evidence without inheriting research snapshot authority semantics before PR #96 merges Rejected: reuse research artifact naming | it leaked snapshot-oriented identifiers and banned settlement surface semantics Directive: Keep content_sha256/content_bytes distinct from snapshot contracts and preserve the no-clobber triplet publisher as one rollout unit with the CLI Confidence: high Scope-risk: narrow Tested: cargo fmt --all; cargo test -p hft-collector manifest_semantics_disclose_content_digest_and_recording_limits --locked; cargo test -p hft-collector publish_polymarket_evidence_cli_requires_a_bounded_window_and_output_root --locked; cargo test -p hft-collector --locked; cargo clippy -p hft-collector --lib --locked -- -D warnings; git diff --check; git diff --cached --check
Constraint: Downstream importers need content-addressed collector evidence without inheriting research snapshot authority semantics before PR #96 merges Rejected: reuse research artifact naming | it leaked snapshot-oriented identifiers and banned settlement surface semantics Directive: Keep content_sha256/content_bytes distinct from snapshot contracts and preserve the no-clobber triplet publisher as one rollout unit with the CLI Confidence: high Scope-risk: narrow Tested: cargo fmt --all; cargo test -p hft-collector manifest_semantics_disclose_content_digest_and_recording_limits --locked; cargo test -p hft-collector publish_polymarket_evidence_cli_requires_a_bounded_window_and_output_root --locked; cargo test -p hft-collector --locked; cargo clippy -p hft-collector --lib --locked -- -D warnings; git diff --check; git diff --cached --check
Constraint: Downstream importers need content-addressed collector evidence without inheriting research snapshot authority semantics before PR #96 merges Rejected: reuse research artifact naming | it leaked snapshot-oriented identifiers and banned settlement surface semantics Directive: Keep content_sha256/content_bytes distinct from snapshot contracts and preserve the no-clobber triplet publisher as one rollout unit with the CLI Confidence: high Scope-risk: narrow Tested: cargo fmt --all; cargo test -p hft-collector manifest_semantics_disclose_content_digest_and_recording_limits --locked; cargo test -p hft-collector publish_polymarket_evidence_cli_requires_a_bounded_window_and_output_root --locked; cargo test -p hft-collector --locked; cargo clippy -p hft-collector --lib --locked -- -D warnings; git diff --check; git diff --cached --check Co-authored-by: proerror <proerror@mandonothing-Bravo-1-1.local>
Change contract
Normalize already validated and selected governed BTC/SOL five-minute Polymarket records into deterministic, point-in-time, typed collector evidence for five source surfaces: market contract, both token orderbooks, Chainlink reference, Polymarket trades, and official closed-market settlement evidence.
Out of scope
Evaluator-authoritative binary labels,
pm_token_settlements,snapshot_contract_hash, prediction-market snapshot import, immutable artifact publication, CLI wiring, evaluator/training changes, promotion policy, deployment, risk, and live execution.Dependency or merge order
Depends on merged PRs #89, #93, and #94. This PR is based directly on
main; it is not stacked on an open PR. A laterrust_hft/prediction-marketschange must convert this neutral evidence through its canonical label and snapshot-v2 contracts.Focused validation
cargo +1.91 fmt -p hft-collector -- --checkcargo +1.91 test --locked -p hft-collector --lib --features collector-binance polymarket_research_normalize::tests— 6 passedcargo +1.91 test --locked -p hft-collector --lib --features collector-binance— 135 passed, 1 ignoredcargo +1.91 test --locked -p hft-collector --bins --features collector-binance— 28 + 10 + 1 passedcargo +1.91 clippy --locked -p hft-collector --lib --features collector-binance --no-deps -- -D warningsgit diff --check origin/main...HEADThe broader
--all-targets -D warningscommand is not claimed by this PR: currentmainreports pre-existing dead-code warnings for Binancefallback_symbolsandbinance_spot_pairs. This PR does not modify those symbols.Rollout/rollback impact
Collector-owned staging evidence only. It creates no evaluator label, prediction snapshot identity, runtime activation, deployment, promotion, or live-trading authority. Reverting this PR removes evidence normalization while leaving raw validation and contract selection intact.
Architecture and scope note
The content digest is explicitly not a
snapshot_contract_hash. Output excludesraw_market,raw_trade,resolved_up_won, primaryoutcome, collector-derived Up/Down token mapping, and semantic winner/settled-price label mapping. Trade evidence retains onlysource_outcome, token identity/index, and source facts; orderbook evidence retains token identity and source facts; official settlement evidence retains the source token/outcome/price arrays and official winning fields without deriving an evaluator label.The final NDJSON encoder rejects
outcome,up_token_id, anddown_token_idon every surface row. Its regression round-trips reversed source token/outcome arrays unchanged and fails each forbidden field by presence.Atomic scope exception
This change is 2 files and 775 non-generated inserted lines, 25 lines above the 750-line split-assessment threshold. Matt Standards reviewer
/root/pr96_standards_reviewexplicitly APPROVED the atomic exception. The output-boundary guard and its targeted counterexample are inseparable from the source-neutral normalizer safety contract: a test-only split cannot pass; a code-only split violates the required safety-boundary counterexample; extracting a generic encoder PR would add speculative abstraction. This remains one Research-domain behavior and rollback unit.