fix(consider): a per-holding integrity warning excludes and discloses that holding instead of disabling the whole account (closes #673)

[atomchung]

Closes #673. Third leaf of QA repair Front A, after the two cross-currency truth defects, and implements the maintainer disposition of 2026-07-31 exactly: per-holding integrity degrades locally, whole-book corruption still refuses.

The defect

consequence.py gated the entire basis on the integrity list being non-empty:

if basis.get("integrity"):
    raise ConsequenceError("canonical PortfolioBasis has claim-affecting integrity warnings")

ledger.py records one oversell row per sell with no matching prior buy — ordinary for an export that starts mid-account — so a single such sell disabled consider for the whole account, permanently: the rows derive from history that has already happened, so no future trade clears them. The review lane discloses the identical condition and delivers its card.

Reproduced on origin/main@dc654a1 with a five-line synthetic mid-history CSV (prepare → consider):

{"error": "canonical PortfolioBasis has claim-affecting integrity warnings", "status": "error"}

The gate was one line, and the docstring immediately above it already argued against it: basis["integrity"] is the same shape of whole-book summary over a per-holding fact that cost_basis was, and #515 had already removed that one.

What changed

consequence.integrity_exclusions(basis) is the single scoper: it maps each warning to the holding it names and to an exclusion reason (integrity_oversell), and refuses whole-book for anything it cannot scope. rows_from_portfolio_basis then excludes that holding exactly the way #515 excludes an unvaluable one — recorded in excluded_holdings, disclosed as partial_book, and never silent.

• The denominator shrinks. review._canonical_consider_before re-divides sizing_projection's own per-holding values over the surviving holdings. Nothing is re-valued here — no price, cost, share count or FX rate is read — and math.fsum is value_partition's own exactly-rounded summation, so with nothing integrity-excluded the sum is bit-for-bit the projection's denominator and every weight is the pre-[defect·P1] One unmatched sell anywhere in the history disables consider for the whole account — the review lane discloses the same condition and continues #673 weight.
• The projection-agreement gate is checked against the valuation half only. An integrity exclusion is not a claim about valuation — the projection can value the holding fine; the share count it multiplies is what is in doubt — so this is the one exclusion the two readers are expected to differ on. About valuation they still may not.
• A premise on an excluded holding refuses by name, before validate_premise can call it "not currently held" — that is a claim about the book, and the wrong claim, since the recorded zero is what the warning puts in doubt.
• A ticker the book records no position in is still recorded when a warning names it. It removes nothing from the denominator, but it is what the premise gate reads, and "you don't hold X" is not a fact this answer may rest on.
• The review lane's honesty statement and the canonical integrity record are untouched. This is a route-specific use policy over the same facts; portfolio_basis still stores every row.

EXCLUSION_REASONS is the single vocabulary, mirrored by trade-evaluation.schema.json and evaluation-challenge.schema.json and locked by a drift test — the same discipline DISCLOSURES already has.

Proof

1. A mid-history export with one unmatched sell answers an unaffected premise, with the exclusion disclosed. End to end through prepare on a CSV then consider (the real user route from the issue):

status: considered
disclosures: ['cash_unreliable', 'unclassified_book', 'partial_book']
excluded_holdings: [{'reason': 'integrity_oversell', 'ticker': 'ORPH'}]
before held: ['AAPL', 'MSFT', 'NVDA']

The exclusion reaches the answer contract, not only the payload — the emitted challenge owes it:

{"topic": "excluded_holding", "anchor": "consequence.excluded_holdings.0.ticker",
 "value": "ORPH", "detail": {"reason": "integrity_oversell"}}

2. A premise on the excluded holding refuses by name, buy and sell alike:

{"error": "this book's record for ORPH carries an integrity warning (integrity_oversell), so its
recorded shares and cost are not derivable from the supplied history and no consequence for a trade
in ORPH can be computed from them. The rest of the book is unaffected and can still be asked about;
supplying the transactions that ORPH's history is missing is what makes this question answerable.",
 "status": "error"}

3. Whole-book conditions stay fatal. Three shapes, all pinned:

Condition	Refusal
A bad_* warning that does name a ticker (bad_avg_cost)	no trustworthy canonical current book in … — portfolio_basis._bad_integrity still drops the whole basis; naming a holding does not make unknown state a bounded subset
A warning that cannot be scoped (ledger.py's own bad_trade_event shape: issue + detail, no ticker)	canonical PortfolioBasis has an integrity warning that cannot be scoped to one holding: bad_trade_event
Every holding integrity-excluded	canonical PortfolioBasis has no usable holding: AAA (integrity_oversell), BBB (integrity_oversell)

The last message replaces no holding that can be valued: …, which is false for a book emptied by integrity warnings and would send the user after costs already on record. One existing assertion updated with it.

4. The denominator provably shrinks. The regression fixture holds five positions of 1000 plus a damaged ORPH of 2000. Over the whole 6000 book AAA is 16.7%; over the 4000 usable book it is exactly 25%, and the test asserts the number:

assert abs(result["before"]["weights"][ticker] - 0.25) < 1e-9
assert abs(result["after"]["weights"]["AAA"] - 0.4) < 1e-9   # 2000 of the 5000 it becomes

Rule collisions inherit it: with a 20% cap, AAA is already_over on the usable book and would read would_breach on the whole one — so the collision state, not just the partial_book marker, pins which denominator judged the rule.

5. Mutation proof. cp backup/restore, __pycache__ cleared, import-checked before each run so every red is semantic rather than a crash. All five reddened tests/test_consider.py:

• A — restore the whole-list gate (the required one), 118/122:

  FAIL test_an_unmatched_sell_excludes_that_holding_instead_of_disabling_the_account: expected success, got 2
  FAIL test_a_premise_on_the_integrity_excluded_holding_refuses_by_name: wanted 'ORPH' in error, got 'canonical PortfolioBasis has claim-affecting integrity warnings'
  FAIL test_a_book_whose_every_holding_is_integrity_excluded_still_refuses: … got 'canonical PortfolioBasis has claim-affecting integrity warnings'
  FAIL test_a_rule_is_judged_against_the_book_the_integrity_exclusion_left: expected success, got 2
  

• B2 — keep the bounded key set but read the whole-book weight back (the number mutation; the structural set-equality guard still passes, so this is what proves the figure is load-bearing), 120/122:

  FAIL test_an_unmatched_sell_excludes_that_holding_instead_of_disabling_the_account: AAA must be sized against the 4000 that is usable, not the 6000 whole book: got 0.16666666666666666
  FAIL test_a_rule_is_judged_against_the_book_the_integrity_exclusion_left: the cap must be judged on the usable book's own weights: [{… 'state': 'would_breach' …}]
  

• C — drop the premise gate, 121/122: FAIL test_a_premise_on_the_integrity_excluded_holding_refuses_by_name: expected failure, got exit 0
• D — exclude the holding silently (no record, so no disclosure), 118/122: … got 'canonical PortfolioBasis has no usable holding: ' and 'canonical PortfolioBasis holdings disagree with consider adapter'
• E — absorb an unscopable warning instead of refusing, 121/122: FAIL test_an_integrity_warning_this_route_cannot_scope_refuses_the_whole_book: an integrity warning this route has no reason for (bad_avg_cost) must not yield a usable book

E is worth naming: its first version stayed green. The test then only covered the ticker-less half, so the issue-kind half of the gate was unpinned. Widened to both shapes, using ledger.derive_holdings' own real row shapes, before it reddened.

6. Offline suite. python3 tests/run_all.py → PASS: all 49 suites passed. (tests/test_consider.py 122/122).

Scope

One leaf. #674 (what the user receives when computation genuinely cannot proceed) is the next one and is not implemented here; the fatal path's error payloads name their condition and their repair, so they do not get in its way. #528 (value-only positions) is untouched — a holding excluded for a missing cost still does not refuse a premise about it, deliberately.

🤖 Generated with Claude Code