Skip to content

fix(fx): every currency entering account aggregation is fetched, and a missing rate fails closed (closes #649) - #685

Merged
atomchung merged 1 commit into
mainfrom
claude/649-cash-flow-currency-universe
Jul 31, 2026
Merged

fix(fx): every currency entering account aggregation is fetched, and a missing rate fails closed (closes #649)#685
atomchung merged 1 commit into
mainfrom
claude/649-cash-flow-currency-universe

Conversation

@atomchung

Copy link
Copy Markdown
Owner

Closes #649. Implements the maintainer disposition (candidate remedy 2): fetch every currency that can enter a cross-currency aggregate, then fail closed only when a required rate is genuinely unavailable.

The defect

cur_map is filled by load(), which keeps BUY/SELL rows only. load_cash_flows reads a separate per-row Currency for DEPOSIT/WITHDRAWAL/DIVIDEND/INTEREST/FEE. #612's refusal and the mixed_ccy fetch decision both read cur_map, so a currency present only in a cash-flow row was invisible to both — no rate was ever requested, and nothing was ever reported missing.

Reproduced on main@e58fef9 with the issue's synthetic CSV (a USD stock book with one TWD interest row), --cash anchors in both currencies, an isolated --root and TR_LEDGER=/dev/null:

Field Before After (rate = 0.030849)
cash.balance 1,005,000 35,849
cash.weight 96.7% 51.3%
currency_meta.mixed false true
currency_meta.currencies ["USD"] ["TWD", "USD"]
price_provenance.fx "not_needed" "feed"
honesty_ledger no currency_mix entry currency_mix present

What changed

File Why
skills/fomo-kernel/engine/trade_recap.py aggregate_currencies is the one domain builder (held positions + cash-flow rows); missing_aggregate_fx_rates is the one predicate over whatever domain a lane built; held_currency_fx_gaps becomes their composition over the holdings domain. main() composes the universe before market_request, so acquisition and the gate read one set, and refuses upstream of the card/state/ledger. cash_position refuses two buckets it has no rate for. MissingHeldCurrencyRateMissingAggregateCurrencyRate, with a sentence that names the aggregate's span rather than claiming every currency in it is held.
skills/fomo-kernel/engine/perf.py _fx_getter returns the currencies it could not cover instead of silently emitting [1.0] * len(px_index), and account_perf withholds the whole account pillar behind a missing_fx gate. Its domain drops the unconditional union with USD, which had made a pure TWD book indistinguishable from a mixed one missing a rate.
docs/maintainer-guide.md The mirrored-surfaces row for this rule, rewritten around the builder/predicate split and the new readers.
docs/prd-ledger.md The multi-currency policy line still said "show original currencies rather than guessing" — the disclose-and-degrade posture #612 already replaced, which #649 widens.
skills/fomo-kernel/references/price-feed.md, schemas/price-feed.schema.json Both name the refusal an fx-only envelope repairs; "held-currency rate" is no longer the whole of it.
tests/test_engine_units.py Three unit tests: the builder (including the anchor and estimate-fallback boundaries), cash_position's refuse/convert pair, and its two compatibility halves.
tests/test_price_paths.py account_perf's refuse/convert pair and the pure-TWD compatibility half; the two existing mixed-currency fixtures now carry the fx_spot the production path always supplies.
tests/test_review_v2.py The end-to-end CLI walk on the issue's own fixture, plus a regression for single-currency books that have cash-flow rows.

Two decisions worth reviewing

Accepted cash anchors get no term of their own in the universe. The disposition names them, but cash_position buckets by cash-flow currency and _anchor_for pairs anchors into those buckets, so an anchor in a currency no flow created is dropped rather than converted. Including it would refuse a review over an amount nothing reads. Asserted, not reasoned: test_cash_position_single_bucket_and_flowless_anchor_keep_the_identity.

The universe is built from load_cash_flows(paths) without trade_rows. The estimate that argument unlocks needs split-adjusted amounts, and splits cannot be applied before the market request is composed. Omitting it cannot narrow the answer, because those rows take their currency from a trade row, which is already a cur_map value — pinned by an equality assertion in test_aggregate_currencies_spans_cash_flow_rows_not_only_holdings rather than left as a comment.

The four proofs

1. The fixture requests TWD FX and converts correctly when the rate resolves. test_a_currency_only_a_cash_flow_row_carries_is_fetched_and_refused_when_absent (second half). cash.balance is 35,849.00 (1,000,000 × 0.030849 + 5,000) against the 1,005,000 it reported before, currency_meta.fx == {"TWD": 0.030849}, mixed is True, price_provenance.fx == "feed". The request half is proven by the code, not the value: market_data._from_feed iterates request["currencies"], so a fx_unavailable gap for TWD (asserted in the first half) exists only if TWD entered the acquisition request.

2. The same fixture fails closed before account-level metrics when the rate does not resolve. Same test, first half — returncode == 2, one controlled CLI error, no traceback, and .pending/ledger.jsonl absent from the refused root:

this account's aggregate spans TWD, USD and no FX rate covers TWD, so its amounts
cannot be added into one denominator. Every weight, concentration figure, ranking,
cash balance and stored metric would be computed with 1 TWD treated as 1 USD.
Supply the rate through --prices (the `fx` block in references/price-feed.md),
then run prepare again. (market data reported: fx_unavailable)

3. Pure-USD and pure-TWD books are unchanged. Two receipts. test_a_book_whose_cash_rows_share_the_stock_currency_is_byte_stable drives both through the CLI and asserts mixed is False, fx is None, price_provenance.fx == "not_needed" and no currency_mix entry. Out of band, the same books plus #612's own mixed-holdings shape were run through origin/main's engine and this branch's and the persisted plan.json compared field by field, normalizing only the two fields that are functions of the environment (engine_version, from git-describe, and the mkdtemp path inside engine_meta):

pure_usd         identical=True
pure_twd         identical=True
mixed_holdings   identical=True

4. Mutation proof. Each mutation applied to a cp backup, __pycache__ cleared between runs, restored by checksum afterward.

(a1) cash_position's 1.0 fallback restored — delete the if gaps: raise, leaving fx.get(c, 1.0):

FAIL  test_cash_position_refuses_a_second_bucket_it_has_no_rate_for: two cash buckets and no rate must refuse, never sum at 1.0
106 passed, 1 failed, 0 skipped

(a2) _fx_getter's 1.0 fallback restored — delete the missing.append(c) guard:

FAIL  test_acct_refuses_a_cash_only_currency_it_has_no_rate_for: {'acct_twr': 300.49999999999983,
'hold_twr': 0.49999999999999956, 'cash_drag': 299.99999999999983, 'drag_dollar_approx': 125000.0,
'avg_cash_weight': 0.8298, ... 'basis': {'cash_source': 'anchored', 'unanchored': [],
'fx_approx': False, ...}, 'gate': None, ...}
61 passed, 1 failed

The failure output is the defect itself: 300,000 TWD read as 300,000 USD gives acct_twr 30,050% and cash_drag 30,000%, with fx_approx: false and unanchored: [] still claiming nothing is wrong — the same shape the issue measured at 2945% / 2903%.

(b) held-currency-only universe restoredaggregate_currencies(cur_map, load_cash_flows(paths))aggregate_currencies(cur_map):

FAIL  test_a_currency_only_a_cash_flow_row_carries_is_fetched_and_refused_when_absent
AssertionError("engine failed (1): ❌ this account's aggregate spans TWD, USD and no FX rate covers
TWD, USD, so its amounts cannot be added into one denominator. ...")
260 passed, 1 failed

It reddens on the fx_unavailable assertion specifically, which is the intended kill: with the narrow universe the book is still refused, but by cash_position rather than by acquisition, because TWD was never requested and so was never reported missing. The single-currency regression stays green under this mutation, as a compatibility test should.

Suite

PASS: all 49 suites passed.

Run from the repository root with pandas 3.0.3, yfinance 1.3.0 and rich importable, so nothing skipped silently.

Adjacent findings, deliberately not folded in

  • account_perf can KeyError on a residual currency. fx_at(r["currency"], n - 1) at the cash_residual gate indexes cols, which is built from the value-carrying currencies. A ledger cash snapshot declaring a currency with no flow and no holding produces a residual outside that set. Pre-existing, unchanged here, and unreachable from main() today.
  • currency_meta.alpha_beta_note is written and never read — a [design·P1] Two questions collect an answer nothing reads: initial_thesis and exit_consistency (proved by a byte-identical card) #429-class dead field. It also carries a hard-coded Traditional Chinese sentence onto an English card whenever a book is mixed, which this change makes reachable for one more book shape.
  • A cash anchor in a currency with no cash flow is silently dropped (_anchor_for). The user declares a balance and the engine ignores it without a word. [feat·design] Pre-review data-sanity check: confirm cash anchor (and flag known-incomplete data) before the motive questions #357-adjacent; the correct fix is a disclosure or a bucket, not a refusal.
  • perf.account_perf's missing_fx gate has no account_gate copy key. It degrades to the generic annualized sentence by the renderer's documented fallback. Left uncovered on purpose: the gate is defence in depth and cannot reach a card while main() refuses upstream, so a copy key would be a string with no producer.

Note for the merge queue

PR #684 touches docs/maintainer-guide.md and tests/test_review_v2.py as well. Different row and different region — text conflicts are possible, semantic overlap is not.

🤖 Generated with Claude Code

…a missing rate fails closed (closes #649)

`cur_map` is filled by `load()`, which keeps BUY/SELL rows only, while
`load_cash_flows` reads a separate per-row `Currency`. #612's refusal and
the `mixed_ccy` fetch decision both read `cur_map`, so a currency present
only in a cash-flow row was invisible to both: on a USD stock book with
one TWD interest row no rate was ever requested, and `cash_position`
summed raw TWD into a USD total at 1.0 — a balance 28x its real size,
reported under `currency_meta.mixed: false`, `fx: "not_needed"`,
`basis.fx_approx: false`, `unanchored: []` and `cash_source: "anchored"`.

Remedy 2 from the maintainer disposition: fetch every currency that can
enter the aggregate, then fail closed only when acquisition genuinely
fails.

- `aggregate_currencies` is the one domain builder (held positions plus
  cash-flow rows) and `missing_aggregate_fx_rates` the one predicate over
  whatever domain a lane built. `held_currency_fx_gaps` becomes their
  composition over the holdings domain instead of a second walk of
  `cur_map`, so `usd_view` and `consequence` are unchanged.
- `main()` composes the universe before `market_request`, so the same set
  drives acquisition and the gate, and refuses upstream of the card, the
  state write and the ledger ingest when a required rate did not arrive.
- The identity fallback is removed wherever currencies differ rather than
  only guarded upstream: `cash_position` refuses two buckets it has no
  rate for, and `account_perf` gates `missing_fx` instead of letting
  `_fx_getter` return `[1.0] * len(px_index)`. That getter's domain drops
  the unconditional union with USD, which had made a pure TWD book
  indistinguishable from a mixed one missing a rate.
- Accepted cash anchors need no term of their own: `cash_position` buckets
  by cash-flow currency, so an anchor in a currency no flow created is
  dropped rather than converted, and refusing over it would block a book
  on an amount nothing reads.
- `MissingHeldCurrencyRate` is now `MissingAggregateCurrencyRate`; its
  sentence names the aggregate's span rather than claiming every currency
  in it is held.

Single-currency books are byte-stable: a pure USD book, a pure TWD book
and #612's own mixed-holdings shape each produce a plan identical to
`origin/main`'s, compared field by field.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant