accrual: AccrualPostings.make_consistent() groups accruals by date.

This accommodates cases of contracts without separate invoices,
where a series of payments are scheduled over time.

The dance we used to do of group-by-invoice, then make consistent was
already suspect. It was originally motivated by the consistency checks that
are now gone. Use this opportunity to clean up and just make make_consistent
a classmethod.

conservancy_beancount/reports/accrual.py

@@ -81,6 +81,9 @@ from typing import (
     Any,
     BinaryIO,
     Callable,
+    Deque,
+    Dict,
+    Hashable,
     Iterable,
     Iterator,
     List,
@@ -119,7 +122,7 @@ from .. import rtutil
 
 PROGNAME = 'accrual-report'
 
-PostGroups = Mapping[Optional[str], 'AccrualPostings']
+PostGroups = Mapping[Optional[Hashable], 'AccrualPostings']
 T = TypeVar('T')
 
 logger = logging.getLogger('conservancy_beancount.reports.accrual')
@@ -166,6 +169,7 @@ class AccrualAccount(enum.Enum):
 class AccrualPostings(core.RelatedPostings):
     __slots__ = (
         'accrual_type',
+        'date',
         'end_balance',
         'account',
         'entity',
@@ -183,12 +187,15 @@ class AccrualPostings(core.RelatedPostings):
         if isinstance(self.account, Sentinel):
             self.accrual_type: Optional[AccrualAccount] = None
             norm_func: Callable[[T], T] = lambda x: x
-            entity_pred: Callable[[data.Posting], bool] = bool
+            accrual_pred: Callable[[data.Posting], bool] = bool
         else:
             self.accrual_type = AccrualAccount.by_account(self.account)
             norm_func = self.accrual_type.normalize_amount
-            entity_pred = lambda post: norm_func(post.units).number > 0
+            accrual_pred = lambda post: norm_func(post.units).number > 0
-        self.entity = self._single_item(self.entities(entity_pred))
+            if not any(accrual_pred(post) for post in self):
+                accrual_pred = bool
+        self.date = self._single_item(self.dates(accrual_pred))
+        self.entity = self._single_item(self.entities(accrual_pred))
         self.invoice = self._single_item(self.first_meta_links('invoice', None))
         self.end_balance = norm_func(self.balance_at_cost())
 
@@ -202,6 +209,9 @@ class AccrualPostings(core.RelatedPostings):
             all_same = all(item == item1 for item in items)
         return item1 if all_same else self.INCONSISTENT
 
+    def dates(self, pred: Callable[[data.Posting], bool]=bool) -> Iterator[datetime.date]:
+        return filters.iter_unique(post.meta.date for post in self if pred(post))
+
     def entities(self, pred: Callable[[data.Posting], bool]=bool) -> Iterator[MetaValue]:
         return filters.iter_unique(
             post.meta['entity']
@@ -209,29 +219,52 @@ class AccrualPostings(core.RelatedPostings):
             if pred(post) and 'entity' in post.meta
         )
 
-    def make_consistent(self) -> Iterator[Tuple[str, 'AccrualPostings']]:
+    @classmethod
-        account_ok = isinstance(self.account, str)
+    def make_consistent(cls,
-        entity_ok = isinstance(self.entity, str)
+                        postings: Iterable[data.Posting],
-        # `'/' in self.invoice` is just our heuristic to ensure that the
+    ) -> Iterator[Tuple[Hashable, 'AccrualPostings']]:
-        # invoice metadata is "unique enough," and not just a placeholder
+        accruals: Dict[Tuple[str, ...], List[data.Posting]] = collections.defaultdict(list)
-        # value like "FIXME". It can be refined if needed.
+        payments: Dict[Tuple[str, ...], Deque[data.Posting]] = collections.defaultdict(Deque)
-        invoice_ok = isinstance(self.invoice, str) and '/' in self.invoice
+        key: Tuple[str, ...]
-        if account_ok and entity_ok and invoice_ok:
+        for post in postings:
-            # type ignore for <https://github.com/python/mypy/issues/6670>
+            norm_func = core.normalize_amount_func(post.account)
-            yield (self.invoice, self)  # type:ignore[misc]
+            invoice = str(post.meta.get('invoice', 'BlankInvoice'))
-            return
+            if norm_func(post.units.number) >= 0:
-        groups = collections.defaultdict(list)
+                entity = str(post.meta.get('entity', 'BlankEntity'))
-        for post in self:
+                key = (post.meta.date.isoformat(), entity, invoice, post.account)
-            post_invoice = self.invoice if invoice_ok else (
+                accruals[key].append(post)
-                post.meta.get('invoice') or 'BlankInvoice'
+            else:
-            )
+                key = (invoice, post.account)
-            post_entity = self.entity if entity_ok else (
+                payments[key].append(post)
-                post.meta.get('entity') or 'BlankEntity'
+
-            )
+        for key, acc_posts in accruals.items():
-            groups[f'{post.account} {post_invoice} {post_entity}'].append(post)
+            pay_posts = payments[key[2:]]
-        type_self = type(self)
+            if not pay_posts:
-        for group_key, posts in groups.items():
+                continue
-            yield group_key, type_self(posts, _can_own=True)
+            norm_func = core.normalize_amount_func(key[-1])
+            balance = norm_func(core.MutableBalance(post.at_cost() for post in acc_posts))
+            while pay_posts and not balance.le_zero():
+                pay_post = pay_posts.popleft()
+                acc_posts.append(pay_post)
+                balance += norm_func(pay_post.at_cost())
+            if balance.le_zero() and not balance.is_zero():
+                # pay_post causes the accrual to be overpaid. Split it into two
+                # synthesized postings: one that causes the accrual to be
+                # exactly zero, and one with the remainder back in payments.
+                post_cost = pay_post.at_cost()
+                # Calling norm_func() reverses the call in the while loop to add
+                # the amount to the balance.
+                overpayment = norm_func(balance[post_cost.currency])
+                amt_to_zero = post_cost._replace(number=post_cost.number - overpayment.number)
+                acc_posts[-1] = pay_post._replace(units=amt_to_zero, cost=None, price=None)
+                pay_posts.appendleft(pay_post._replace(units=overpayment, cost=None, price=None))
+            acc_posts.sort(key=lambda post: post.meta.date)
+
+        for key, acc_posts in accruals.items():
+            yield key, cls(acc_posts, _can_own=True)
+        for key, pay_posts in payments.items():
+            if pay_posts:
+                yield key, cls(pay_posts, _can_own=True)
 
     def is_paid(self, default: Optional[bool]=None) -> Optional[bool]:
         if self.accrual_type is None:
@@ -739,9 +772,7 @@ def main(arglist: Optional[Sequence[str]]=None,
     # but they do help symbolize what's being grouped.
     # For the outgoing approval report, groups maps rt-id link strings to
     # associated accruals.
-    # For all other reports, groups starts by grouping postings together by
+    # For all other reports, groups comes from AccrualReport.make_consistent().
-    # invoice link string, then uses AccrualReport.make_consistent() to split
-    # out groups that need it.
     groups: PostGroups
     if args.report_type is None or args.report_type is ReportType.OUTGOING:
         groups = dict(AccrualPostings.group_by_first_meta_link(postings, 'rt-id'))
@@ -754,11 +785,7 @@ def main(arglist: Optional[Sequence[str]]=None,
         ):
             args.report_type = ReportType.OUTGOING
     if args.report_type is not ReportType.OUTGOING:
-        groups = {
+        groups = dict(AccrualPostings.make_consistent(postings))
-            key: group
-            for _, source in AccrualPostings.group_by_first_meta_link(postings, 'invoice')
-            for key, group in source.make_consistent()
-        }
     if args.report_type is not ReportType.AGING:
         groups = {
             key: posts for key, posts in groups.items() if not posts.is_paid()

conservancy_beancount/reports/core.py

@@ -272,8 +272,8 @@ class RelatedPostings(Sequence[data.Posting]):
                  *,
                  _can_own: bool=False,
     ) -> None:
-        self._postings: List[data.Posting]
+        self._postings: Sequence[data.Posting]
-        if _can_own and isinstance(source, list):
+        if _can_own and isinstance(source, Sequence):
             self._postings = source
         else:
             self._postings = list(source)

tests/books/accruals.beancount

@@ -139,7 +139,7 @@
   Liabilities:Payable:Accounts  -220.00 USD
   payment-method: "USD ACH"
 
-2010-06-12 * "Lawyer" "Additional legal fees for May"
+2010-06-10 * "Lawyer" "Additional legal fees for May"
   rt-id: "rt:510"
   invoice: "rt:510/6100"
   contract: "rt:510/4000"

tests/test_reports_accrual.py

@@ -116,7 +116,9 @@ AGING_AR = [
     AgingRow.make_simple('2010-03-05', 'EarlyBird', -500, 'rt:40/400'),
     AgingRow.make_simple('2010-05-15', 'MatchingProgram', 1500,
                          'rt://ticket/515/attachments/5150'),
-    AgingRow.make_simple('2010-06-15', 'GrantCo', 11500, 'rt:470/4700',
+    AgingRow.make_simple('2010-06-15', 'GrantCo', 5500, 'rt:470/4700',
+                         project='Development Grant'),
+    AgingRow.make_simple('2010-09-15', 'GrantCo', 6000, 'rt:470/4700',
                          project='Development Grant'),
 ]
 
@@ -343,12 +345,15 @@ def test_accrual_postings_rt_id_none():
     itertools.count(1),
 ))
 def test_make_consistent_bad_invoice(acct_name, invoice, day):
+    if acct_name.startswith('Assets:'):
+        mult = 10
+    else:
+        mult = -10
     txn = testutil.Transaction(date=datetime.date(2019, 1, day), postings=[
-        (acct_name, index * 10, {'invoice': invoice, 'entity': f'BadInvoice{day}'})
+        (acct_name, index * mult, {'invoice': invoice, 'entity': f'BadInvoice{day}'})
         for index in range(1, 4)
     ])
-    related = accrual.AccrualPostings(data.Posting.from_txn(txn))
+    consistent = dict(accrual.AccrualPostings.make_consistent(data.Posting.from_txn(txn)))
-    consistent = dict(related.make_consistent())
     assert len(consistent) == 1
     key = next(iter(consistent))
     assert acct_name in key
@@ -367,8 +372,7 @@ def test_make_consistent_across_accounts():
         (acct_name, 100, {'invoice': invoice, 'entity': 'CrossAccount'})
         for acct_name in ACCOUNTS
     ])
-    related = accrual.AccrualPostings(data.Posting.from_txn(txn))
+    consistent = dict(accrual.AccrualPostings.make_consistent(data.Posting.from_txn(txn)))
-    consistent = dict(related.make_consistent())
     assert len(consistent) == len(ACCOUNTS)
     for key, posts in consistent.items():
         assert len(posts) == 1
@@ -378,8 +382,7 @@ def test_make_consistent_both_invoice_and_account():
     txn = testutil.Transaction(date=datetime.date(2019, 2, 2), postings=[
         (acct_name, 150) for acct_name in ACCOUNTS
     ])
-    related = accrual.AccrualPostings(data.Posting.from_txn(txn))
+    consistent = dict(accrual.AccrualPostings.make_consistent(data.Posting.from_txn(txn)))
-    consistent = dict(related.make_consistent())
     assert len(consistent) == len(ACCOUNTS)
     for key, posts in consistent.items():
         assert len(posts) == 1
@@ -392,8 +395,7 @@ def test_make_consistent_across_entity(acct_name):
         (acct_name, amt_sign(n), {'invoice': 'Inv/1.pdf', 'entity': f'Entity{n}'})
         for n in range(1, 4)
     ])
-    related = accrual.AccrualPostings(data.Posting.from_txn(txn))
+    consistent = dict(accrual.AccrualPostings.make_consistent(data.Posting.from_txn(txn)))
-    consistent = dict(related.make_consistent())
     assert len(consistent) == 3
     for key, posts in consistent.items():
         assert len(posts) == 1
@@ -411,12 +413,143 @@ def test_make_consistent_entity_differs_accrual_payment(acct_name):
         (acct_name, 125, {'invoice': invoice, 'entity': 'Positive'}),
         (acct_name, -125, {'invoice': invoice, 'entity': 'Negative'}),
     ])
-    related = accrual.AccrualPostings(data.Posting.from_txn(txn))
+    related = list(data.Posting.from_txn(txn))
-    consistent = related.make_consistent()
+    consistent = accrual.AccrualPostings.make_consistent(related)
     _, actual = next(consistent)
-    assert actual is related
+    assert len(actual) == len(related)
+    assert all(post in actual for post in related)
     assert next(consistent, None) is None
 
+def test_make_consistent_by_date_accruals_differ():
+    meta = {'rt-id': '1', 'invoice': 'rt:1/2', 'entity': 'MultiDate'}
+    entries = [testutil.Transaction(date=date, postings=[
+        ('Assets:Receivable:Accounts', date.day * 100, meta),
+    ]) for date in itertools.islice(testutil.date_seq(), 3)]
+    actual = [group for _, group in
+              accrual.AccrualPostings.make_consistent(data.Posting.from_entries(entries))]
+    assert len(actual) == 3
+    assert {post.units.number for group in actual for post in group} == {100, 200, 300}
+
+def test_make_consistent_by_date_with_exact_payment():
+    meta = {'rt-id': '1', 'invoice': 'rt:1/3', 'entity': 'OnePayment'}
+    entries = [testutil.Transaction(date=date, postings=[(
+        'Assets:Receivable:Accounts',
+        35 * (1 if date.day % 2 else -1),
+        meta,
+    )]) for date in itertools.islice(testutil.date_seq(), 3)]
+    actual = [group for _, group in
+              accrual.AccrualPostings.make_consistent(data.Posting.from_entries(entries))]
+    assert len(actual) == 2
+    assert actual[0].is_zero()
+    assert len(actual[1]) == 1
+    assert actual[1][0].meta.date.day == 3
+
+def test_make_consistent_by_date_with_underpayment():
+    meta = {'rt-id': '1', 'invoice': 'rt:1/4', 'entity': 'UnderPayment'}
+    entries = [testutil.Transaction(date=date, postings=[(
+        'Assets:Receivable:Accounts',
+        40 * (1 if date.day % 2 else -.5),
+        meta,
+    )]) for date in itertools.islice(testutil.date_seq(), 3)]
+    actual = [group for _, group in
+              accrual.AccrualPostings.make_consistent(data.Posting.from_entries(entries))]
+    assert len(actual) == 2
+    assert len(actual[0]) == 2
+    assert actual[0][0].units.number == 40
+    assert actual[0][1].units.number == -20
+    assert len(actual[1]) == 1
+    assert actual[1][0].meta.date.day == 3
+
+def test_make_consistent_by_date_with_overpayment():
+    meta = {'rt-id': '1', 'invoice': 'rt:1/5', 'entity': 'OverPayment'}
+    entries = [testutil.Transaction(date=date, postings=[(
+        'Assets:Receivable:Accounts',
+        50 * (1 if date.day % 2 else -1.5),
+        meta,
+    )]) for date in itertools.islice(testutil.date_seq(), 3)]
+    actual = [group for _, group in
+              accrual.AccrualPostings.make_consistent(data.Posting.from_entries(entries))]
+    assert len(actual) == 2
+    assert actual[0].is_zero()
+    assert len(actual[1]) == 2
+    assert actual[1][0].meta.date.day == 2
+    assert actual[1][0].units.number == -25
+    assert actual[1][1].meta.date.day == 3
+
+def test_make_consistent_by_date_with_late_payment():
+    meta = {'rt-id': '1', 'invoice': 'rt:1/6', 'entity': 'LatePayment'}
+    entries = [testutil.Transaction(date=date, postings=[(
+        'Assets:Receivable:Accounts',
+        60 * (-1 if date.day > 2 else 1),
+        meta,
+    )]) for date in itertools.islice(testutil.date_seq(), 3)]
+    actual = [group for _, group in
+              accrual.AccrualPostings.make_consistent(data.Posting.from_entries(entries))]
+    assert len(actual) == 2
+    assert len(actual[0]) == 2
+    assert actual[0][0].meta.date.day == 1
+    assert actual[0][1].meta.date.day == 3
+    assert len(actual[1]) == 1
+    assert actual[1][0].meta.date.day == 2
+
+def test_make_consistent_by_date_with_split_payments():
+    meta = {'rt-id': '1', 'invoice': 'rt:1/7', 'entity': 'SplitPayments'}
+    entries = [testutil.Transaction(date=date, postings=[(
+        'Assets:Receivable:Accounts', amount, meta,
+    )]) for date, amount in zip(testutil.date_seq(), [70, 80, -50, -100])]
+    actual = [group for _, group in
+              accrual.AccrualPostings.make_consistent(data.Posting.from_entries(entries))]
+    assert len(actual) == 2
+    assert [post.units.number for post in actual[0]] == [70, -50, -20]
+    assert [post.units.number for post in actual[1]] == [80, -80]
+
+@pytest.mark.parametrize('account,day', itertools.product(
+    ACCOUNTS,
+    [1, 10, 20, 30],
+))
+def test_make_consistent_with_three_one_split(account, day):
+    meta = {'rt-id': '1', 'invoice': 'rt:1/8', 'entity': '3Split'}
+    entries = [testutil.Transaction(date=datetime.date(2019, 5, dd), postings=[(
+        account, dd, meta,
+    )]) for dd in [5, 15, 25]]
+    meta['entity'] = '1Split'
+    entries.insert(day // 10, testutil.Transaction(
+        date=datetime.date(2019, 5, day),
+        postings=[(account, -45, meta)],
+    ))
+    postings = data.Posting.from_entries(entries)
+    actual = dict(accrual.AccrualPostings.make_consistent(iter(postings)))
+    if account.startswith('Assets:'):
+        group_count = 3
+        post_count = 2
+    else:
+        group_count = 1
+        post_count = 4
+    assert len(actual) == group_count
+    for related in actual.values():
+        assert len(related) == post_count
+        assert sum(post.units.number for post in related) == 0
+        assert all(post.meta['invoice'] == meta['invoice'] for post in related)
+        assert {post.meta['entity'] for post in related} == {'1Split', '3Split'}
+
+@pytest.mark.parametrize('account', ACCOUNTS)
+def test_make_consistent_with_three_two_split(account):
+    meta = {'rt-id': '1', 'invoice': 'rt:1/9'}
+    entries = [testutil.Transaction(date=datetime.date(2019, 5, day), postings=[(
+        account, day * (1 if day % 10 else -1.5), meta,
+    )]) for day in range(5, 30, 5)]
+    postings = data.Posting.from_entries(entries)
+    actual = dict(accrual.AccrualPostings.make_consistent(iter(postings)))
+    if account.startswith('Assets:'):
+        group_count = 3
+    else:
+        group_count = 2
+    assert len(actual) == group_count
+    for related in actual.values():
+        assert len(related) >= 2
+        assert sum(post.units.number for post in related) == 0
+        assert all(post.meta['invoice'] == meta['invoice'] for post in related)
+
 def check_output(output, expect_patterns):
     output.seek(0)
     testutil.check_lines_match(iter(output), expect_patterns)
@@ -465,7 +598,7 @@ def test_outgoing_report(accrual_postings, caplog):
         r'^\s*2010-06-10\s',
         fr'^\s+rt-id: "{rt_id_url}"$',
         r'^\s+Expenses:Services:Legal\s+220\.00 USD$',
-        r'^\s*2010-06-12\s',
+        r'^\s*2010-06-10\s',
         fr'^\s+contract: "{contract_url}"$',
         r'^\s+Expenses:FilingFees\s+60\.00 USD$',
     ])
@@ -536,11 +669,7 @@ def run_aging_report(postings, today=None):
         post for post in postings
         if post.account.is_under('Assets:Receivable', 'Liabilities:Payable')
     )
-    groups = {
+    groups = dict(accrual.AccrualPostings.make_consistent(postings))
-        key: group
-        for _, related in accrual.AccrualPostings.group_by_meta(postings, 'invoice')
-        for key, group in related.make_consistent()
-    }
     output = io.BytesIO()
     rt_wrapper = rtutil.RT(RTClient())
     report = accrual.AgingReport(rt_wrapper, output, today)
@@ -561,11 +690,6 @@ def test_aging_report(accrual_postings):
 def test_aging_report_date_cutoffs(accrual_postings, date, recv_end, pay_end):
     expect_recv = AGING_AR[:recv_end]
     expect_pay = AGING_AP[:pay_end]
-    if 10 <= date.day < 12:
-        # Take the 60 USD posting out of the invoice 510/6100 payable.
-        expect_pay[-1] = expect_pay[-1]._replace(
-            at_cost=testutil.Amount(expect_pay[-1].at_cost.number - 60),
-        )
     output = run_aging_report(accrual_postings, date)
     check_aging_ods(output, date, expect_recv, expect_pay)