Fix OCC option symbol classification priority#764
Conversation
afadil
force-pushed
the
claude/analyze-issue-630-xeOlZ
branch
from
86d88a7 to
8e52107
Compare
![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: 86d88a7613
ℹ️ 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".
| Some(format!( | ||
| "{}:{}", | ||
| InstrumentType::Equity.as_db_str(), | ||
| upper_symbol |
There was a problem hiding this comment.
The fallback for previously misclassified OCC options only builds EQUITY:{SYMBOL}, but the legacy misclassification path described in this change happens when an exchange MIC is present, which stores keys as EQUITY:{SYMBOL}@{MIC} (same format used in expected_key for non-crypto instruments). In that common case, this fallback never matches and the code will still create duplicate option assets instead of reusing the existing misclassified one.
Useful? React with 👍 / 👎.
| if let Some(ref fallback) = fallback_equity_key { | ||
| if asset_key == Some(fallback.as_str()) { | ||
| return Some(asset.id.clone()); |
There was a problem hiding this comment.
The loop checks the fallback equity key for each asset immediately after checking the exact key, so if a legacy misclassified asset appears earlier in assets than a correctly keyed option asset, the function returns the legacy ID and never reaches the exact match. This makes matching order-dependent and can keep attaching new activities to the wrong asset even when a correct option record exists.
Useful? React with 👍 / 👎.
OCC option symbols (e.g. SLV260821C00065000) were misclassified as EQUITY when a search provider attached an exchange MIC, because the MIC check ran before the OCC heuristic in infer_asset_kind. This caused sell transactions to create a new asset instead of matching the existing one, splitting holdings across two assets. - Move OCC symbol detection before exchange MIC check in infer_asset_kind - Add fallback lookup in find_existing_asset_id to match options that were already stored as EQUITY due to the prior classification bug https://claude.ai/code/session_01398hEBBYCG9Jwjms4wzSJP
- Build fallback key with @mic suffix when exchange MIC is present, matching the format the old misclassification code would have stored - Separate exact-match and fallback into two passes so a correctly keyed option asset is always preferred over a legacy misclassified one
afadil
force-pushed
the
claude/analyze-issue-630-xeOlZ
branch
from
b1eef11 to
a2256a8
Compare
1 participant
Description
This PR fixes the instrument type classification logic to correctly identify OCC option symbols before checking for exchange MIC. Previously, OCC option symbols (e.g.,
AAPL240119C00150000) were being misclassified as equities when an exchange MIC like "OPRA" was attached by search providers.Changes
Reordered classification heuristics: Moved the OCC option symbol check (step 3) before the exchange MIC check (now step 4). This ensures options are correctly identified even when they have an exchange MIC attached.
Added fallback lookup logic: When resolving asset IDs, the code now includes a fallback mechanism to find previously misclassified OCC options that were stored with an EQUITY instrument type key. This maintains backward compatibility with existing data.
Updated step numbering: Adjusted comments to reflect the new order (steps 4-6).
Why This Matters
Search providers may attach exchange MICs (like "OPRA" for options) to symbols, which would cause the old logic to classify them as equities. By checking the OCC symbol pattern first, we ensure options are correctly identified regardless of whether an exchange MIC is present.
Checklist
Contributor License Agreement.
By submitting this PR, I agree to the
CLA.
https://claude.ai/code/session_01398hEBBYCG9Jwjms4wzSJP