Since i'm seeing IB records with a `None` value and i don't want to be
debugging every time order-mode boots up..
Also use `pdb=debug` in `.open_ledger_dfs()`
Note, this had conflicts on `piker/accounting/calc.py` when rebasing
onto the refactored `brokers_refinery` history which were resolved
manually!
To start this is just a shell for the test, there's no checking logic
yet.. put it as `test_accounting.test_ib_account_with_duplicated_mktids()`.
The test is composed for now to be completely runtime-free using only
the offline txn-ledger / symcache / account loading APIs, ideally we
fill in the activated symbology-data-runtime cases once we figure a sane
way to handle incremental symcache updates for backends like IB..
To actually fill the test out with real checks we still need to,
- extract the problem account file from my ib.algopape into the test
harness data.
- pick some contracts with multiple fqmes despite a single bs_mktid and
ensure they're aggregated as a single `Position` as well as,
* ideally de-duplicating txns from the account file section for the
mkt..
* warning appropriately about greater-then-one fqme for the bs_mktid
and providing a way for the ledger re-writing to choose the
appropriate `<venue>` as the "primary" when the
data-symbology-runtime is up and possibly use it to incrementally
update the IB symcache and store offline for next use?
Since under `trio`-cancellation the `.remove()` is a checkpoint and will
be masked by a taskc AND we **always want to remove the rect** despite
the surrounding teardown conditions.
In the `translate_and_relay_brokerd_events()` loop task that is, such
that we never crash on a `status_msg = book._active.pop(oid)` in the
'closed' status handler whenever a double removal happens.
Turns out there were unforeseen races here when a benign backend error
would cause an order-mode dialog to be cancelled (incorrectly) and then
a UI side `.on_cancel()` would trigger too-early removal from the
`book._active` table despite the backend sending an actual 'closed'
event (much) later, this would crash on the now missing entry..
So instead we now,
- obviously use `book._active.pop(oid, None)`
- emit a `log.warning()` (not info lol) on a null-read and with a less
"one-line-y" message explaining the double removal and maybe *why*.
For backends which opt to set the new `BrokerdPosition.bs_mktid` field,
give (matching logic) priority to it such that even if the `.symbol`
field doesn't match the mkt currently focussed on chart, it will
always match on a provider's own internal asset-mapping-id. The original
fallback logic for `.fqme` matching is left as is.
As an example with IB, a qqq.nasdaq.ib txn may have been filled on
a non-primary venue as qqq.directedea.ib, in this case if the mkt is
displayed and focused on chart we want the **entire position info** to
be overlayed by the `OrderMode` UX without discrepancy.
Other refinements,
- improve logging and add a detailed edge-case-comment around the
`.on_fill()` handler to clarify where if a benign 'error' msg is
relayed from a backend it will cause the UI to operate as though the
order **was not-cleared/cancelled** since the `.on_cancel()` handler
will have likely been called just before, popping the `.dialogs`
entry. Return `bool` to indicate whether the UI removed-lines
/ added-fill-arrows.
- inverse the `return` branching logic in `.on_cancel()` to reduce
indent.
- add a very loud `log.error()` in `Status(resp='error')` case-block
ensuring the console yells about the order being cancelled, also
a todo for the weird msg-field recursion nonsense..
Such that backends can deliver their own internal unique
`MktPair.bs_mktid` when they can't seem to get it right via the
`.fqme: str` export.. (COUGH ib, you piece of sh#$).
Also add todo for possibly replacing the msg with a `Position.summary()`
"snapshot" as a better and more rigorously generated wire-ready msg.
Despite a `.bs_mktid` ideally being a bijection with `MktPair.fqme`
values, apparently some backends (cough IB) will switch the .<venue>`
part in txn records resulting in multiple account-conf-file sections for
the same dst asset. Obviously that means we can't allocate new
`Position` entries keyed by that `bs_mktid`, instead be sure to **update
them instead**!
Deats,
- add case logic to avoid pp overwrites using a `pp_objs.get()` check.
- warn on duplicated pos entries whenever the current account-file
entry's `mkt` doesn't match the pre-existing position's.
- mk `Position.add_clear()` return a `bool` indicating if the record was
newly added, warn when it was already existing/added prior.
Also,
- drop the already deprecated `open_pps()`, also from sub-pkg exports.
- draft TODO for `Position.summary()` idea as a replacement for
`BrokerdPosition`-msgs.
That is inside embedded `.accounting.calc.dyn_parse_to_dt()` closure add
an optional `_invalid: list` param to where we can report
bad-timestamped records which we instead override and return as
`from_timestamp(0.)` (when the parser loop falls through) and report
later (in summary ) from the `.accounting.calc.iter_by_dt()` caller. Add
some logging and an optional debug block for future tracing.
NOTE, this commit was re-edited during a conflict between the orig
branches: `dev/binance_api_3.1` & `dev/alt_tpts_for_perf`.
Tyler Goodlet