Skip to content

docs(tally): Unit A review follow-ups — empty-partition corroboration, breaker feed, telemetry, currency #107

Description

@lamemustafa

Four findings from the PR #106 review that are real and validated but deliberately not fixed in that PR, each with the reason.

1. Corroborate every empty date partition (correctness — highest value here)

assemble_scan guards false-empty whole-book only: if high_water.get() > 0 && vouchers.is_empty(). If Tally returns a false-empty for one partition while others carry vouchers, vouchers is non-empty overall, the guard never fires, and that period's bills are silently omitted from the totals.

Ruling 6 made the wider-date witness optional. That was under-cautious: §2.8 documents real false-empty routes, so an uncorroborated empty partition is a silent-wrong-number path.

Why not fixed in #106: the witness API exists (verify_empty_date_window_with_wider_pair) but a strictly wider window around a ≤31-day partition exceeds the 31-day cap the sealed request enforces. Making corroboration mandatory therefore needs a new request shape, the sealed-boundary review that implies, and a template-hash repin — not just runtime wiring.

2. Segment transport failures never reach the circuit breaker

In fetch_outstandings, a failed fetch_outstandings_segment_pair is converted to Ok(partial_result(...)). The enclosing execute_cancellable therefore records a successful VoucherExport and resets endpoint health, so timeouts and connection failures never feed the breaker.

Why not fixed in #106: the obvious fix (propagate Err) removes documented typed-Partial UX such as tally_segment_deadline_restart_recommended, which the screen renders and the docs reference. Needs a deliberate split between transport failures (feed the breaker) and verification failures (stay Partial).

3. Scan telemetry reports the wrong payload size

The closing extent pair is the last caller of the overwrite-only record_observed_body_bytes, so every outstandings scan reports the small company-extent response instead of the voucher payloads it actually consumed. This systematically under-reports the largest reads in the repo.

4. Displayed totals hard-code

OutstandingsScreen prepends unconditionally. Neither the company-extent request nor OutstandingsReport carries currency evidence, so a non-INR company's correct amounts are shown under the wrong symbol.

Why not fixed in #106: needs base-currency fetched into the extent (new fetch field, parser, template-hash repin) or an explicit INR assertion. Note the related known limitation that the currency formal name is unreachable by export.


Context: #106 fixed sixteen other review findings including optional-voucher exclusion, ledger-opening detection, per-segment company binding, as-of clamping, and an injective XML sanitiser. See docs/tally/UNIT_A_RULING_9.md.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions