Replace tag-based voiding with reversing entries (single mutation surface) #24

Open
opened 2026-06-06 11:01:13 +00:00 by padreug · 0 comments
Owner

Context

Libra's current correction model edits the source file in-place: reject_pending_entry (views_api.py:2860) calls Fava's PUT /api/source to append a #voided tag to the original transaction's line. Balance computations then filter voided entries out via if "voided" in entry.get("tags", []): continue at four sites.

Two problems with this:

  1. The void semantics live in Libra's app layer, not in the journal. A #voided-tagged transaction is, to any consumer that doesn't know Libra's tag convention (Fava's own balance computation, bean-query, an auditor's script, Tackler), a live transaction. The journal is no longer self-describing. Beancount has no native #voided flag/tag — confirmed by inspecting beancount/core/flags.py (only *, !, and system-generated P/S/T/C/M exist).
  2. In-place file mutation defeats append-only invariants needed for the audit follow-ups (#25 git-backed journal, #26 audit triplet). Under in-place editing, a hash over the journal at HEAD is hashing a file whose existing transactions can grow new tags, so the txn-set-checksum loses its evidentiary weight.

The canonical Beancount approach to corrections is the same as traditional double-entry bookkeeping: post a reversing transaction. Original stands, reversal cancels it, both consumers and humans get the right answer with zero special knowledge.

Scope

This is one coherent change that touches several places. Don't split:

Construction

  • New FavaClient.void_entry(entry_hash, sha256sum) method that:
    • Loads the original entry's postings
    • Constructs a reversing Transaction with sign-flipped amounts
    • Dates the reversal action-date (today), not the original's date — see "Date policy" below
    • Carries a reverses: <original_entry_hash> meta key for directional identification
    • Carries the original's libra-<entry_id> link plus a distinct reversal tag for retrieval/grouping
    • Must NOT carry expense-entry or income-entry tags — these are the gate for lifetime-totals (models.py:93, fava_client.py:get_user_lifetime_totals_bql, get_user_contributions_bql). One helper enforces this; one test asserts it.
    • Posts via add_entry so the existing _write_lock and idempotency machinery apply naturally.

Deletion (close the capability surface)

  • Delete FavaClient.update_entry_source (fava_client.py:1392) — dead code, no call sites.
  • Delete FavaClient.delete_entry (fava_client.py:1443) — dead code, no call sites.
  • Delete the direct /api/source httpx call in reject_pending_entry (views_api.py:2913-2954) — replace with await fava.void_entry(...).
  • Closes #23 (lock-gap bug) as a side-effect: void path now goes through _write_lock.

Voided-tag filter removal

The voided-tag exclusion is the old model's mechanism. Under reversing entries, both legs are supposed to be summed (they net to zero); leaving the exclusion in place would suppress the original while the reversal still subtracts, producing a phantom credit. Remove at all four sites:

  • fava_client.py:325
  • fava_client.py:471
  • views_api.py:489-490
  • views_api.py:782-783

Backfill

Any pre-existing entries voided under the current model carry #voided on the original. After the exclusion filter is removed, those will start counting in balances with no reversal to cancel them. Options:

  • If zero such entries exist in any deployed instance: confirm and delete the filter outright.
  • Otherwise: one-time migration script that finds #voided originals, posts a synthetic reversal for each (action-dated to migration date, linked back to the original), then strips the #voided tag from the originals.

Check this before writing the refactor.

Date policy: action-date, not original-date

Reversals are dated to the action-date (today, when the void happens), not back-dated to the original. Rationale:

  • Consistent with the codebase's existing stance. models.py:93 carries the comment "original entries only; not net of reconciliation" — the lifetime-totals field deliberately treats originally-entered numbers as a stable historical fact. Original-dated reversals would silently change those numbers; action-dated ones preserve them.
  • Preserves option value for period-close. Libra has no closed-period concept today, but balance_assertions is the natural foundation for a soft-close later (a passed assertion implicitly says "the books at date Z are these"). Original-dated reversals would silently invalidate already-passed assertions; action-dated ones touch only the current period.
  • Cheaper to back out if a future decision wants original-dating per-collective config.

Tradeoff to document

Action-dating creates a cross-period question: a March transaction voided in June leaves the original in March and the reversal in June. As-of-date balance summation handles this correctly (tasks.py reconciliation watchdog), but per-period activity reporting (which doesn't exist today) would need to decide whether the reversal counts in its origin period or its action period.

Add a docstring next to models.py:93 documenting both the action-date decision and this consequence, so future-you debugging "why doesn't June's activity net to zero" finds the tradeoff written down.

Dependencies

  • Supersedes #23 (lock-gap bug) — closes it as a side-effect.
  • Prerequisite for #25 (git-backed journal): commit-per-write becomes a one-line decorator around add_entry once the mutation surface is consolidated.
  • Prerequisite for #26 (audit triplet): txn-set-checksum becomes meaningful only when postings are immutable by construction.

Out of scope

UI changes to surface "this entry was voided by reversal X" pairings — file a follow-up when the data model lands. The data is sufficient (link + reverses: meta) for the UI to do the join later.

## Context Libra's current correction model edits the source file in-place: `reject_pending_entry` (`views_api.py:2860`) calls Fava's `PUT /api/source` to append a `#voided` tag to the original transaction's line. Balance computations then filter voided entries out via `if "voided" in entry.get("tags", []): continue` at four sites. Two problems with this: 1. **The void semantics live in Libra's app layer, not in the journal.** A `#voided`-tagged transaction is, to any consumer that doesn't know Libra's tag convention (Fava's own balance computation, `bean-query`, an auditor's script, Tackler), a *live* transaction. The journal is no longer self-describing. Beancount has no native `#voided` flag/tag — confirmed by inspecting `beancount/core/flags.py` (only `*`, `!`, and system-generated `P/S/T/C/M` exist). 2. **In-place file mutation defeats append-only invariants** needed for the audit follow-ups (#25 git-backed journal, #26 audit triplet). Under in-place editing, a hash over the journal at HEAD is hashing a file whose existing transactions can grow new tags, so the txn-set-checksum loses its evidentiary weight. The canonical Beancount approach to corrections is the same as traditional double-entry bookkeeping: post a reversing transaction. Original stands, reversal cancels it, both consumers and humans get the right answer with zero special knowledge. ## Scope This is one coherent change that touches several places. Don't split: ### Construction - New `FavaClient.void_entry(entry_hash, sha256sum)` method that: - Loads the original entry's postings - Constructs a reversing `Transaction` with sign-flipped amounts - Dates the reversal **action-date** (today), not the original's date — see "Date policy" below - Carries a `reverses: <original_entry_hash>` meta key for directional identification - Carries the original's `libra-<entry_id>` link plus a distinct `reversal` tag for retrieval/grouping - **Must NOT carry `expense-entry` or `income-entry` tags** — these are the gate for lifetime-totals (`models.py:93`, `fava_client.py:get_user_lifetime_totals_bql`, `get_user_contributions_bql`). One helper enforces this; one test asserts it. - Posts via `add_entry` so the existing `_write_lock` and idempotency machinery apply naturally. ### Deletion (close the capability surface) - Delete `FavaClient.update_entry_source` (`fava_client.py:1392`) — dead code, no call sites. - Delete `FavaClient.delete_entry` (`fava_client.py:1443`) — dead code, no call sites. - Delete the direct `/api/source` `httpx` call in `reject_pending_entry` (`views_api.py:2913-2954`) — replace with `await fava.void_entry(...)`. - Closes #23 (lock-gap bug) as a side-effect: void path now goes through `_write_lock`. ### Voided-tag filter removal The `voided`-tag exclusion is the *old model's* mechanism. Under reversing entries, both legs are supposed to be summed (they net to zero); leaving the exclusion in place would suppress the original while the reversal still subtracts, producing a phantom credit. Remove at all four sites: - `fava_client.py:325` - `fava_client.py:471` - `views_api.py:489-490` - `views_api.py:782-783` ### Backfill Any pre-existing entries voided under the current model carry `#voided` on the original. After the exclusion filter is removed, those will start counting in balances with no reversal to cancel them. Options: - If zero such entries exist in any deployed instance: confirm and delete the filter outright. - Otherwise: one-time migration script that finds `#voided` originals, posts a synthetic reversal for each (action-dated to migration date, linked back to the original), then strips the `#voided` tag from the originals. Check this **before** writing the refactor. ## Date policy: action-date, not original-date Reversals are dated to the action-date (today, when the void happens), not back-dated to the original. Rationale: - **Consistent with the codebase's existing stance.** `models.py:93` carries the comment "original entries only; not net of reconciliation" — the lifetime-totals field deliberately treats originally-entered numbers as a stable historical fact. Original-dated reversals would silently change those numbers; action-dated ones preserve them. - **Preserves option value for period-close.** Libra has no closed-period concept today, but `balance_assertions` is the natural foundation for a soft-close later (a passed assertion implicitly says "the books at date Z are these"). Original-dated reversals would silently invalidate already-passed assertions; action-dated ones touch only the current period. - **Cheaper to back out** if a future decision wants original-dating per-collective config. ### Tradeoff to document Action-dating creates a cross-period question: a March transaction voided in June leaves the original in March and the reversal in June. As-of-date balance summation handles this correctly (`tasks.py` reconciliation watchdog), but per-period *activity* reporting (which doesn't exist today) would need to decide whether the reversal counts in its origin period or its action period. Add a docstring next to `models.py:93` documenting both the action-date decision and this consequence, so future-you debugging "why doesn't June's activity net to zero" finds the tradeoff written down. ## Dependencies - Supersedes #23 (lock-gap bug) — closes it as a side-effect. - Prerequisite for #25 (git-backed journal): commit-per-write becomes a one-line decorator around `add_entry` once the mutation surface is consolidated. - Prerequisite for #26 (audit triplet): txn-set-checksum becomes meaningful only when postings are immutable by construction. ## Out of scope UI changes to surface "this entry was voided by reversal X" pairings — file a follow-up when the data model lands. The data is sufficient (link + `reverses:` meta) for the UI to do the join later.
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
aiolabs/libra#24
No description provided.