[design·P2] A weekly review given part of the book withdraws its current-view figures — computing them over the canonical book is the other half of #630

[atomchung]

Status

Design. Follow-up to #630, which stopped the wrong number but did not deliver the right one. Needs owner scoping before implementation — the cheap versions are all wrong in a way that is hard to see.

What #630 left

#630's acceptance had two halves. The landed fix delivers one:

• A weekly file covering part of the book no longer publishes a whole-book claim measured over it. The current-view surfaces are withdrawn and the gap is disclosed (accounting_reconciliation), the durable metrics stay None instead of 1.0, engine_state.holdings becomes the canonical ledger book, and no oversize/concentration breach is recorded from the partial view.
• It does not compute concentration over the canonical book. A user who hands in only this period's trades now gets no sizing, diversification, ETF-structure, unrealized-P&L, what-if or per-ticker reading that week, where a user who re-supplies the whole history gets all of them.

Withdrawing is the fail-closed answer and it is a strict improvement on a fabricated 100%. It is not the answer the product wants: handing over the new trades is ordinary use, nothing in flows/weekly-review.md asks for a cumulative file, and the ingest path is built to append incrementals.

Why this is not a small change

The engine's whole current-book view is derived from the supplied rows, in trade_recap.main:

held = fifo_held(open_lots)                     # from the CSV's own trades
tickers = {r["ticker"] for r in rts} | set(held.keys())
px, yf_err = fetch_prices(tickers | bench, ...)  # the price universe
cur_map / currencies                             # the currency map, from rows

Everything downstream reads held/held_u/held_dx: dim_size, dim_diversify, instrument_policy.portfolio_analysis, dim_avgdown, overview_stats, what_if, ticker_diagnosis, the mkt_weights that pick the alpha/beta scope, data_integrity.unclassified_drivers, cash_position's held_mv, account_perf, and build_state's holdings/n_held/metrics/problem_events.

So measuring the wider book is not a matter of swapping a denominator:

1. The price universe is built from the CSV's tickers. Valuing the recorded book from a one-ticker week would put that ticker on market value and every other position on cost basis — a systematically mixed basis, not a data-availability accident.
2. The currency map is built from the CSV's rows. A ledger-only position has no entry, and [defect·P1] prepare aggregates a missing FX rate as 1.0 — #602's refusal never reached the review lane #612 is open on prepare aggregating a missing FX rate as 1.0. Weighting a mixed-currency book through that is the inverted-largest-holding failure [bug·data·M1-blocker] Split-adjusted shares can be valued against prices on a different split basis #583 exists to prevent.
3. Prices are resolved once, before the book is known ([architecture·implementation] One market-data resolver — minimum Yahoo calls, one fact bundle for prepare and consider #605's single-request contract), and the split map is resolved in the same request. Widening the universe means the recorded book has to be known before retrieval.

Options, none free

• A. Widen the engine's book to the recorded one. trade_recap reads TR_LEDGER already. The book would have to be derived before the price request, which reorders main() against [architecture·implementation] One market-data resolver — minimum Yahoo calls, one fact bundle for prepare and consider #605's ordering constraint, and it needs a split map before the review has resolved one.
• B. Feed the engine the full history. Reconstruct prior trades from the ledger and hand them to the engine alongside the new file, so the weekly path becomes the cumulative path. Correct by construction for every consumer above, and it is what a cumulative-file user already gets — but it changes what a weekly review measures: round_trips, dim_exit, dim_hold, realized P&L, payoff attribution and alpha/beta would span the whole history rather than the period. That may well be right (two round trips is not an exit-discipline diagnosis, and insufficient_data fires on almost every incremental week today), but it is a product decision, not a refactor. review._rows_from_ledger is explicitly documented as unsuitable for prepare.
• C. Read the canonical partition the review already computes. _virtual_review_basis builds a portfolio_basis.query_current_book over the post-import virtual ledger on every CSV prepare — currency-correct, split-aware, with its own coverage.scope — and then keeps it only as a change-detection receipt. Having the current-view dimensions read that instead of trade_recap.current_book_projection is the "collapse to one reader" move, and it settles [design·P2] A considered trade is measured on a different basis than the card, and the record itself can lag the real book #456's "two bases" question for the card. It also means deciding how dim_diversify's sector/AI classification maps onto the canonical book and what happens under bounded_valued_subset.

What the owner needs to settle

1. Should a weekly review's behavioral diagnosis (exits, holding time, payoff, alpha) stay scoped to the period, or span the recorded history like the current-view half? Option B only works if the answer is "span it".
2. Is the card's current-book denominator supposed to become portfolio_basis's (option C, closing [design·P2] A considered trade is measured on a different basis than the card, and the record itself can lag the real book #456 for this surface), or stay trade_recap's with a wider input (option A)?
3. Interaction with [defect·P1] prepare aggregates a missing FX rate as 1.0 — #602's refusal never reached the review lane #612: whatever computes the wider book inherits the missing-FX behavior. [defect·P1] prepare aggregates a missing FX rate as 1.0 — #602's refusal never reached the review lane #612 should probably land first.

Related

• [defect·P1] A weekly review given only this period's trades reports that week's single ticker as 100% of the portfolio #630 (the defect; the landed fix and its deliberate scope)
• [design·P2] A considered trade is measured on a different basis than the card, and the record itself can lag the real book #456 (two bases, one book — this is the same family on the card surface)
• [defect·P1] prepare aggregates a missing FX rate as 1.0 — #602's refusal never reached the review lane #612 (missing FX aggregated as 1.0 — a dependency of any wider aggregation)
• [architecture·implementation] One market-data resolver — minimum Yahoo calls, one fact bundle for prepare and consider #605 (one market-data request per review; constrains when the book can be known)

[atomchung]

Owner ruling, 2026-07-31: question 1 settled, and a second principle this issue had not asked about

Verbatim: 「只看這一期，然後沒有太多東西，就不用出太多東西，因為結合整個歷史紀錄來看，變化不大」

Two decisions, and the second one is the more consequential.

1. Behavioral diagnosis stays period-scoped

Exits, holding time, payoff and alpha keep measuring the period, not the recorded history. This settles this issue's question 1 and rules out option B — feeding the engine the reconstructed full history would make the weekly path the cumulative path and silently widen exactly these diagnostics. Off the table.

2. Output volume tracks what actually changed — this was not on the question list

The owner's reason for keeping it period-scoped is also a statement about what a weekly review should look like: a week barely moves the picture, so a week that produced little should produce little. A thin week's honest output is a short card, not a full-length one padded to look substantial.

That is a product definition this issue did not have, and it inverts how the current landed behavior reads. #630's fix withdraws the current-view half (sizing, diversification, ETF structure, unrealized P&L, what-if, per-ticker) and keeps the behavioral half. The ruling wants the opposite emphasis:

Half	What the ruling implies
Current book (structure, concentration, weights)	Must be right and present. It is a fact about the user's position today and does not depend on how much they traded this week.
Behavioral (exits, holding time, payoff, alpha)	As much as the period actually supports, and no more. A quiet week legitimately yields little here, and that is the correct output rather than a gap to apologize for.

So the target is not "restore everything #630 withdrew." It is: compute the current-book half over the recorded book, and let the behavioral half be as short as the week honestly is.

Note the connection to #507's framing — this is the same question in a different place: what does this interaction owe, given how much actually happened? A short card for a quiet week is proportionate, not degraded.

Also clarified: the withdrawal is a "did not fetch," not a "cannot compute"

Worth stating plainly because it changes how large this issue is. The recorded book is intact — #630's fix makes engine_state.holdings the canonical ledger book. What is missing is a current price for the positions this week's file never mentions, because the price universe is built from the supplied rows' tickers. The blocker is retrieval scope and its ordering against #605's single-request constraint, not an inability to know the book.

Still open

• Question 2 (whether the card's current-book denominator becomes portfolio_basis's — option C, closing [design·P2] A considered trade is measured on a different basis than the card, and the record itself can lag the real book #456 for this surface — or stays trade_recap's with a wider input — option A) is not settled by this ruling. Both remain viable; the ruling only removes B.
• Question 3 stands: [defect·P1] prepare aggregates a missing FX rate as 1.0 — #602's refusal never reached the review lane #612 lands first. Whatever computes the wider book inherits its missing-FX-as-1.0 behavior.

Privacy

No real ticker, amount, date, or account identifier.