ui.order_mode: prioritize mkt-match on `.bs_mktid`

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..

piker/accounting/__init__.py

@@ -33,7 +33,6 @@ from ._pos import (
     Account,
     load_account,
     load_account_from_ledger,
-    open_pps,
     open_account,
     Position,
 )
@@ -68,7 +67,6 @@ __all__ = [
     'load_account_from_ledger',
     'mk_allocator',
     'open_account',
-    'open_pps',
     'open_trade_ledger',
     'unpack_fqme',
     'DerivTypes',

piker/accounting/_pos.py

@@ -353,13 +353,12 @@ class Position(Struct):
     ) -> bool:
         '''
         Update clearing table by calculating the rolling ppu and
-        (accumulative) size in both the clears entry and local
-        attrs state.
+        (accumulative) size in both the clears entry and local attrs
+        state.
 
         Inserts are always done in datetime sorted order.
 
         '''
-        # added: bool = False
         tid: str = t.tid
         if tid in self._events:
             log.debug(
@@ -367,7 +366,7 @@ class Position(Struct):
                 f'\n'
                 f'{t}\n'
             )
-            # return added
+            return False
 
         # TODO: apparently this IS possible with a dict but not
         # common and probably not that beneficial unless we're also
@@ -448,6 +447,12 @@ class Position(Struct):
     # def suggest_split(self) -> float:
     #     ...
 
+    # ?TODO, for sending rendered state over the wire?
+    # def summary(self) -> PositionSummary:
+        # do minimal conversion to a subset of fields
+        # currently defined in `.clearing._messages.BrokerdPosition`
+
+
 
 class Account(Struct):
     '''
@@ -714,7 +719,7 @@ class Account(Struct):
         # XXX WTF: if we use a tomlkit.Integer here we get this
         # super weird --1 thing going on for cumsize!?1!
         # NOTE: the fix was to always float() the size value loaded
-        # in open_pps() below!
+        # in open_account() below!
         config.write(
             config=self.conf,
             path=self.conf_path,
@@ -898,7 +903,6 @@ def open_account(
             clears_table['dt'] = dt
             trans.append(Transaction(
                 fqme=bs_mktid,
-                # sym=mkt,
                 bs_mktid=bs_mktid,
                 tid=tid,
                 # XXX: not sure why sometimes these are loaded as
@@ -921,7 +925,18 @@ def open_account(
         ):
             expiry: pendulum.DateTime = pendulum.parse(expiry)
 
-        pp = pp_objs[bs_mktid] = Position(
+        # !XXX, should never be duplicates over
+        # a backend-(broker)-system's unique market-IDs!
+        if pos := pp_objs.get(bs_mktid):
+            if mkt != pos.mkt:
+                log.warning(
+                    f'Duplicated position but diff `MktPair.fqme` ??\n'
+                    f'bs_mktid: {bs_mktid!r}\n'
+                    f'pos.mkt: {pos.mkt}\n'
+                    f'mkt: {mkt}\n'
+                )
+        else:
+            pos = pp_objs[bs_mktid] = Position(
                 mkt,
                 split_ratio=split_ratio,
                 bs_mktid=bs_mktid,
@@ -933,8 +948,13 @@ def open_account(
         # state, since today's records may have already been
         # processed!
         for t in trans:
-            pp.add_clear(t)
-
+            added: bool = pos.add_clear(t)
+            if not added:
+                log.warning(
+                    f'Txn already recorded in pp ??\n'
+                    f'\n'
+                    f'{t}\n'
+                )
     try:
         yield acnt
     finally:
@@ -942,20 +962,6 @@ def open_account(
             acnt.write_config()
 
 
-# TODO: drop the old name and THIS!
-@cm
-def open_pps(
-    *args,
-    **kwargs,
-) -> Generator[Account, None, None]:
-    log.warning(
-        '`open_pps()` is now deprecated!\n'
-        'Please use `with open_account() as cnt:`'
-    )
-    with open_account(*args, **kwargs) as acnt:
-        yield acnt
-
-
 def load_account_from_ledger(
 
     brokername: str,

piker/brokers/ib/broker.py

@@ -358,6 +358,10 @@ async def update_and_audit_pos_msg(
         size=ibpos.position,
 
         avg_price=pikerpos.ppu,
+
+        # XXX ensures matching even if multiple venue-names
+        # in `.bs_fqme`, likely from txn records..
+        bs_mktid=mkt.bs_mktid,
     )
 
     ibfmtmsg: str = pformat(ibpos._asdict())
@@ -426,7 +430,8 @@ async def aggr_open_orders(
 
 ) -> None:
     '''
-    Collect all open orders from client and fill in `order_msgs: list`.
+    Collect all open orders from client and fill in `order_msgs:
+    list`.
 
     '''
     trades: list[Trade] = client.ib.openTrades()

piker/clearing/_messages.py

@@ -301,6 +301,9 @@ class BrokerdError(Struct):
 
 # TODO: yeah, so we REALLY need to completely deprecate
 # this and use the `.accounting.Position` msg-type instead..
+# -[ ] an alternative might be to add a `Position.summary() ->
+#   `PositionSummary`-msg that we generate since `Position` has a lot
+#   of fields by default we likely don't want to send over the wire?
 class BrokerdPosition(Struct):
     '''
     Position update event from brokerd.
@@ -313,3 +316,4 @@ class BrokerdPosition(Struct):
     avg_price: float
     currency: str = ''
     name: str = 'position'
+    bs_mktid: str|int|None = None

piker/ui/order_mode.py

@@ -555,14 +555,13 @@ class OrderMode:
 
     def on_fill(
         self,
-
         uuid: str,
         price: float,
         time_s: float,
 
         pointing: str | None = None,
 
-    ) -> None:
+    ) -> bool:
         '''
         Fill msg handler.
 
@@ -575,13 +574,33 @@ class OrderMode:
         - update fill bar size
 
         '''
-        dialog = self.dialogs[uuid]
+        # XXX WARNING XXX
+        # if a `Status(resp='error')` arrives *before* this
+        # fill-status, the `.dialogs` entry may have already been
+        # popped and thus the below will skipped.
+        #
+        # NOTE, to avoid this confusing scenario ensure that any
+        # errors delivered thru from the broker-backend are not just
+        # "noisy reporting" (like is very common from IB..) and are
+        # instead ONLY errors-causing-order-dialog-cancellation!
+        if not (dialog := self.dialogs.get(uuid)):
+            log.warning(
+                f'Order was already cleared from `.dialogs` ??\n'
+                f'uuid: {uuid!r}\n'
+            )
+            return False
+
         lines = dialog.lines
         chart = self.chart
 
-        # XXX: seems to fail on certain types of races?
+        if not lines:
+            log.warn("No line(s) for order {uuid}!?")
+            return False
+
+        # update line state(s)
+        #
+        # ?XXX this fails on certain types of races?
         # assert len(lines) == 2
-        if lines:
         flume: Flume = self.feed.flumes[chart.linked.mkt.fqme]
         _, _, ratio = flume.get_ds_info()
 
@@ -607,28 +626,31 @@ class OrderMode:
                 pointing=pointing,
                 color=lines[0].color
             )
-        else:
-            log.warn("No line(s) for order {uuid}!?")
 
     def on_cancel(
         self,
-        uuid: str
+        uuid: str,
 
-    ) -> None:
+    ) -> bool:
 
-        msg: Order = self.client._sent_orders.pop(uuid, None)
-
-        if msg is not None:
-            self.lines.remove_line(uuid=uuid)
-            self.chart.linked.cursor.show_xhair()
-
-            dialog = self.dialogs.pop(uuid, None)
-            if dialog:
-                dialog.last_status_close()
-        else:
+        msg: Order|None = self.client._sent_orders.pop(uuid, None)
+        if msg is None:
             log.warning(
                 f'Received cancel for unsubmitted order {pformat(msg)}'
             )
+            return False
+
+        # remove GUI line, show cursor.
+        self.lines.remove_line(uuid=uuid)
+        self.chart.linked.cursor.show_xhair()
+
+        # remove msg dialog (history)
+        dialog: Dialog|None = self.dialogs.pop(uuid, None)
+        if dialog:
+            dialog.last_status_close()
+
+        return True
+
 
     def cancel_orders_under_cursor(self) -> list[str]:
         return self.cancel_orders(
@@ -1057,13 +1079,23 @@ async def process_trade_msg(
     if name in (
         'position',
     ):
-        sym: MktPair = mode.chart.linked.mkt
+        mkt: MktPair = mode.chart.linked.mkt
         pp_msg_symbol = msg['symbol'].lower()
-        fqme = sym.fqme
-        broker = sym.broker
+        pp_msg_bsmktid = msg['bs_mktid']
+        fqme = mkt.fqme
+        broker = mkt.broker
         if (
+            # match on any backed-specific(-unique)-ID first!
+            (
+                pp_msg_bsmktid
+                and
+                mkt.bs_mktid == pp_msg_bsmktid
+            )
+            or
+            # OW try against what's provided as an FQME..
             pp_msg_symbol == fqme
-            or pp_msg_symbol == fqme.removesuffix(f'.{broker}')
+            or
+            pp_msg_symbol == fqme.removesuffix(f'.{broker}')
         ):
             log.info(
                 f'Loading position for `{fqme}`:\n'
@@ -1086,7 +1118,7 @@ async def process_trade_msg(
         return
 
     msg = Status(**msg)
-    resp = msg.resp
+    # resp: str = msg.resp
     oid = msg.oid
     dialog: Dialog = mode.dialogs.get(oid)
 
@@ -1150,19 +1182,32 @@ async def process_trade_msg(
                         mode.on_submit(oid)
 
         case Status(resp='error'):
-
-            # do all the things for a cancel:
-            # - drop order-msg dialog from client table
-            # - delete level line from view
-            mode.on_cancel(oid)
-
             # TODO: parse into broker-side msg, or should we
             # expect it to just be **that** msg verbatim (since
             # we'd presumably have only 1 `Error` msg-struct)
             broker_msg: dict = msg.brokerd_msg
+
+            # XXX NOTE, this presumes the rxed "error" is
+            # order-dialog-cancel-causing, THUS backends much ONLY
+            # relay errors of this "severity"!!
             log.error(
-                f'Order {oid}->{resp} with:\n{pformat(broker_msg)}'
+                f'Order errored ??\n'
+                f'oid: {oid!r}\n'
+                f'\n'
+                f'{pformat(broker_msg)}\n'
+                f'\n'
+                f'=> CANCELLING ORDER DIALOG <=\n'
+
+                # from tractor.devx.pformat import ppfmt
+                # !TODO LOL, wtf the msg is causing
+                # a recursion bug!
+                # -[ ] get this shit on msgspec stat!
+                # f'{ppfmt(broker_msg)}'
             )
+            # do all the things for a cancel:
+            # - drop order-msg dialog from client table
+            # - delete level line from view
+            mode.on_cancel(oid)
 
         case Status(resp='canceled'):
             # delete level line from view
@@ -1178,10 +1223,10 @@ async def process_trade_msg(
             # TODO: UX for a "pending" clear/live order
             log.info(f'Dark order triggered for {fmtmsg}')
 
-        case Status(
-            resp='triggered',
         # TODO: do the struct-msg version, blah blah..
         # req=Order(exec_mode='live', action='alert') as req,
+        case Status(
+            resp='triggered',
             req={
                 'exec_mode': 'live',
                 'action': 'alert',