Repository navigation
Conversation
chyunsu3
changed the title
chyunsu3
force-pushed
the
next-protobuf
branch
from
October 9, 2026 06:12
6804dd0 to
1580926
Compare
mattsta
added a commit
that referenced
this pull request
Oct 13, 2026
…ncelled What: `OrderStatus.terminalFromCompleted` classifies the status a completed-order replay reports: a plain `DoneStates` name passes through, the descriptive text TWS 10.51 sends — `Filled Size: <n>` and `Cancelled by <who>[:\n<reason>]` — maps to Filled / Cancelled, and anything else returns None. `Wrapper.completedOrder` uses it and keeps its Inactive-with-warning fallback for None. Any rewritten status still logs the original text on the trade. Tests cover the classifier table, the wire shape (text in `completedStatus`, `status` empty), and the completed echo terminalizing a trade already seen live. Why: on TWS 10.51 `completedStatus` carries text instead of the status names IBKR documents for it, and `status` stays empty on that path, so none of it matched a terminal state. Every filled or cancelled order from `reqCompletedOrders` came back as Inactive with a "non-terminal status" warning, leaving fills and cancels indistinguishable. The trades stayed out of `openTrades()`, but their reported outcome was wrong. How: the phrasing knowledge lives in one table beside the status vocabulary, not inline in the wrapper, so the next phrasing IBKR introduces is a one-line, separately testable addition. Only phrasings observed on the wire are mapped. Unknown text still falls back to Inactive, so a future wording change degrades the way it did before instead of being guessed. `Filled Size` also accepts fractional and comma-grouped sizes, which can only mean a fill. Based on Hyunsu Cho's report and fix in #237. That version matched the text inline; this one moves it into OrderStatus and tests the field the text actually arrives in. Closes #237. Co-Authored-By: Hyunsu Cho <chohyu01@gmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Interesting find! Thanks for tracking that down so we can use more "actual meaning status details" for status reporting instead of less detailed "status fields" themselves. I had this refactored a bit for just common platform structure. It'll be in the next major release here which will have to be soon because IBKR is moving to 10.51+ required by end of year. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TWS 1051 now emits new status messages:
Filled Size: 10Cancelled by TraderCancelled by System:\nCancel parent on new cancelling orderI use
ib_asyncevery day and would like to say thank you for this great software.