From 0cf4e07b84e66c5395fc7009a7ea96dfcb4af273 Mon Sep 17 00:00:00 2001
From: Tyler Goodlet <jgbt@protonmail.com>
Date: Mon, 15 Aug 2022 10:42:58 -0400
Subject: [PATCH] Use `datetime` sorting on clears table appends

In order to avoid issues with reloading ledger and API trades after an
existing `pps.toml` exists we have to make sure we not only avoid
duplicate entries but also avoid re-adding entries that would have been
removed during a prior call to the `Position.minimize_clears()` filter.
The easiest way to do this is to sort on timestamps and avoid adding any
record that pre-existed the last net-zero position ledger event that
`.minimize_clears()` discarded. In order to implement this it means
parsing config file clears table's timestamps into datetime objects for
inequality checks and we add a `Position.first_clear_dt` attr for
storing this value when managing pps in object form but never store it
in the config (since it should be obviously from the sorted clear event
table).
---
 piker/pp.py | 60 +++++++++++++++++++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 20 deletions(-)

diff --git a/piker/pp.py b/piker/pp.py
index 0ff8c9d5..27ab7af8 100644
--- a/piker/pp.py
+++ b/piker/pp.py
@@ -141,6 +141,7 @@ class Position(Struct):
         Union[str, int, Status],  # trade id
         dict[str, Any],  # transaction history summaries
     ] = {}
+    first_clear_dt: Optional[datetime] = None
 
     expiry: Optional[datetime] = None
 
@@ -164,6 +165,9 @@ class Position(Struct):
         if self.split_ratio is None:
             d.pop('split_ratio')
 
+        # should be obvious from clears/event table
+        d.pop('first_clear_dt')
+
         # TODO: we need to figure out how to have one top level
         # listing venue here even when the backend isn't providing
         # it via the trades ledger..
@@ -189,7 +193,8 @@ class Position(Struct):
         ):
             inline_table = toml.TomlDecoder().get_empty_inline_table()
 
-            inline_table['dt'] = data['dt']
+            # serialize datetime to parsable `str`
+            inline_table['dt'] = str(data['dt'])
 
             # insert optional clear fields in column order
             for k in ['ppu', 'accum_size']:
@@ -236,6 +241,10 @@ class Position(Struct):
             )
             ppu = cppu
 
+        self.first_clear_dt = min(
+            list(entry['dt'] for entry in self.clears.values())
+        )
+
         return size, ppu
 
     def update_from_msg(
@@ -449,15 +458,16 @@ class Position(Struct):
             'cost': t.cost,
             'price': t.price,
             'size': t.size,
-            'dt': str(t.dt),
+            'dt': t.dt,
         }
 
         # TODO: compute these incrementally instead
         # of re-looping through each time resulting in O(n**2)
-        # behaviour..
-        # compute these **after** adding the entry
-        # in order to make the recurrence relation math work
-        # inside ``.calc_size()``.
+        # behaviour..?
+
+        # NOTE: we compute these **after** adding the entry in order to
+        # make the recurrence relation math work inside
+        # ``.calc_size()``.
         self.size = clear['accum_size'] = self.calc_size()
         self.ppu = clear['ppu'] = self.calc_ppu()
 
@@ -499,16 +509,22 @@ class PpTable(Struct):
                     expiry=t.expiry,
                 )
             )
+            clears = pp.clears
+            if clears:
+                first_clear_dt = pp.first_clear_dt
 
-            # don't do updates for ledger records we already have
-            # included in the current pps state.
-            if t.tid in pp.clears:
-                # NOTE: likely you'll see repeats of the same
-                # ``Transaction`` passed in here if/when you are restarting
-                # a ``brokerd.ib`` where the API will re-report trades from
-                # the current session, so we need to make sure we don't
-                # "double count" these in pp calculations.
-                continue
+                # don't do updates for ledger records we already have
+                # included in the current pps state.
+                if (
+                    t.tid in clears
+                    or first_clear_dt and t.dt < first_clear_dt
+                ):
+                    # NOTE: likely you'll see repeats of the same
+                    # ``Transaction`` passed in here if/when you are restarting
+                    # a ``brokerd.ib`` where the API will re-report trades from
+                    # the current session, so we need to make sure we don't
+                    # "double count" these in pp calculations.
+                    continue
 
             # update clearing table
             pp.add_clear(t)
@@ -850,17 +866,21 @@ def open_pps(
         # index clears entries in "object" form by tid in a top
         # level dict instead of a list (as is presented in our
         # ``pps.toml``).
-        pp = pp_objs.get(bsuid)
-        if pp:
-            clears = pp.clears
-        else:
-            clears = {}
+        clears = pp_objs.setdefault(bsuid, {})
 
+        # TODO: should be make a ``Struct`` for clear/event entries?
+        # convert "clear events table" from the toml config (list of
+        # a dicts) and load it into object form for use in position
+        # processing of new clear events.
         for clears_table in clears_list:
             tid = clears_table.pop('tid')
+            dtstr = clears_table['dt']
+            dt = pendulum.parse(dtstr)
+            clears_table['dt'] = dt
             clears[tid] = clears_table
 
         size = entry['size']
+
         # TODO: remove but, handle old field name for now
         ppu = entry.get('ppu', entry.get('be_price', 0))
         split_ratio = entry.get('split_ratio')