Skip to content

fix(data): preserve unknown activity side values#305

Open
aproskill81 wants to merge 1 commit intoPolymarket:mainfrom
aproskill81:fix/data-preserve-unknown-activity-side
Open

fix(data): preserve unknown activity side values#305
aproskill81 wants to merge 1 commit intoPolymarket:mainfrom
aproskill81:fix/data-preserve-unknown-activity-side

Conversation

@aproskill81
Copy link
Copy Markdown

@aproskill81 aproskill81 commented Mar 27, 2026

Summary

Preserve unknown Activity.side values instead of dropping them during Data API deserialization.

Problem

Activity.side is deserialized through a custom helper that currently maps:

  • BUY -> Some(Side::Buy)
  • SELL -> Some(Side::Sell)
  • empty string -> None
  • anything else -> None

This loses information for forward-compatibility cases, even though Side already supports Unknown(String).

As a result, if the API introduces a new non-empty side value, the SDK silently drops it instead of preserving the raw value.

Fix

Update deserialize_optional_side so that:

  • BUY deserializes to Some(Side::Buy)
  • SELL deserializes to Some(Side::Sell)
  • empty string still deserializes to None
  • any other non-empty value deserializes to Some(Side::Unknown(raw))

Tests

Added coverage for:

  • preserving an unknown side value
  • keeping empty string behavior as None

Note

Low Risk
Low risk: small, localized change to Activity.side deserialization plus tests; behavior only differs for previously-unrecognized non-empty side values.

Overview
Updates Data API Activity.side deserialization to preserve any non-empty, unrecognized side string as Some(Side::Unknown(raw)) rather than coercing it to None (while keeping empty strings as None).

Adds test coverage to ensure unknown side values round-trip into Side::Unknown and that empty side strings continue to deserialize to None.

Written by Cursor Bugbot for commit 938b4ba. This will update automatically on new commits. Configure here.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.58%. Comparing base (da07892) to head (938b4ba).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #305      +/-   ##
==========================================
+ Coverage   85.54%   85.58%   +0.04%     
==========================================
  Files          32       32              
  Lines        5167     5169       +2     
==========================================
+ Hits         4420     4424       +4     
+ Misses        747      745       -2     
Flag Coverage Δ
rust 85.58% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Zorak556 added a commit to Zorak556/polymarket-client-sdk that referenced this pull request Apr 10, 2026
Upstream merges:
- Polymarket#321: accept custom reqwest::Client via Config
- Polymarket#322: BuilderTradeResponse.builder Address → ApiKey
- Polymarket#306: add match_time_nano to TradeResponse
- Polymarket#305: preserve unknown activity side values
- Polymarket#303: version bump to 0.4.5
- Dep bumps: rust_decimal 1.41, uuid 1.23

Local patches:
- Reduce auth header allocations (HeaderValue::from for integers)
- Reorder order_builder size validation before async fee_rate call
- Skip double deserialization when tracing is disabled
- subscribe_book_and_prices: single-stream book + price_change
- subscribe_user/orders/trades: async for tokio::sync::RwLock
- Subscription lag: escalate to error and force resubscribe
- Broadcast capacity: 1024 → 16384
- excludeDepositsWithdrawals param on ActivityRequest
- cancel_order: orderId → orderID

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Zorak556 added a commit to Zorak556/polymarket-client-sdk that referenced this pull request Apr 10, 2026
Upstream merges:
- Polymarket#321: accept custom reqwest::Client via Config
- Polymarket#322: BuilderTradeResponse.builder Address → ApiKey
- Polymarket#306: add match_time_nano to TradeResponse
- Polymarket#305: preserve unknown activity side values
- Polymarket#303: version bump to 0.4.5
- Dep bumps: rust_decimal 1.41, uuid 1.23

Local patches:
- Reduce auth header allocations (HeaderValue::from for integers)
- Reorder order_builder size validation before async fee_rate call
- Skip double deserialization when tracing is disabled
- subscribe_book_and_prices: single-stream book + price_change
- subscribe_user/orders/trades: async for tokio::sync::RwLock
- Subscription lag: escalate to error and force resubscribe
- Broadcast capacity: 1024 → 16384
- excludeDepositsWithdrawals param on ActivityRequest
- cancel_order: orderId → orderID

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant