diff --git a/docs/maintainer-guide.md b/docs/maintainer-guide.md index 2af602bf..490f1dbf 100644 --- a/docs/maintainer-guide.md +++ b/docs/maintainer-guide.md @@ -141,7 +141,7 @@ When multiple sessions are active: | What a `consider` answer owes the user (#479 Wave B cut 2) | `engine/evaluation_challenge.py`'s `build_challenge` is the single statement — `must_state`, `quote_verbatim`, `unchecked`, `case_required`, `required_coverage` — emitted by `review.py` `cmd_consider` beside the row. Its readers: `schemas/evaluation-challenge.schema.json` ↔ `references/trade-consequence.md` "What the answer owes" ↔ `SKILL.md` rule 3 / `AGENTS.md` boundary 2 ↔ `tests/test_evaluation_challenge.py` + `tests/test_consider.py` section M. Wave B's first cut made a *fabricated* case unstorable and left the user's side untouched; this is the other half, and it is this guide's own "Honesty decisions belong in code" applied to the surface that never had a `build_honesty_ledger()`. Four rules. **The floor and the gate are one list, not two**: `required_coverage` is `answer_provenance.required_coverage`, the same function `validate_agent_case` calls internally, so what the agent is *told* it owes and what a `--agent-case` is *refused* for dropping cannot drift — deriving it in both places is the hand-mirrored surface this file forbids, and `test_dropping_any_single_entry_the_challenge_named_is_refused` fails in both directions (an obligation nothing enforces, and an obligation nobody was told about). Widening it is now the only way to add an enforced obligation; the `would_breach`/`already_over` arm is what this cut added, and before it a rule the trade breaks could vanish from a case with the whole suite green. The `price_basis` arm (#618) is the second widening and it carried a latent defect out with it: **one claim pays one debt, the narrowest it matches**. `basis.price_observations` is the first required path ever to sit *under* another (`basis`), and plain prefix matching let a single price-day citation discharge the staleness obligation too — a case that never mentioned staleness accepted, whole suite green, because until then no two required paths nested. `answer_provenance._paid_path` is the repair: the longest matching path wins and the broader obligation stays open. Adding any nested required path is now safe; adding one without that rule was not. **`must_state` is deliberately the larger list**: an `unjudged`/`unmapped` collision must be *named* — an unevaluated rule presented as no issue tells the user something the engine never checked — but forcing a claim for each would make a book with eight behavioral rules need eight claims saying nothing was measured, which is answer-padding to satisfy a checker. **An anchor is never offered unless it resolves**, through `answer_provenance.resolve_anchor` rather than a second walk, so the surface that says how to cite a fact and the gate that judges the citation agree by construction; a real ticker containing a dot (`2330.TW`) cannot be addressed by a dotted path, and such an entry keeps its value and loses only its `anchor` — dropping the fact instead would have been the wrong repair. **Emitted, never stored and never in the id seed**: it is a pure function of fields the row already freezes, so storing it would be a derived duplicate able to disagree with its own inputs, and no reader needs the historical version (#429 in the other direction). Boundary with rule 8: this is the "which facts it owes" clause made computable, and says nothing about length — the delivery half is instruction-only, the same footing `docs/development-guide.md` §4 admits for the recommendation ban, and is observed by an owner-live receipt on `tools/ux_receipt.py`'s `consider` route (#544 Slice B; the receipt-route row above holds its contract), which machine-checks what containment and digits can decide and leaves the rest to the owner's `comprehension` verdict. | | Snapshot position provenance (`carried`, `since`/`since_basis`/`cycle_seq` — #485 Slice C, #531, #539/#536) | `ledger.SNAPSHOT_POSITION_KEYS` and `ledger.ENGINE_ASSIGNED_POSITION_KEYS`/`SINCE_BASES` are the single declaration — `snapshot_adapter.POSITION_KEYS` reads them rather than restating them, which is exactly the hand-mirrored whitelist that let `carried` ship broken with a green suite. Three rules no surface may weaken. **`since` and `since_basis` are one fact, validated as a pair** in `snapshot_adapter._normalize_provenance`: a date that could travel without its stamp is a reconstruction every renderer is free to print as an exact day, so the pairing — not renderer discipline — is what makes rule 2 of #531's ruling true. **A caller never supplies them**: `normalize_envelope`/`normalize_book` refuse both by default and `_load` (the file path, i.e. everything an agent can write) has no way to opt in; `book_refresh.carry_recorded_starts` is the single `allow_engine_provenance=True` call site, counted mechanically by `tests/test_book_refresh.py::test_only_the_refresh_lane_may_write_engine_assigned_provenance`. **`ledger.derive_holdings` is the only reader**, via `_anchored_cycle_start`, and it degrades to the anchor date with a named `integrity` issue rather than trusting a hand-edited row. **Both adoption lanes reach that one primitive** (#539/#536) — `build_adoption` for the refresh lane, `review._carried_declaration` for the review lane, which calls it before `snapshot_adapter.prepare` so the plan's cycle ids and the ledger anchor come from one stamped envelope rather than two derivations. The count test is what keeps that true, so read its assertion as "one implementation, both lanes through it", not "one lane"; a second literal anywhere is a second place a cycle start could be authored. **`since_basis` is an evidence vocabulary, not a transport marker** — four values, carried unchanged, never collapsed (owner ruling, 2026-07-31). The two new ones exist because the stamp was never the fact worth carrying: the appearance question is only asked for a position that appears *after* the book exists, so every position in a user's first declaration is unstampable and reminted on every adoption until the *start itself* was carried. A single "came from the record" value was refused because it would state how the date travelled and destroy why it is believed, and the loss is unrecoverable: after an adoption `origin` describes the new snapshot writer, not the original evidence for the start. So `_carried_start` decides the date and the basis together and **prefers the recorded stamp over recomputation** — a position the record still traces to its anchor keeps that anchor's own basis whatever it says, and only one the anchor does not describe is labelled fresh (`trade_event` when the ledger watched the cycle open, `snapshot_anchor` when a declaration first put it on the books). That branch order is the whole protection: an adopted trade-proven position reads `origin == "snapshot"` like every other row, so a basis recomputed from origin would demote an exact start to a lower bound on the second declaration, silently and permanently — `tests/test_book_refresh.py::test_a_start_the_ledger_can_prove_is_not_overwritten_by_a_declaration` adopts twice for exactly that reason. Nothing is written when the carried start is later than the declaration's `as_of` (the declaration predates the record, and that refusal belongs to the lane that owns it), nor when a `snapshot_anchor` start equals it (restating the default would rewrite the anchor's content address and break idempotent replay); the equality guard is deliberately not extended to the other bases, for which the stamp carries something the default does not. No surface reads the distinction yet — #532 owns visibility — so what this contract owes is that it survives, which is what the repeated-adoption assertions gate. **`cycle_seq` travels with the start or not at all**, because `cycle_id` is `ticker#since#seq` and the start alone is not the identity: two cycles opened on one day — a position flattened and bought back in one session — differ in the sequence and nothing else, so carrying the date without it hands the live position the *sold* cycle's id, along with that cycle's thesis, standing conditions and closed status. That is strictly worse than the reminting being fixed, and #539 widens the door to it (before, the live position landed on the declaration's own date and collided only by coincidence), which is why the two are one change. `_normalize_provenance` refuses a sequence with no dated start and refuses one on `unknown` (a two-segment cycle has none to state); `_anchor` emits it only past the first, for the same content-addressing reason `carried` is conditional — an unconditional key would rewrite every existing `snapshot_id`. `ledger.cycle_sequence` is the only inverse of the composition in `derive_holdings` and lives beside it; `_anchored_cycle_seq` degrades to 1 with a named `integrity` issue exactly as `_anchored_cycle_start` degrades to the anchor date. It stays out of `portfolio_basis._normalized_anchor` by construction (that function builds a fresh dict), which is also the right semantics: a sequence is provenance, so two declarations of one book keep one `state_version`. Its readers: `flows/book-refresh.md` step 3 ↔ `references/data-contract.md` ↔ `docs/prd-ledger.md` ↔ `schemas/book-refresh.schema.json`'s lane description, and the value set itself is asserted once, in `tests/test_book_refresh.py::test_the_classification_enum_matches_the_engine_constant`. Neither key enters `portfolio_basis._normalized_anchor` (provenance stays out of the book's identity) or the `holdings` dict (`portfolio_basis`'s `holding_keys` is an exact-key gate and `since` must stay a real date there) — `cycle_id` is what carries an unknown start, as `ticker#unknown`. Kinds, options, and answer fields are locked across `book_refresh.CONFIRMATION_KINDS`/`CLASSIFICATIONS_BY_KIND`/`HELD_MONTHS_MAX` ↔ `schemas/book-refresh.schema.json` ↔ `flows/book-refresh.md` step 2 ↔ `references/data-contract.md` ↔ `docs/prd-ledger.md` by two drift tests in `tests/test_book_refresh.py`. Widening what refresh raises automatically widens `review._refuse_if_the_book_must_catch_up`'s refusal (#530), which calls `plan_refresh` rather than restating its criterion. | | Freeform informational answer shape (#543, second cut 2026-07-29) | `skills/fomo-kernel/SKILL.md` rule 8 (`## Non-negotiable rules`) ↔ `AGENTS.md` boundary 8 (`## Non-negotiable boundaries`) — the two guaranteed-delivery entry points (`docs/development-guide.md` §6) — ↔ `skills/fomo-kernel/references/freeform-answers.md` (the full rationale and the named chart set) ↔ cross-references in `references/interaction-delivery.md` and `references/trade-consequence.md` ↔ `tests/test_doc_language.py`'s `test_freeform_answer_shape_is_a_boundary_in_both_entry_points` and its paired mutation test. Three rules, not one. **The default is text, not a picture**: an ad hoc question, including a `consider` call, gets a quick, direct answer with no chart, rendered artifact, or multi-tool production unless the user asks for more — the 34-turn dogfood session that motivated this rule is the counter-example the rule forbids, not a hypothetical. **A chart is named in advance, never improvised**: the owner reversed the first cut's empty set (PR #554) into two named entries — the review card itself, reachable in freeform conversation only through the existing `card-delivery.md` contract (never a fresh rendering, and the card's own sparkline stays scoped to the card rather than becoming independently reachable), and a Positions view, at the time a fixed ungraphical table (`Ticker | Shares | Avg cost | Weight of book`, sorted by weight descending) that `freeform-answers.md` stated plainly had no dedicated no-side-effect `engine/review.py` data outlet yet — `consider` requires a real premise and durably records an evaluation, `refresh` requires a newly supplied broker snapshot, and neither is a passive-lookup shortcut. A third, owner-approved entry is still a name written into the reference file, not a shape invented inline. The gap closed in #561 (owner ruling 2026-07-30): the shape widened into the richer, already-demoed per-position diagnosis (README.md's "What it looks like") — ticker, shares, avg cost, value, $ P&L, and the sizing/averaging-down/exit-discipline/hold-consistency tags, sorted by size — and `engine/review.py positions` is the outlet, reconstructing the book from `ledger.jsonl` alone through the exact FIFO pipeline (`round_trips` → `fifo_held` → `classify_adds` → `dim_size` → `ticker_diagnosis`) `trade_recap.py`'s own CSV review already runs, deliberately not the canonical average-cost book `consider`'s ledger route uses (`ticker_diagnosis` sums FIFO-realized and unrealized P&L into one figure, and mixing that with an average-cost remaining balance misstates the sum on a multi-lot partial sell — `engine/review.py`'s `_positions_diagnosis` docstring). Whitelisted alongside `resolve-market-data` as writing no session, ledger, or evaluation row. **The set bounds the agent, not the user**: both rules above constrain what the agent decides to produce unprompted; neither is a ceiling on an explicit user request, and every other non-negotiable rule still governs however that request gets answered. Only the rule's *presence in both entry points* is mechanically gated — the shared literal phrase check catches one entry point silently dropping the rule while the other keeps it, but nothing offline can observe whether a live host conversation actually stayed brief, the same instruction-only footing `docs/development-guide.md` §4 already admits for the recommendation ban. Boundary with #525: this is an effort/scope ceiling on production cost, not a disclosure-content contract — whether a freeform answer states every material limitation as rigorously as the card's `honesty_ledger` is #525's separate, still-open question, not settled here. **That boundary is itself a pinned phrase in both entry points**, not only in the soft-routed reference: the rule names `consider`, which is exactly the surface #479 delivers a visible two-sided challenge on, so an entry point that says "answered briefly" without `Brevity bounds what an answer produces, never which facts it owes` invites compressing away the disclosures that surface exists to make. The user-override clause is pinned the same way — `bounds what the agent decides to produce on its own initiative, never what the user explicitly asks for` appears verbatim in both entry points, so one cannot silently narrow back to "not even if the user asks" while the other still permits it. **The rule now carries exactly one exception, pinned the same way** (#629, owner ruling 2026-07-30): price recovery *is* multi-tool production by the letter of the rule, so an entry point carrying only the rule tells a host not to do what the ruling requires — and the failure is silent, because the host then relays a concentration answer weighted on cost. Five further phrases are therefore pinned in both entry points — the exception itself (`recovering a price the engine could not retrieve is completing the input, not production`), its narrowness (`It licenses no chart, artifact, or other multi-tool work`), its bound (`transcription, not analysis`, `at most twenty instruments`, `delegated to whatever faster tier the host has`), and what it degrades to (`refused rather than answered on cost basis`). The delegation clause names no model, tier, or subagent mechanism (owner ruling 2026-07-26): this is a cross-host public skill, so the contract states the outcome and the host decides how. The row below owns the engine half. | -| Market-data acquisition (#605) | `engine/market_data.py` is the only place this repository reaches a market-data provider, and `tests/test_market_data.py`'s AST guard says so mechanically — only that module may import `yfinance`, and a companion test fails when a grandfathered site has migrated, so the exemption list decays instead of becoming a permanent hole. Its readers are the four projections in `trade_recap.py` (`fetch_prices`, `fetch_splits`, `fetch_fx`, `fetch_fx_series`, which now select out of one bundle rather than fetching) ↔ `market_request` / `_ledger_rebase_origin` / `_project_frame` ↔ `price_feed.py`'s supplied adapter and its `_ERROR_SIGNATURES` table ↔ the `cache` entry in `coach.py`'s `DATA_FILES`. Four rules. **One request per distinct symbol, never twice**: `yf.download` reads like a batch endpoint but fans out one chart request per symbol, so the floor is one per symbol and the contract is not exceeding it — measured 17→8 on a six-instrument, two-currency book, where the waste was `fetch_splits` re-requesting the endpoint the price download already hit. **One instant**: closes, split observations and FX come out of the same response, because two calls seconds apart really do disagree (`^VIX` did, in the probe that motivated this). **Offline is a value the resolver reads, never discipline at a call site**: `TR_OFFLINE`, set once by `tests/run_all.py`, makes the adapter degrade to *exactly* the missing-provider answer — which is why `price_feed.classify_error` maps both to one `client_missing` code, so the state sha256 behind `session_id` cannot tell them apart. **A gap is a stable code, never a number**: `GAP_CODES` refuses an undeclared code at the emit site, a missing rate stays absent rather than becoming 1.0, and a resolution that answered nothing is retried rather than frozen as the day's conclusion. **A resolved bundle reaches a route only as a `price-feed` envelope**: `market_data.to_price_feed_envelope` ↔ `review._resolve_consider_prices` / `_consider_market_universe` ↔ `cmd_resolve_market_data`. `consider` with no `--prices` now retrieves current facts (#605 section E, superseding the claim that the route is offline by construction) and pours them into the *supplied* lane, which already reconciles currencies, checks the split basis, builds the valuation manifest and freezes provenance — so live resolution adds no second downstream path, and "a resolved run produces the same normalized valuation facts as an equivalent supplied fixture" is pinned by `tests/test_consider.py`'s section N rather than argued. Two rules that lane imposes. **Only current observations, never a restated series**: a provider close is already retro-adjusted while the envelope's contract is that a close is raw and gets rebased from its own date, so a declared split *newer than the close it would divide* double-adjusts it — the tenfold error #583 exists to prevent, arriving through the repair for it — and is therefore dropped, which is also the correct pairing, since a split no close reflects has been applied to neither operand. **`TR_OFFLINE` degrades to exactly the pre-#605 answer** (`valuation_basis: unpriced`), which is what keeps ninety-odd `--prices`-free `consider` tests meaningful instead of silently re-baselined. The route half is gated by driving the real CLI against an injected provider, not by calling the helper — a test that calls the helper stays green when the call site is severed, the escape that got past the split gates three times in one day. The window half of this boundary is stated in the split row below, because that is where being wrong about it costs a share count. | +| Market-data acquisition (#605) | `engine/market_data.py` is the only place this repository reaches a market-data provider, and `tests/test_market_data.py`'s AST guard says so mechanically — only that module may import `yfinance`, and a companion test fails when a grandfathered site has migrated, so the exemption list decays instead of becoming a permanent hole. Its readers are the four projections in `trade_recap.py` (`fetch_prices`, `fetch_splits`, `fetch_fx`, `fetch_fx_series`, which now select out of one bundle rather than fetching) ↔ `market_request` / `_ledger_rebase_origin` / `_project_frame` ↔ `price_feed.py`'s supplied adapter and its `_ERROR_SIGNATURES` table ↔ the `cache` entry in `coach.py`'s `DATA_FILES`. Four rules. **One request per distinct symbol, never twice**: `yf.download` reads like a batch endpoint but fans out one chart request per symbol, so the floor is one per symbol and the contract is not exceeding it — measured 17→8 on a six-instrument, two-currency book, where the waste was `fetch_splits` re-requesting the endpoint the price download already hit. **One instant**: closes, split observations and FX come out of the same response, because two calls seconds apart really do disagree (`^VIX` did, in the probe that motivated this). **Offline is a value the resolver reads, never discipline at a call site**: `TR_OFFLINE`, set once by `tests/run_all.py`, makes the adapter degrade to *exactly* the missing-provider answer — which is why `price_feed.classify_error` maps both to one `client_missing` code, so the state sha256 behind `session_id` cannot tell them apart. **A gap is a stable code, never a number**: `GAP_CODES` refuses an undeclared code at the emit site, a missing rate stays absent rather than becoming 1.0, and a resolution that answered nothing is retried rather than frozen as the day's conclusion. **A resolved bundle reaches a route only as a `price-feed` envelope**: `market_data.to_price_feed_envelope` ↔ `review._resolve_consider_prices` / `_consider_market_universe` ↔ `cmd_resolve_market_data`. `consider` with no `--prices` now retrieves current facts (#605 section E, superseding the claim that the route is offline by construction) and pours them into the *supplied* lane, which already reconciles currencies, checks the split basis, builds the valuation manifest and freezes provenance — so live resolution adds no second downstream path, and "a resolved run produces the same normalized valuation facts as an equivalent supplied fixture" is pinned by `tests/test_consider.py`'s section N rather than argued. Two rules that lane imposes. **Only current observations, never a restated series**: a provider close is already retro-adjusted while the envelope's contract is that a close is raw and gets rebased from its own date, so a declared split *newer than the close it would divide* double-adjusts it — the tenfold error #583 exists to prevent, arriving through the repair for it — and is therefore dropped, which is also the correct pairing, since a split no close reflects has been applied to neither operand. **`TR_OFFLINE` degrades to exactly the pre-#605 answer** (`valuation_basis: unpriced`), which is what keeps ninety-odd `--prices`-free `consider` tests meaningful instead of silently re-baselined. The route half is gated by driving the real CLI against an injected provider, not by calling the helper — a test that calls the helper stays green when the call site is severed, the escape that got past the split gates three times in one day. The window half of this boundary is stated in the split row below, because that is where being wrong about it costs a share count. **Two postures, one reader each, and neither may be re-decided per route**: `network_allowed`/`TR_OFFLINE` answers *may this machine reach a provider*, and `frame_frozen`/`FROZEN_ENV` answers *may this pass observe the market again at all* — the second exists because a pass amending a review the user has already read wants the frame that review was rendered from, and `covers` is deliberately the wrong rule for it (#665; see the cash-anchor row). Offline is checked first, so an offline run's gap code describes the machine rather than the subcommand. `tests/test_market_data.py`'s section E2 pins the reuse, the never-reaches-the-provider claim on every outcome including a missing frame, and the day boundary; section H pins one reader for each posture by AST, which is why `review.py` exports `market_data.FROZEN_ENV` instead of spelling the variable out. | | Split adjustment (#550) | `engine/splits.py` is the only implementation of what a split does to a recorded quantity — `trade_recap.adjust_for_splits` is now a delegate, and `revisit.detect_exits` reads the same rule rather than walking raw quantities — in **both** of its lanes: the trade walk, and the `absence_exits` rows it appends for a confirmed disappearance (#485 Slice C). That second lane shipped without the map, so one returned list carried two bases and the book reported a departure ten times the exit row beside it — #550's own defect one level down, and the reason `absence_exits` takes `splits` at all. Retrieval moved to `engine/market_data.py` (#605) and is now a projection: `trade_recap.fetch_splits` selects this book's tickers out of one resolved bundle whose split **observations**, closes and FX all came from a single provider response, so a review can no longer state a price, a split and a rate observed at three different instants. `splits.py` is still stdlib and offline and never fetches. One property of that boundary is load-bearing here: a resolved map is complete only from its request's window start, where `Ticker(t).splits` was unbounded — safe because every factor applies splits strictly *after* a real book date, enforced by `market_data.build_request` refusing a window that cannot cover its caller's `rebase_origin`, and by `trade_recap.market_request` deriving that origin from the **union** of the CSV's first trade and the ledger anchor (`_ledger_rebase_origin`) rather than from the CSV alone — the map a review freezes is read back by `consider` and `refresh` against an anchor the CSV never mentioned. Gated by `tests/test_split_basis.py`'s three window tests and `tests/test_market_data.py`. The map is derived once per review and **stamped** into `state["splits"]` (JSON-safe, key locked by `tests/test_tr_json_contract.py`), which `review._prepare_exit_capture` passes to `revisit.enqueue_from_ledger` — the collapse-to-one-reader move from development-guide §7, so the analytics path and the exit path cannot disagree about what a split did. Two invariants. **The ledger keeps as-transacted quantities**: normalization happens at read time, never by rewriting `ledger.jsonl`. **The exit basis is the exit's own date, not today's**: `factor_between` scales the *running* position as each split arrives, so `shares_sold` — and therefore `revisit_id` — is the count that actually executed and a later split cannot re-base an exit already in the queue. Owner-ratified 2026-07-29 against the alternative of restating old exits in today's basis: that count is the one on the user's own broker statement for that sale, and re-basing it would move `revisit_id` for exits already answered, so the user would be asked a second time why they sold something they have already explained. Every reader of the recorded book must be handed the map — gated mechanically by `tests/test_split_basis.py`'s `test_every_reader_of_the_book_is_told_what_a_split_did_to_it`, because a caller that omits `splits` gets raw quantities and raises nothing; its `_NO_MAP_NEEDED` exemptions are themselves checked for still matching a live call site. **A reader of the book is anything that returns one**, which is why that net watches `portfolio_basis.query_current_book`/`query_current_book_from_ledger` alongside the two `ledger` functions: scoped to the latter, it read a split-blind `query_current_book` call as compliant — the wrapper forwards its own map faithfully — and the caller one level up went unexamined for a release. That caller was `review._virtual_valuation_frame`, which narrows the price frame from a *provisional* book and must be given the same map as the canonical query that then validates the frame against holdings; split-blind on one read and split-aware on the other, a position whose raw quantities reach zero is dropped from the frame without even appearing as `missing_price`, and `prepare` refuses the very book `derive_holdings`, `refresh` and `consider` read correctly. `factor_after` is the other basis (rebase onto today) and belongs to `rebase_rows` alone. Split events are fail-closed on parse: a ratio is a multiplier on a share count, and a dropped bad one is a confident wrong number. Readers: `references/price-feed.md`'s `splits` bullet ↔ `schemas/price-feed.schema.json` ↔ `tests/test_revisit.py` section A2 + `tests/test_engine_units.py` + `tests/test_review_v2.py`. `ledger.derive_holdings` closed the same gap for the book itself (#558): it takes the map as a parameter and applies it at **three** points — the anchor's declared shares, each trade as it arrives, and once more at the tail, which is the only one that fires in the most common shape of all (a split with no trading since). `holdings_as_of` bounds that tail with its own `as_of`, so a diff computed for an earlier day is not silently stated in today's basis. Everything downstream threads it: `reconcile`, `snapshot_reconciliation`, `build_derived_book` (which **writes the number down**, so an unadjusted count there stops being a read-time error and becomes a durable one), `book_refresh`, `portfolio_basis.query_current_book`. **Cost is never scaled** by any of the three — a split is a zero-dollar event, and preserving cost while shares move is what makes the derived `avg_cost` correct without a second adjustment. `revisit.rebased_exit_price` is the #559 half: a read-time rebase of a stored exit price onto today's quote basis, never a stored one, because `revisit_id` derives from what executed and must not churn on the next split. Retrieval for the lanes with no review of their own is `review._recorded_splits`, which reads the map the last review froze rather than fetching — `refresh` runs as two CLI calls and content-addresses what the user was shown, so a network blip between them must not invalidate a `refresh_id` mid-answer. `consider` resolves its map once, in `_consider_rows`, for **both** of its routes: precedence is **per ticker, never per call** (#583 post-merge finding): the frozen map is the floor, a ticker with supplied `price_feed.splits_map` events overrides its own entry, and an unrelated ticker's recorded split survives — whole-map replacement read a legitimate omission (a close already past its split needs no `splits` row) as "never split", mis-valued that book by the split factor, and disarmed `basis_conflicts` for exactly that ticker; it still never fetches. Its CSV route was the last reader with no adjustment at all — `trade_recap.load()` rows go straight into `consequence.portfolio_state`, so a split-crossing position was simply absent from the book a pre-trade challenge reasoned about, which is #558's own defect on the one lane a CSV-only user reaches. That route applies `trade_recap.adjust_for_splits` (rebase onto today, `factor_after`) rather than the running-position carry, because `last_px` is a current quote and the rows must be denominated against it — the same call `prepare` makes right after resolving its bundle, and the reason the two bases coexist by design rather than by accident. Gate: `tests/test_split_basis.py`, one test per application point plus the counterweight that a real full exit still reads as one — **and a route-level half, because the call-graph half was escaped three times in one day for three different reasons** (#572: a caller one level above a watched callee; #577: a route that calls no watched callee at all; #576: a supplier returning nothing, which left four suites green). `test_every_route_that_reads_the_book_is_classified` derives the route list from `review.py`'s own call graph rather than a hand list — `consider`, `prepare`, `refresh`, and nothing else — so a new subcommand that reads the book arrives as a failure instead of as an untested route, and the four route tests beside it drive one split-crossing book through each and assert that route's own honest observable: the share count for `consider` (**both** of its book-building routes), `reconciled` with nothing to confirm for `refresh`, and that the review starts at all for `prepare`. That last one is the compounding failure worth knowing — split-blind, `plan_refresh` invents a share change, #530's catch-up gate then refuses the review outright, and the user is sent to settle a purchase they never made. Everything in this row is about the **quantity**; the price it gets multiplied by has a basis of its own, and that is the row below. | | Split basis of a price observation (#583) | The other operand. Every row above audits quantities, and #558's closing sweep said so explicitly — "no third unadjusted accumulator remains" was true, and true of one dimension only. `engine/splits.py` owns this side too: `rebase_price` (divide a price by the splits after its observation — the mirror of `rebase_rows` multiplying a share count by them), `rebase_series` (the same, per entry, for a daily series), `basis_date` (**the only** derivation of what basis a rebased value is stated in), and `last_applied` (when a window last moved a basis, the companion to the factors' *how much*). `revisit.rebased_exit_price` and `_applied`'s single walk mean every factor and every basis statement read one implementation. Readers, in the order the number travels. **`price_feed.parse` normalizes on the way in**: a supplied `close`/`history` is declared a raw observation as of its own date (`schemas/price-feed.schema.json` ↔ `references/price-feed.md` "Prices are raw observations"), and each is rebased from that date onto the envelope's declared-split basis, keeping `observed_close`/`observed_date`/`basis_date`/`split_factor` beside it so the alignment replays. Before this, a structurally valid envelope paired a pre-split close with a post-split share count: a tenfold market value, a tenfold weight, and an inverted answer to *which position is the largest* — `consider` naming the split-crossing holding as the book's biggest at 51% when it was nine per cent of it. Because the split is declared on the price row, `parse` fixes every downstream consumer at once, including `to_frame`'s series before beta, alpha, the P&L curve and account TWR difference it. **`price_feed.basis_conflicts` is the fail-closed half**, called by `review.cmd_consider` before any book is read: `prepare` with `--prices` performs no split retrieval of its own so the two sides are the same events by construction, but `consider` falls back to `review._recorded_splits` when the envelope declares none — the shares then cross a split the prices were never told about. It compares the divisor each observation received against the one its share count did, past that same date; unequal is a refusal naming the ticker, the observation date and the split, because applying the external map would assert a corporate action the price source never confirmed (#566) and ignoring it is the tenfold error. `review._effective_splits` is the single reader of the precedence rule — per-ticker merge, recorded floor, supplied override, per the #550 row — so the map the check validates is the map the arithmetic used, and a recorded event cannot vanish because an unrelated ticker supplied one (gated by `tests/test_split_basis.py`'s `test_a_recorded_split_survives_an_unrelated_tickers_supplied_one`). **`trade_recap.price_observations` is the single producer** of per-ticker evidence, feeding both `state["price_snapshot"].observations` and `build_valuation_frame(price_observations=...)`; `last_prices` is now its projection, so the staleness gate and the basis statement are one read. One frame-level `as_of` could not say which session a given ticker's number came from, so one fresh instrument made every stale one look same-day (#583 §2). The frame's per-ticker `observed_at`/`basis_date` are **additive** — a stored frame from before them still validates untouched and is never rewritten — but never partial within one frame, and never a basis older than its own observation. **`ledger.derive_holdings` states the book's side** as `quantity_basis`, recorded at each rebase by `_moved_basis` rather than re-derived, and `portfolio_basis.query_current_book` folds it into `effective_date`: a split is the day a share count changed, exactly as a trade is, and without it a valuation dated between the last trade and the split passed the `reference_as_of` gate while the holdings had already moved (§3). Advancing that date rather than adding a parallel field is what keeps a book no split touched **byte-identical** — `quantity_basis` is `None` there, so `state_version` does not move and there is nothing to migrate or roll back. `_refuse_unaligned_price_basis` is the gate the two sides exist for, **per ticker, never per book**: the newest split in a book belongs to some instrument, and holding every other price to it would refuse an ordinary book for an action that never touched them. **`revisit.compare`/`scan_backlog` take `price_basis`** and `review._prepare_exit_capture` supplies it from the frozen `observations` — it previously dropped `price_snapshot.as_of` entirely, so the comparison rebased across every split after the exit and merely assumed the quote postdated them (§4). A missing entry degrades to that same unbounded rebase, which is #559's answer and stays correct; `rebased_exit_price` is still read-time only, so `revisit_id` never churns. Gate: `tests/test_split_basis.py`'s price-basis sections (both `consider` routes on one crossing book, the refusal *and* the repair it names, the return consumer through `pnl_curve`, the per-ticker evidence from the real `TR_STATE_OUT` artifact, the effective date and its byte-identical counterweight) + `tests/test_price_feed.py` section 1b + `tests/test_tr_json_contract.py`'s `price_snapshot` key set and coverage relation. One coercion rule earns its own mention because a green suite hid it for a release: **every factor coerces its own window edges**. `build_derived_book` hands `derive_holdings` an ISO string where `holdings_as_of` hands it a date, the tail rebase compared the two directly, and recording the derived book for any split-crossing history raised `TypeError` — `prepare` died with a traceback rather than the refusal this module promises. It was invisible offline because the only split-crossing `prepare` test used a snapshot root that never reaches that writer; `test_a_review_records_its_derived_book_on_a_split_crossing_history` drives the CSV lane that does. | | Which basis the transaction file itself is on (#582) | The premise under both rows above, unstated anywhere until now: **a transaction file records what executed, un-rebased**. `rebase_rows` is correct only on a file of that kind, and a broker export that has already restated its own history gets the split applied twice — ten times the real share count on a ten-for-one, cost preserved, so `avg_cost` falls tenfold and weight, concentration, the sizing cap and every rule measured against them read on a book that never existed. It survived every gate above because it **inflates**: #550's family erased positions, and a book reaching zero is a visible anomaly, while a position ten times too large reads as a large position rather than a broken one. `splits.basis_disagreements` is the single implementation of the check, and it lives beside the arithmetic it audits because it needs nothing this module does not already own; `splits.EXECUTION_BAND` is its only constant — how far a fill may sit from its own session's close — and the separation requirement is *derived* from it (`band ** 2`), so a three-for-two is not examined rather than guessed at. Its readers, and the whole path: `trade_recap.close_series` supplies `{ticker: [(date, close), ...]}` from **`bundle.frame`, not the narrowed frame `fetch_prices` returns** — that window starts at the earliest round-trip entry, and a position bought long ago and never sold has no round trip, so its own trades would fall outside the very window the check needs — ↔ `trade_recap` main, which calls it immediately after `adjust_for_splits` and records a non-empty result as `data_integrity["split_basis"]` ↔ `review._refuse_an_unsettled_transaction_basis`, the only reader, called from `_prepare_session` **before `save_pending`, before any ledger write, and before a card is built** ↔ `references/data-contract.md` "Transaction basis" ↔ `tests/test_split_basis.py`'s #582 section. Three design facts no surface may weaken. **It refuses rather than discloses**, unlike #330's `price_feed.plausibility_flags` one row over: that check flags one suspicious *supplied close* and the card still prices normally, while this one says the share basis of the whole file may be wrong, and every number on the card is measured against it — continuing while merely disclosing *is* choosing the as-executed reading, silently. That is also why it is deliberately **not** in `build_honesty_ledger`: no card carrying it is ever rendered, so a ledger key there would be a dead field (#429). **It never adjudicates and never re-bases** — an adjusted export, a currency in the wrong unit and a strange fill all print the same disagreement, and picking one from the numbers is what #416 forbids; the refusal states the contradiction, names the rows it measured, and hands the question to the user, which is the confirmation posture #530/#531 established for an observed difference (`book_refresh`'s own `pending_confirmations` lane cannot be used here: it needs a declared holdings view, and a CSV-only user — the entire exposed population — has none. `book_refresh.plan_refresh` does catch this by accident for a user who supplies one, because derived comes out exactly `factor ×` declared and that raises `large_change` — but only while the inflated position clears both `REFRESH_MAJOR_DELTA` and `REFRESH_CORE_WEIGHT`; measured, the same defect on a small corner of the book returns `status: ready` with nothing pending and is adopted silently, so this complements that defence rather than duplicating it, and fires at ingest at any position size). **It cannot see the case #330 sees and #330 cannot see this one**: #330 anchors a supplied close against the ticker's *most recent* trade, which for almost every real book is after the split, where both readings agree — the comparison has to reach a **pre-split** row, which is why every row it examines has a non-unit `factor_after`. Gate: `tests/test_split_basis.py`'s pair of CLI runs over one synthetic already-adjusted file and the identical history as-executed, plus the no-split counterweight on a ticker in the same file whose fill sits a factor of ten under its own close and is never examined, plus `test_the_check_reads_the_real_close_series_and_not_merely_its_absence` — the supply is driven on a real resolved frame, because a gate that only proves a parameter was passed is the failure shape #576 shipped. `consider` is deliberately not covered: its CSV route fetches current prices only, so checking there would need the extra network segment #582 rules out. | @@ -149,12 +149,12 @@ When multiple sessions are active: | What a recorded book is, and which one re-bases the replay (#549) | `ledger.latest_anchor(events, *, declared_only=False)` is the single reader of both questions, one argument apart, so they cannot drift about ordering or validity. **Default = "has this root recorded a book at all"**: every `type: "snapshot"` row qualifies, whatever its `source` and whoever wrote it — nothing has to qualify, because whatever the user handed over *is* what the system records. Its callers are the ones asking that question: `book_refresh.plan_refresh`'s gate, `ledger.snapshot_reconciliation`'s existence check and `against` stamp, and `session._assert_initial_snapshot_boundary`'s idempotent-replay check. **`declared_only=True` = "which row re-bases the replay"**, and exactly four readers call it — `ledger.derive_holdings`, `portfolio_basis.query_current_book`, `review._rows_from_ledger`, and `revisit.detect_exits` — because a `DERIVED_BOOK_SOURCE` row is `derive_holdings`' own output written down: replaying the trades it summarizes reproduces it exactly, while re-basing on it would discard the cycle start, cycle sequence and add count a summary row cannot carry. Two more readers sit one level down and follow the same rule for their own reason: `book_refresh.carry_recorded_starts`' `recorded` map (a restatement never stamps a `since_basis`, so reading the newest row would spend the user's #531 answer the first time they imported a CSV; the map is assembled inside that primitive rather than by either lane, because a single reader is still two readers when its input is composed twice) and both ingest lanes' overlay gate in `review.py` (a restatement is the derivation being compared, so routing it there reconciles the book against itself). That round-trip equality is the load-bearing invariant and it is gated, not argued: `tests/test_ledger.py::test_a_recorded_derived_book_re_derives_to_exactly_the_same_book` re-derives six histories with the recorded row appended and demands field-for-field equality; its matching mutation (`derive_holdings` dropping `declared_only`) turns it red. Two rules about the row itself. **The writer is single**: `ledger.build_derived_book` (self-validating against `SNAPSHOT_POSITION_KEYS`, the `build_position_absence` precedent) ↔ `review._record_derived_book`, the one place either transaction lane records its result, appending through the shared `session.append_book_adoption` rather than a second writer of "this is the book now". **A derived book states positions and never a balance**: `perf.cash_reconcile_residuals` reads every cash-bearing snapshot row as a declaration, so a copied-forward balance would manufacture the exact shape it reports as an unexplained residual; `snapshot_reconciliation` reads its cash baseline from the last *declaration* instead. Its readers: `references/data-contract.md` ↔ `docs/prd-ledger.md` "Holding derivation" ↔ `flows/book-refresh.md`'s two routes ↔ `README.md`/`README.zh-TW.md` "What does a snapshot anchor?" ↔ `tests/test_book_refresh.py` (the trades-built root that can now be refreshed) ↔ `tests/test_consider.py` (the CSV route writes the row and `consider` still reads the replay). | | Completeness is gone, everywhere (#549, finishing #485) | There is no `is_complete`: not on the envelope (`snapshot_adapter.ENVELOPE_KEYS`), not on a ledger row's identity (`session._snapshot_payload`), not in `latest_anchor`'s selection, not in `portfolio_basis` (`_normalized_anchor`, the `current_book.anchor` key set, `schemas/portfolio-basis.schema.json`, and `_COMPLETENESS`, whose `declared_partial` value is now unreachable and survives only in `schemas/trade-evaluation.schema.json` for replay compatibility), not in `session`'s projection gates, and not on the card (`card_renderer._snapshot_strength_line` has two branches, and `copy/*.json` `snapshot.strength` has `weighted`/`baseline`). `tests/test_book_refresh.py::test_no_engine_module_reads_is_complete_any_more` parses every non-test engine module for the exact string constant and fails on a reader coming back — prose that merely names the removed flag is not a reader, which is why it parses rather than greps. The reason it may not come back: a user who said their view covered one brokerage of several got a helpful `false`, and their book then silently stopped updating — #549's own defect in a second place. Nothing replaces it on the input side; the anomaly worth raising between two updates is `book_refresh`'s confirmation lane (#530, #531), which asks about a *specific observed difference* rather than about an account nobody can see. Taking the flag out of the engine was only half of it: both guaranteed-delivery entry points and three routed surfaces went on teaching the retired rule for a release, and `flows/snapshot-review.md` went furthest — it told the agent to *ask for the complete account view*, the one question this ruling forbids, behind an engine rejection that no longer existed. `tests/test_doc_language.py`'s `RECORDED_BOOK_SECTIONS` pins the replacement rule in `AGENTS.md` and `SKILL.md`, and `RECORDED_BOOK_RETIRED_PHRASES` rejects the retired phrasing across every file `agent_runtime_files()` reaches; scoping that rejection to the two entry points is precisely how the routed surfaces kept the old semantics while the entry points read correctly. One consequence to know: `thesis.build_snapshot_cycle_relinks` lost its `snapshot_complete is False` condition with the flag, and since every declaration now anchors, no shipped flow reaches it — its remaining coverage is unit-level. | | What book a current-view figure was measured over (#630) | `trade_recap.book_scope` is the single derivation of a current-book `coverage.scope`, and it takes two halves: every holding valued, **and** those holdings known to be the recorded book. Its readers: `trade_recap.current_book_projection` (which mints `book_basis` and forwards it from `dim_size`) ↔ `snapshot_adapter.prepare` (a declared holdings view *is* the recorded book, per #549, so it states `BOOK_BASIS_RECORDED` directly) ↔ `review._stamp_recorded_book_basis` (the single decider for the transaction lanes, called only after `_record_derived_book` wrote the book this import produced) ↔ `tests/test_engine_units.py`'s `test_current_book_projection_vocabulary_locked_to_portfolio_basis`, which locks the scope vocabulary to `portfolio_basis._COVERAGE_SCOPES` and therefore to `schemas/trade-evaluation.schema.json`'s stored enum. Three rules. **A projection may not assert what it cannot check**: it receives holdings and no book to compare them with, so the whole-book claim is the caller's to make and the default is the value that assumes nothing — deciding `scope` from valuation coverage alone is exactly how a weekly review declared `full_current_book` with `total_holdings: 1` over a six-position account, telling every reader that checked the field the number was trustworthy. **A bounded book and a bounded valuation are one word, not two**: both are `bounded_valued_subset` with `book_basis` naming which bounded it, because `portfolio_basis` always measures the recorded book and a fourth value only this shell could produce would sit permanently unreachable in that stored enum. **The reconciliation that withdrew the figures may not also relabel them**: a mismatching `_overlay_ledger_holdings` result has already stripped every current-view surface through `_gate_current_view`, so the stamp refuses rather than skipping silently. | -| Which lane reconciles the card against the recorded book (#630) | `review._overlay_ledger_holdings` runs from **both** ingest lanes, but on different terms, and the terms are the point. `_verify_and_ingest_frozen_trades` — the lane every review a real user runs takes, because `cmd_prepare`'s `freeze_candidates` is true for any CSV without `--card-json` — reconciles whenever the **post-import** book has positions, declared anchor or #549-derived; `_ingest_trades` is reachable only with injected `--card-json`/`--state-json`, where `engine_state.holdings` was asserted by the caller and is not a derivation of the CSVs beside it, so it keeps `declared_only`. Scoping the first one to `declared_only` was the defect: a root whose book came from an earlier CSV import carries only a `trades_derived` row, so an ordinary weekly export covering one ticker was never compared with the book and became the denominator for every current-view figure, the rule the user was asked to commit to, and a `problems.jsonl` breach recorded against them. `declared_anchor` carries what the derived lane may **not** compare, and both exclusions are false positives rather than loosened standards: **cost basis** differs by method (`ledger.derive_holdings` keeps a moving average, the card's own accumulation is FIFO — 23250 against 24900 for `mock/sample_ai_holder.csv`'s NVDA), and **market/currency** are not written onto a holding by `trade_recap.build_state` at all, so the US/USD default would report every non-US position as misclassified. Position coverage and share counts are pure arithmetic over the same rows, agree in both lanes, and are exactly what a partial file differs in. **The whole thing must be idempotent across a re-entrant `prepare`**, and that takes two independent halves, because either one alone hides the defect. The predicate reads the *post-import* book, never `latest_anchor(live_events)`: `prepare` writes that ledger, so a pre-import predicate is false on a fresh root's first review and true on every later run of the identical file. And on the derived lane a reconciliation that **agrees** adopts nothing — both sides are readings of the same rows, so there is nothing the card does not already have, while adopting anyway would move `cost`/`avg_cost` onto `derive_holdings`' moving average for every ordinary review. Together they are what makes `add-cash` possible at all on a trades-only root: it re-enters this exact pipeline to add an anchor to a session the user has already answered against, and `_cash_recompute_drift` refuses when anything but the anchor moved. Gated by `tests/test_review_v2.py`'s five `#630` tests, each driving the real CLI over a `mock/` fixture: the partial week, the cumulative week that must still compute, the non-US book that must not read as misclassified, the lane asymmetry, and the re-entrant idempotency — whose two assertions redden under one mutation each. What this does **not** do is recompute concentration over the canonical book — a partial week withdraws those figures rather than restating them, because the engine's price universe and currency map are built from the supplied rows (`trade_recap.main`'s `tickers`/`cur_map`), so measuring the wider book would price one position and cost-basis the rest. That half is #634. | +| Which lane reconciles the card against the recorded book (#630) | `review._overlay_ledger_holdings` runs from **both** ingest lanes, but on different terms, and the terms are the point. `_verify_and_ingest_frozen_trades` — the lane every review a real user runs takes, because `cmd_prepare`'s `freeze_candidates` is true for any CSV without `--card-json` — reconciles whenever the **post-import** book has positions, declared anchor or #549-derived; `_ingest_trades` is reachable only with injected `--card-json`/`--state-json`, where `engine_state.holdings` was asserted by the caller and is not a derivation of the CSVs beside it, so it keeps `declared_only`. Scoping the first one to `declared_only` was the defect: a root whose book came from an earlier CSV import carries only a `trades_derived` row, so an ordinary weekly export covering one ticker was never compared with the book and became the denominator for every current-view figure, the rule the user was asked to commit to, and a `problems.jsonl` breach recorded against them. `declared_anchor` carries what the derived lane may **not** compare, and both exclusions are false positives rather than loosened standards: **cost basis** differs by method (`ledger.derive_holdings` keeps a moving average, the card's own accumulation is FIFO — 23250 against 24900 for `mock/sample_ai_holder.csv`'s NVDA), and **market/currency** are not written onto a holding by `trade_recap.build_state` at all, so the US/USD default would report every non-US position as misclassified. Position coverage and share counts are pure arithmetic over the same rows, agree in both lanes, and are exactly what a partial file differs in. **The whole thing must be idempotent across a re-entrant `prepare`**, and that takes two independent halves, because either one alone hides the defect. The predicate reads the *post-import* book, never `latest_anchor(live_events)`: `prepare` writes that ledger, so a pre-import predicate is false on a fresh root's first review and true on every later run of the identical file. And on the derived lane a reconciliation that **agrees** adopts nothing — both sides are readings of the same rows, so there is nothing the card does not already have, while adopting anyway would move `cost`/`avg_cost` onto `derive_holdings`' moving average for every ordinary review. Together they are what makes `add-cash` possible at all on a trades-only root: it re-enters this exact pipeline to add an anchor to a session the user has already answered against, and `_cash_recompute_drift` refuses when the source facts underneath that session moved (#665; the cash-anchor row below owns the gate). Gated by `tests/test_review_v2.py`'s five `#630` tests, each driving the real CLI over a `mock/` fixture: the partial week, the cumulative week that must still compute, the non-US book that must not read as misclassified, the lane asymmetry, and the re-entrant idempotency — whose two assertions redden under one mutation each. What this does **not** do is recompute concentration over the canonical book — a partial week withdraws those figures rather than restating them, because the engine's price universe and currency map are built from the supplied rows (`trade_recap.main`'s `tickers`/`cur_map`), so measuring the wider book would price one position and cost-basis the rest. That half is #634. | | The preview precondition on finalize (#628) | `review.py`'s `_preview_receipt_key` is the single derivation of what a preview receipt is keyed on, called by exactly two sites — `cmd_preview`, which writes the receipt into the pending bundle after the card really rendered, and `_require_preview`, which recomputes it at `cmd_finalize`. Deriving it twice is the hand-mirrored surface this file forbids: the writer and the reader would drift about which fields count, and the failure would be silent in the permissive direction. Its readers: `session.load_pending`'s artifact list (the receipt must survive an interruption and a `resume`, so it lives beside the artifacts it describes) ↔ `SKILL.md`'s fixed lifecycle ↔ `AGENTS.md` step 6 ↔ `tests/test_preview_gate.py`. Four rules. **The key covers every input to the card the user was shown, and excludes exactly one**: `commitment` is left out because the documented order shows the card *first* and asks for the rule *after*, so keying on it would make the contract's own sequence unsatisfiable — every other input is in, which is what makes a preview of different answers, a different narrative, a re-prepared plan, or differently frozen question surfaces fail to certify what is being committed. That exclusion is narrow and was measured, not reasoned: `require_commitment=False` does null `bundle["commitment"]`, but `card_renderer` also reads `answers.commitment.choice` directly, so a previewed `skip` renders a different card from a previewed nothing — the obvious reading of `_draft_bundle` is wrong and a key built on it would have been wrong for the right-looking reason. **It is a precondition on committing, never on validating**: the check runs after `_draft_bundle` and before `commit_bundle`, so `finalize` stays the complete independent validator #628 measured — a session that skipped preview *and* carries invalid answers is still told what is invalid, rather than being sent to preview to find out. **An already-committed session is exempt**, because its pending directory and receipt are gone by construction; it passed the gate on the way in, and refusing the documented-safe finalize retry would make this gate the data loss it exists to prevent. **The refusal names its own fix and says the session survives**, per `AGENTS.md`'s rule that an existing canonical session is not data loss. Every test that reaches `finalize` now goes through `tests/test_review_v2.py`'s `_run_finalize`, which previews first and deliberately does not assert the preview's own result — the suites that assert a finalize-side refusal must keep observing that refusal, not a complaint about the preview. | | A card that names a reward with no way to claim it (#623) | Three different defects wearing one shape, and the fix differs per class — which is why `copy/*.json`'s `block_missing` cannot be treated as one surface. **An invitation** (`snapshot_unlock`, `rule_structural`) names the one answer this user's book cannot reach and nothing else; its rule is `references/decision-framing.md`'s "Earning the next piece of evidence" (#617), routed to from `references/card-policy.md` rather than restated, because two surfaces disagreeing about what an invitation owes is its own defect. Locked by `tests/test_doc_language.py`'s `CARD_INVITATION_REQUIRED_PHRASES` (per-phrase mutation proof) and by `tests/test_review_v2.py`'s retired-catalogue check across all three locales — the retired fragments are what is pinned, because a presence test on the new wording goes green the moment a feature list is appended back onto it. The counterweight matters as much as the rule: a complete behavioral review renders **no** invitation at all, and that absence is asserted, since a manufactured invitation is the same defect as a manufactured disclosure. **A disclosure with a product path** (`annualized_reconciliation`) names the path inline, in the user's own terms, beside the number it explains — never as a command and never promoted into the closing block, which would put two invitations on one card. **A disclosure with no user action at all** is the other 12 strings, correct as they stand and deliberately untouched. Branch tests assert the catalog entry, never a fragment of its wording; `tests/copy_corpus.py`'s golden owns what the sentences say, and `block_missing` is an unclaimed register there — claiming it would demand a scene for all 17 keys. | | A price-degraded card is a stated dead end, never a skipped step (#623 class 2) | `review._price_feed_status`'s `recovery` block is the single statement — `attempted` comes from whether an envelope or an explicit declaration actually arrived, never inferred from `provenance.mode`, because an envelope too broken to frame anything still leaves that mode `unavailable` and reading the attempt off it would call a real attempt a skipped one. Its readers: `review._refuse_a_card_built_on_a_skipped_price_recovery` (on the shared `_draft_bundle` path, so `finalize` called directly cannot walk around `preview`) ↔ `prepare --prices-unavailable` and its entry in `_fingerprint` ↔ `references/price-feed.md` ↔ `flows/first-review.md` step 0 and `flows/weekly-review.md` step 0 ↔ `review.cmd_add_cash` (which replays the declaration off the plan rather than re-typing it) ↔ `tests/test_review_v2.py`. Two rules. **The refusal is not the hard block #357 ruled out**: the user is asked for nothing and nothing waits on them — it is a gate on a step `flows/first-review.md` step 0 already required, clearable either by doing it or by declaring in one flag that it was done and found nothing. **The declaration arrives on a second `prepare`**, so it must be in the fingerprint (the #289/#369/#412 class, fourth instance) or the rerun resumes the undeclared pending session and the only thing separating a skipped step from an honest dead end is dropped; the session id legitimately does not move, because a statement about the outside world changes no engine number. | | The price recovery kit, and the two lanes that degrade in opposite directions (#629) | `review._price_feed_status` is the single builder of the kit — `provenance`, `request`, the `recovery` block above, and the instruction. Two routes call it and neither restates it: `cmd_prepare` (through the plan's `input.price_feed`) and `review._consider_price_feed_status`, which supplies only this route's own inputs and delegates. Only two things differ between them, and both are parameters rather than a second sentence: `command` (which subcommand takes the envelope back) and `request_path` (where the caller finds the manifest in the payload it is holding). `review._declared_prices_unavailable` is likewise the single reader of `--prices-unavailable`, now accepted by both commands, so one cannot accept a declaration the other refuses. Readers: `references/price-feed.md` ("Two lanes, two opposite rules" — the single statement of the asymmetry) ↔ `references/freeform-answers.md` (the agent-facing bound, and why the lookup is not the multi-tool production rule 8 forbids) ↔ `references/trade-consequence.md` ("When the engine could not price the book") ↔ `SKILL.md` rule 8 / `AGENTS.md` boundary 8 ↔ `tests/test_consider.py` section O. **The asymmetry is the point and must stay written down**: the identical `--prices-unavailable` declaration delivers the degraded card on the review lane and *refuses* on `consider`. A retrospective card's cost weights describe what the user actually paid; a forward concentration decision computed on cost describes a book that no longer exists — on `mock/sample_momentum.csv` the largest position moves more than thirteen points and the second and third by size invert. `test_o_the_recovery_kit_is_the_prepare_builder_rather_than_a_second_one` is the anti-fork gate: it rewrites `prepare`'s own sentence with those two substitutions and requires `consider`'s to start with it, so a hand-written second manifest reddens. Deliberately **not** gated: whether a host actually performs the lookup, the same instruction-only footing `docs/development-guide.md` §4 admits for the recommendation ban. | -| The cash anchor, and when it is asked for (#357, #507 knife 1) | `review.py`'s `_cash_anchor_status` is the single statement of whether a review has an accounting anchor — every other surface reads `review_plan.input.cash_anchor` rather than re-deriving it from `state["cash"]`. Its readers: `schemas/review-plan.schema.json` (`input.cash_anchor`, `additionalProperties: false`) ↔ `review.py` `cmd_add_cash` / `_cash_recompute_drift` ↔ `tools/ux_receipt.py`'s `CASH_OUTCOMES` and the outcome-dependent ordering gate ↔ `references/data-contract.md` (the one full statement of the beat) ↔ `references/ux-receipt.md` ↔ `references/interaction-delivery.md` ↔ `flows/first-review.md` and `flows/weekly-review.md` (each one short route-specific pointer — the two verbatim-identical preflight paragraphs #358 shipped were an unregistered hand mirror and are gone) ↔ `docs/qa-runbook.md` gate 3 ↔ `SKILL.md` "Fixed lifecycle" step 4 ↔ `tests/test_review_v2.py` + `tests/test_interaction_trajectory.py`. Four rules. **The engine states the gap in both directions**: with an anchor it already demanded a disclosure (`acct_perf_basis` in `required_honesty_keys`) and without one it demanded nothing at all, so the single condition the agent had to notice unprompted was the only one the plan never mentioned — five recurrences, three on real data. `not_applicable` is a positive claim with a `reason`, never an absent key, which is what makes the light-tier promise mechanical instead of depending on a flow paragraph asking before a tier check three lines later (#358 finding 1). **The timing is the opposite of `input.price_feed.request`'s, and `ask_after` says so**: a price gap is recovered before the user sees anything because every price-dependent number is degraded; a missing anchor degrades one pillar and leaves the rest byte-identical, so it is asked at the card beat where the user can see what answering buys (owner ruling 2026-07-30 against a `prepare` hard block; #507 principle 1). **The recompute proves it was the same review rather than asserting it**: `add-cash` re-enters `_prepare_session` — never a second implementation of run-engine-ingest-plan — and refuses unless `engine_state` outside `cash`, `engine_card` outside `cash`/`acct_perf`/`honesty_ledger`, and the `state_snapshot`, `question_queue`, `missing_thesis_positions` and candidate rules the user already acted on all come back byte-identical. `market_data`'s same-day cache is what makes that gate pass on the normal path, at zero requests; a day boundary, an edited CSV or a moved ledger is a named refusal, not a silent re-basing of answers onto numbers nobody saw. The allowed key sets are measured, not assumed. **No receipt outcome means "the agent decided not to ask"**: `skipped` was that value, and #357's fifth recurrence recorded it correctly and in order while the user was never offered the question, so the gate passed and the experience was identical to forgetting. | +| The cash anchor, and when it is asked for (#357, #507 knife 1) | `review.py`'s `_cash_anchor_status` is the single statement of whether a review has an accounting anchor — every other surface reads `review_plan.input.cash_anchor` rather than re-deriving it from `state["cash"]`. Its readers: `schemas/review-plan.schema.json` (`input.cash_anchor`, `additionalProperties: false`) ↔ `review.py` `cmd_add_cash` / `_cash_recompute_drift` ↔ `tools/ux_receipt.py`'s `CASH_OUTCOMES` and the outcome-dependent ordering gate ↔ `references/data-contract.md` (the one full statement of the beat) ↔ `references/ux-receipt.md` ↔ `references/interaction-delivery.md` ↔ `flows/first-review.md` and `flows/weekly-review.md` (each one short route-specific pointer — the two verbatim-identical preflight paragraphs #358 shipped were an unregistered hand mirror and are gone) ↔ `docs/qa-runbook.md` gate 3 ↔ `SKILL.md` "Fixed lifecycle" step 4 ↔ `tests/test_review_v2.py` + `tests/test_interaction_trajectory.py`. Four rules. **The engine states the gap in both directions**: with an anchor it already demanded a disclosure (`acct_perf_basis` in `required_honesty_keys`) and without one it demanded nothing at all, so the single condition the agent had to notice unprompted was the only one the plan never mentioned — five recurrences, three on real data. `not_applicable` is a positive claim with a `reason`, never an absent key, which is what makes the light-tier promise mechanical instead of depending on a flow paragraph asking before a tier check three lines later (#358 finding 1). **The timing is the opposite of `input.price_feed.request`'s, and `ask_after` says so**: a price gap is recovered before the user sees anything because every price-dependent number is degraded; a missing anchor degrades one pillar and leaves the rest byte-identical, so it is asked at the card beat where the user can see what answering buys (owner ruling 2026-07-30 against a `prepare` hard block; #507 principle 1). **The recompute proves it was the same review rather than asserting it**, and #665 is what that costs when the proof is aimed at the wrong thing. `add-cash` re-enters `_prepare_session` — never a second implementation of run-engine-ingest-plan — and the two halves below are what make that a recompute rather than a second review. **It reuses the frame instead of re-resolving it** (`args.amending_session` → `market_data.FROZEN_ENV` → `market_data.frame_frozen` → `_from_frozen_frame`, an *exact-request* same-day reuse that never reaches a provider on any outcome). The same-day cache was carrying that claim before and could not: `MarketDataBundle.covers` deliberately refuses to serve a request naming a symbol the stored bundle failed to price, so a transient outage gets its retry — and one unpriced symbol anywhere in the universe therefore made this command re-price the whole review at a second instant. **And the gate compares source facts, not derived objects.** `CASH_RECOMPUTE_SOURCE_FACTS` names them positively — the input files, the frozen frame, the recorded book, the diagnosis, the rules offered, the question identities already answered — because the predecessor asked the opposite question (exclude a hand-maintained set of cash-derived keys from `engine_state`/`engine_card`, then compare `state_snapshot` and `question_queue` **whole**) and that shape cannot be kept true: the re-entry's own downstream effects arrived as "drift", the refusal named four surfaces a cash anchor cannot reach, and a card-beat answer became unrecordable. Two rules for adding a row: it must be an input or a frozen observation rather than a projection of one, and it must be *measured* invariant under `--cash` on the same book (across five mock books an anchor moves `engine_state.cash`, `engine_card`'s `cash`/`acct_perf`/`honesty_ledger`, and `card_plan.required_honesty_keys`, and nothing else). The refusal names which of the gate's two verdicts it reached — "the facts moved", never merely that something did — and its counterpart `recompute.outcome: "anchor_propagated"` is emitted on success so the allowed outcome is stated too. A transaction file that grew is refused **before any write**, inside `_verify_and_ingest_frozen_trades` (`amending=True`, on the engine's own `fresh` dedup): a gate that only reads the recomputed plan has already appended the rows it is about to refuse. **No receipt outcome means "the agent decided not to ask"**: `skipped` was that value, and #357's fifth recurrence recorded it correctly and in order while the user was never offered the question, so the gate passed and the experience was identical to forgetting. | | Behavior verdict store (#446 cut 1) | `horizon.py`'s existing `horizon_contradiction()` judgment ↔ `review.py` `_horizon_markers_all` (the untruncated computation; `_horizon_markers` is only its `HORIZON_MARKER_LIMIT`-bounded slice for the card's attention budget — the limit is a *display* cap, never a storage cap) ↔ `review.py` `_horizon_verdict_rows` (assembled only from what `_build_plan` already stamped into the plan — `engine_state`, `state_snapshot.thesis_states`, `state_snapshot.recent_exits` — never re-derived from disk or from a second, differently-sourced `active_cycle_ids`) ↔ `engine/verdicts.py` (`build_horizon_verdict`, the pure `replay()` contract, `RULE_PARAMS`/`RULE_PARAM_DIGEST`) ↔ `schemas/behavior-verdict.schema.json` (no `input` sub-object — every field is engine-assigned, the same reason `condition-check.schema.json` refuses `user_response` on its own envelope) ↔ `session.py`'s projection appending to `/verdicts.jsonl`, the same firewall `conditions.py:66-72` gives `conditions.jsonl` — a verdict never enters `problems.check_rules`' mechanical reconciliation ↔ `coach.py`'s `DATA_FILES` ↔ `tests/test_verdicts.py` (+ `tests/test_review_v2.py::test_all_json_schemas_parse`, `tests/test_coach_data_cli.py`). A verdict recomputes to the same `outcome` from exactly its five frozen scalar fields — `replay(rule.id, rule.version, subject.value, observed.value, observed.closed)` — forever, including after the thesis row `subject.row_id` names is revised (a revision writes a new `event_id`; the old one, and every verdict pointing at it, is never rewritten) or a future profile label `profile_label_id` references is superseded (that field is a back-reference only, load-bearing for nothing; `replay`'s signature carries no parameter for a labels store at all). The one way replay could fail silently — a threshold edited without a version bump — is blocked mechanically: `RULE_PARAM_DIGEST` pins `RULE_PARAMS`'s hash as a literal (never recomputed live, which would make the check vacuous), and a companion test cross-checks `RULE_PARAMS`'s live version against `horizon.py`'s own `EXIT_FAST`/`HELD_LONG` constants, so an edit landing in either file is caught. | Date product assumptions when using them for prioritization. Reconfirm assumptions that are several weeks old or contradicted by new evidence. diff --git a/skills/fomo-kernel/engine/market_data.py b/skills/fomo-kernel/engine/market_data.py index f81555dd..f71b5ef7 100644 --- a/skills/fomo-kernel/engine/market_data.py +++ b/skills/fomo-kernel/engine/market_data.py @@ -92,14 +92,22 @@ "MarketDataError", "MarketDataBundle", "OFFLINE_ENV", + "FROZEN_ENV", "GAP_CODES", "network_allowed", + "frame_frozen", "build_request", "resolve", "reset_memo", ] OFFLINE_ENV = "TR_OFFLINE" +#: A pass that may only reuse the exact frame an earlier pass already resolved +#: for the identical request, and may never reach a provider. `review.py +#: add-cash` sets it: that command amends a review the user has already been +#: shown, so a second observation instant is not fresher data, it is a different +#: review wearing the same session (#665). +FROZEN_ENV = "TR_MARKET_DATA_FROZEN" CACHE_KIND = "market_data" #: Every degradation this module can report. Stable strings: a caller may branch @@ -114,6 +122,7 @@ "symbol_unpriced", # a requested symbol came back with no usable close "fx_unavailable", # a requested currency has no usable rate "feed_incomplete", # a supplied envelope does not cover what was asked + "frozen_frame_gone", # a frozen pass found no frame recorded for this request ) # One resolved bundle per normalized request, for this process only. The disk @@ -496,6 +505,54 @@ def network_allowed(env=None): return str(value).strip().lower() not in {"1", "true", "yes", "on"} +def frame_frozen(env=None): + """Whether this process may only reuse an already-resolved frame (#665). + + A posture, read here for the same reason :func:`network_allowed` is: it must + not be re-decided per route, or the next workflow inherits everything except + the one rule that mattered. ``resolve`` is the single consumer. + + The distinction from ``TR_OFFLINE`` is the point. Offline means *this machine + cannot reach a provider*, and the honest answer is a degraded review. Frozen + means *this pass must answer with the frame an earlier pass already resolved* + — the review is not being computed for the first time, it is being amended, + and closes that moved since the user read the card are not fresher facts but + a second review. Where offline degrades, frozen reuses. + """ + value = (env if env is not None else os.environ).get(FROZEN_ENV, "") + return str(value).strip().lower() in {"1", "true", "yes", "on"} + + +def _from_frozen_frame(request, *, root, today=None): + """The exact frame recorded for this request, or a stated absence. + + Exact key, not :meth:`MarketDataBundle.covers`. Coverage is the right rule + for the ordinary cache — it lets a narrower later request ride along free, + and it deliberately refuses to serve a request naming a symbol the stored + bundle failed to price, so a transient provider outage gets the retry it + deserves (#605 external review, finding 5). That refusal is exactly wrong + here, and it is what #665 was: one unpriced symbol anywhere in the universe + made every later pass on the same book re-resolve, `add-cash` re-priced the + whole review at a second instant, and the recompute's own moved numbers were + then reported to the user as "the facts moved". A pass amending a review does + not want a retry; it wants the frame the card it is amending was rendered + from, gaps and all. + + Never reaches the provider, on any outcome. A miss is stated as a gap and the + review degrades exactly as an unreachable provider would, which the caller's + own gate then refuses — the frame this session was built on being gone is a + real change of facts, not something to paper over with fresh closes. + """ + blob = fetch_cache.load(CACHE_KIND, request, root=root, today=today) + bundle = MarketDataBundle.from_json(blob) if blob is not None else None + if bundle is not None and bundle.usable: + return bundle + return _unavailable(request, [_gap( + "frozen_frame_gone", + "this pass may only reuse the frame already resolved for this request, " + "and no usable frame is recorded for it")]) + + def _provider_available(): """Whether :func:`_download` below could run at all — the other half of the provider seam, and module-level for exactly the same two reasons. @@ -769,7 +826,14 @@ def resolve(request, *, root, feed=None, today=None, env=None, memo=True): # TR_OFFLINE, which is precisely the equivalence this flag exists to # guarantee (external review, finding 6). if not network_allowed(env): + # Checked before the frozen posture, not after. A process that may not + # reach a provider has nothing to freeze, and routing it here keeps an + # offline run's degradation byte-identical whether or not the caller + # asked for a frozen frame — the gap code a review reports must describe + # the machine it ran on, not which subcommand invoked it. return _from_yahoo(request, root=root, today=today, env=env) + if frame_frozen(env): + return _from_frozen_frame(request, root=root, today=today) if memo: for bundle in _MEMO.values(): if bundle.covers(request): diff --git a/skills/fomo-kernel/engine/review.py b/skills/fomo-kernel/engine/review.py index 9ff18f5e..e188d843 100644 --- a/skills/fomo-kernel/engine/review.py +++ b/skills/fomo-kernel/engine/review.py @@ -1506,7 +1506,8 @@ def _overlay_ledger_holdings(card, state, derived, *, declared_anchor=True): # accumulation is FIFO, so `cost`/`avg_cost` move, and `origin`/`market`/ # `currency` appear. `add-cash` re-enters this exact pipeline to add an # anchor to a session the user has already answered against, and refuses - # when anything but the anchor moved; that guard is what caught this. + # when the recorded book underneath it moved; that guard is what caught + # this. # # Disagreement is the other case, and it is still adopted below: there # the supplied file did not cover the book, so the card's own view is not @@ -1733,8 +1734,20 @@ def _ingest_trades(root, paths, card, state): return result, card, state -def _verify_and_ingest_frozen_trades(root, inputs, batches, overlay, basis_receipt, card, state, *, append=True): - """Final short-lock gate for one frozen engine/PB transaction (#501).""" +def _verify_and_ingest_frozen_trades(root, inputs, batches, overlay, basis_receipt, card, state, *, + append=True, amending=False): + """Final short-lock gate for one frozen engine/PB transaction (#501). + + ``amending`` is a pass recomputing a review the user has already been shown + — today only ``cmd_add_cash``. Such a pass may find every one of its rows + already recorded and nothing else: a transaction file that grew since the + card was rendered is a different review, and the answers already given were + made against a book that no longer exists. Refused *here*, before the + append, because a refusal that lands after it has already written the rows + it is refusing (#665, maintainer disposition: "remains a refusal before any + write"). The dedup that decides it is the engine's own, not a second digest + with its own opinion about what counts as the same file. + """ frame = basis_receipt.get("valuation_frame") if not isinstance(frame, dict) or _frame_identity(frame) != basis_receipt["valuation_frame_identity"]: raise ReviewError("this review's price basis changed while it was being prepared; rerun prepare") @@ -1789,6 +1802,14 @@ def _verify_and_ingest_frozen_trades(root, inputs, batches, overlay, basis_recei if append and (derived_book.get("holdings") or declared_anchor): card, state, reconciliation = _overlay_ledger_holdings( card, state, derived_book, declared_anchor=declared_anchor) + if amending and verified_overlay["fresh"]: + raise ReviewError( + "the facts moved: this is not the anchor propagating into the account pillar — " + f"the transaction file has grown by {len(verified_overlay['fresh'])} row(s) since " + "this review's card was rendered, so the answers already given were made against " + "a book that no longer exists. Nothing was written. Start a fresh review with " + "prepare (adding --cash if you have the balance) rather than amending one the " + "user answered against different trades") if append and verified_overlay["fresh"]: ledger.append_events(ledger_path, verified_overlay["fresh"], recorded_at=state.get("date_end")) recorded_book = (_record_derived_book(root, ledger_path, verified_overlay["events"], state) @@ -1976,6 +1997,13 @@ def _run_engine(paths, root, args, *, ledger_path=None): env.pop("TR_PRICES", None) # only an explicit --prices may inject a price envelope if getattr(args, "prices", None): env["TR_PRICES"] = os.path.abspath(os.path.expanduser(args.prices)) + # #665: a pass that amends a review the user has already read may only + # reuse the frame that review was rendered from. Never a CLI flag — it is + # a property of the lane, set by `cmd_add_cash` alone, so no caller can + # ask an ordinary `prepare` to answer from an older instant. + env.pop(market_data.FROZEN_ENV, None) + if getattr(args, "amending_session", False): + env[market_data.FROZEN_ENV] = "1" previous = _previous_state(root) if previous and previous.get("date_end"): env["TR_PREV_END"] = str(previous["date_end"]) @@ -3832,8 +3860,8 @@ def _cash_anchor_status(state, route, cadence): + " — stating what it unlocks and that skipping keeps the holdings-only view. " "If they answer, run add-cash --session-id --cash '{\"currency\":\"\"," "\"amount\":,\"as_of\":\"\"}' and continue on the session it " - "returns; it reuses this session's frozen prices and refuses if anything but " - "the anchor moved. Never guess a balance")} + "returns; it reuses this session's frozen prices rather than fetching new ones, " + "and refuses if the facts underneath the card moved. Never guess a balance")} def _build_plan(card, state, engine_meta, root, paths, route, language, fingerprint, nonce, persist, @@ -4242,7 +4270,7 @@ def _prepare_session(args): elif frozen_transaction is not None: ledger_ingest, card, state = _verify_and_ingest_frozen_trades( root, frozen_transaction, batches, overlay, virtual_basis, card, state, - append=persist) + append=persist, amending=bool(getattr(args, "amending_session", False))) elif persist and paths: ledger_ingest, card, state = _ingest_trades(root, paths, card, state) finally: @@ -5629,15 +5657,6 @@ def cmd_resume(args): "next_action": "run resume with --session-id" if pending else "run prepare"}) -# What supplying a cash anchor is allowed to move, and nothing else may. These -# are exactly the keys a with/without-`--cash` pair differs in on the same book -# (measured, not assumed: `engine_state` differs only in `cash`; `engine_card` -# only in `cash`, `acct_perf`, and `honesty_ledger`). Anything outside them -# moving means the second run was not the same review — a re-priced book, an -# edited CSV, a changed ledger — and the user's answers would be silently -# re-based onto numbers nobody saw. -CASH_RECOMPUTE_STATE_KEYS = ("cash",) -CASH_RECOMPUTE_CARD_KEYS = ("cash", "acct_perf", "honesty_ledger") # Artifacts the recomputed session inherits verbatim. `answers`/`narrative` are # what the user already answered; `question-surfaces`/`question-presentations` # are the exact question bytes they were asked, frozen so an interrupted session @@ -5646,34 +5665,97 @@ def cmd_resume(args): CASH_RECOMPUTE_CARRIED = ("answers", "narrative", "question-surfaces", "question-presentations") - -def _without(mapping, keys): - return {key: value for key, value in (mapping or {}).items() if key not in keys} +# The facts underneath the card the user already read, which a cash anchor may +# not move — stated positively, and it is the direction that matters (#665). +# +# The predecessor asked the opposite question. It excluded a hand-maintained set +# of cash-derived keys from `engine_state` and `engine_card`, then compared +# `state_snapshot` and `question_queue` **whole**, and that shape cannot be kept +# true: the recompute re-enters the entire lane, so any downstream effect of the +# re-entry — including its own second market-data resolution — arrived as +# "drift". The command refused the exact session it exists for, and a card-beat +# cash answer became unrecordable. What the recompute owes is not that nothing +# moved; a cash anchor is *supposed* to move the account pillar. It owes that +# the facts the user's answers were made against are the same facts. So this +# names those facts, and compares nothing else. +# +# Two rules for adding a row. It must be an input or a frozen observation, never +# a projection of one — a projection is where a cash-derived value eventually +# appears, and that is the defect above rebuilt. And it must be measurably +# invariant under `--cash` on the same book: measured across five mock books, an +# anchor moves `engine_state.cash`, `engine_card`'s `cash`/`acct_perf`/ +# `honesty_ledger`, and `card_plan.required_honesty_keys`, and nothing else. + +# The valuation inputs the card was priced from. `add-cash` reuses them rather +# than re-resolving (see `cmd_add_cash`), so on the ordinary path these are +# identical by construction and this row is the proof, not the hope. +CASH_RECOMPUTE_FRAME_KEYS = ("price_snapshot", "price_provenance", "price_request", + "valuation_frame", "splits", "splits_window", + "currency_meta", "market_context") +# The recorded book and the window it was read over. An edited CSV, a different +# input file, or a ledger some other session moved in between lands here. +CASH_RECOMPUTE_BOOK_KEYS = ("holdings", "portfolio_structure", "metrics", + "date_start", "date_end", "prev_end", "n_trades", + "n_round_trips", "n_held", "insufficient_data", + "review_tier", "problem_events") +# What the behavior review found. The driver and instrument maps reach a review +# only through here, so a map supplied to this command but not to `prepare` is a +# refusal rather than a silently re-attributed card. +CASH_RECOMPUTE_DIAGNOSIS_KEYS = ("ticker_diagnosis", "top_holes", "dims_raw", + "overview", "strength", "payoff_attribution", + "alpha_beta_breakdown", "what_if", + "portfolio_structure", "prescriptions", + "thesis_questions", "vs_market_gate") + + +def _named(mapping, keys): + return {key: (mapping or {}).get(key) for key in keys} + + +def _question_identities(plan): + """Which questions were asked, without their wording. + + The identity, not the bytes. A queue row's text quotes this period's + numbers, so comparing it byte for byte makes the gate hostage to every + figure on the card — which is how `question_queue` ended up in a refusal + naming a surface a cash anchor cannot reach (#665). What has to hold is that + the user's recorded answers still map onto the questions this session asks, + and that is the id, the kind, and whether an answer was required. + """ + return [{"id": row.get("id"), "kind": row.get("kind"), "required": row.get("required"), + "ticker": row.get("ticker"), "cycle_id": row.get("cycle_id")} + for row in plan.get("question_queue") or []] + + +CASH_RECOMPUTE_SOURCE_FACTS = ( + ("the input files", lambda plan: (plan.get("input") or {}).get("paths")), + ("the valuation frame the card was priced from", + lambda plan: {"engine_state": _named(plan.get("engine_state"), + CASH_RECOMPUTE_FRAME_KEYS), + "price_feed": ((plan.get("input") or {}).get("price_feed") + or {}).get("provenance")}), + ("the recorded book", + lambda plan: _named(plan.get("engine_state"), CASH_RECOMPUTE_BOOK_KEYS)), + ("what the review diagnosed", + lambda plan: _named(plan.get("engine_card"), CASH_RECOMPUTE_DIAGNOSIS_KEYS)), + ("the positions with no recorded thesis", + lambda plan: plan.get("missing_thesis_positions")), + ("the rules the user was offered", + lambda plan: {"candidate_rules": (plan.get("card_plan") or {}).get("candidate_rules"), + "candidate_comparison": + (plan.get("card_plan") or {}).get("candidate_comparison")}), + ("the questions the user already answered", _question_identities), +) def _cash_recompute_drift(before, after): - """Everything the recompute moved that a cash anchor may not move. + """The source facts that moved under the recompute, as human-readable labels. - Returns a list of human-readable labels, empty when the two plans describe - the same review. The comparison is over the engine's own artifacts *and* - over the three agent-facing surfaces the user has already acted on, because - "nothing the user answered or chose was invalidated" is the claim this gate - exists to make true rather than assert. + Empty means the two plans were computed over the same facts — the anchor + propagated into the account pillar and nothing underneath it moved, which is + the outcome this command exists to produce rather than to refuse. """ - checks = ( - ("engine_state", lambda plan: _without(plan.get("engine_state"), - CASH_RECOMPUTE_STATE_KEYS)), - ("engine_card", lambda plan: _without(plan.get("engine_card"), - CASH_RECOMPUTE_CARD_KEYS)), - ("state_snapshot", lambda plan: plan.get("state_snapshot")), - ("question_queue", lambda plan: plan.get("question_queue")), - ("missing_thesis_positions", lambda plan: plan.get("missing_thesis_positions")), - ("card_plan.candidate_rules", - lambda plan: (plan.get("card_plan") or {}).get("candidate_rules")), - ("card_plan.candidate_comparison", - lambda plan: (plan.get("card_plan") or {}).get("candidate_comparison")), - ) - return [label for label, read in checks + return [label for label, read in CASH_RECOMPUTE_SOURCE_FACTS if session.canonical(read(before)) != session.canonical(read(after))] @@ -5690,16 +5772,29 @@ def cmd_add_cash(args): `prepare --cash` is a full second review: it re-resolves market data (a later instant, therefore different closes) and mints a session with none of the first one's frozen work. This command re-enters the same lane with the - anchor added and then **proves** it was the same review: every engine - artifact outside the cash keys must come back byte-identical, and so must - the question queue, the candidate rules and the missing-thesis list the user - has already acted on. In practice `market_data`'s same-day cache makes the - recompute a zero-request replay of the identical bundle, which is what makes - that gate pass rather than a coincidence; when it does not — a day boundary - crossed, an edited CSV, a ledger that moved — the command refuses and names - what drifted instead of re-basing the user's answers onto numbers nobody - saw. SKILL.md's "refetching live data would silently change the facts the - user already answered against" is the rule; this is its enforcement. + anchor added, and the two halves below are what make that a recompute of the + same review rather than a second one. + + **It reuses the frame instead of re-resolving it** (`frozen_market_frame`; + `market_data.frame_frozen`). The same-day cache was carrying that claim + before, and it could not: `MarketDataBundle.covers` refuses to serve a + request naming a symbol the stored bundle failed to price — deliberately, so + a transient outage gets a retry — so a single unpriced symbol anywhere in + the universe made this command re-price the entire review at a second + instant. Every downstream number then moved, and the recompute reported its + own movement to the user as facts moving underneath them (#665). Amending a + review does not want a retry; it wants the frame the card being amended was + rendered from. + + **And it proves the facts held rather than asserting it.** The source facts + the user's answers were made against — the input files, that frame, the + recorded book, the diagnosis, the rules they were offered, the questions + they answered — must come back identical. When they do not (an edited CSV, a + ledger another session moved, a driver map supplied here but not to + `prepare`, a frame no longer recorded) the command refuses and names what + moved, instead of re-basing answers onto numbers nobody saw. SKILL.md's + "refetching live data would silently change the facts the user already + answered against" is the rule; these two are its enforcement. A new session id is unavoidable and correct: the id is content-addressed from engine state and the anchor is part of that state. What must not change @@ -5737,6 +5832,12 @@ def cmd_add_cash(args): cash=args.cash, prices=args.prices, driver_map=args.driver_map, instrument_map=args.instrument_map, condition_checks=args.condition_checks, prices_unavailable=declared_unavailable, + # #665: this pass amends a review the user has already read, so the + # anchor is its only new input. Two enforcement points, one idea: the + # market frame is reused rather than re-observed (`_run_engine`), and a + # transaction file that grew is refused before anything is written + # (`_verify_and_ingest_frozen_trades`). + amending_session=True, snapshot_json=None, card_json=None, state_json=None, timeout=args.timeout) result = _prepare_session(rerun) if result["status"] == "already_committed": @@ -5751,13 +5852,17 @@ def cmd_add_cash(args): # than no recompute at all. shutil.rmtree(session.pending_dir(root, result["session_id"]), ignore_errors=True) raise ReviewError( - "adding the cash anchor changed more than the anchor; these no longer match the " - "session you are amending: " + ", ".join(drift) - + ". The answers already given were made against different facts, which is what a " - "re-priced book, an edited input file, or a ledger that moved in between looks " - "like. Pass the same --prices/--driver-map/--instrument-map this session was " - "prepared with; otherwise start a fresh review with prepare --cash rather than " - "re-basing answers onto numbers the user never saw") + "the facts moved: this is not the anchor propagating into the account pillar, " + "which is expected and allowed — it is that the review underneath it is no longer " + "the one the user answered. These no longer match the session you are amending: " + + ", ".join(drift) + + ". The answers already given were made against different facts, which is what an " + "edited input file, a ledger another session moved, or a driver/instrument map " + "supplied here but not to prepare looks like — and, when the frame itself moved, " + "a review whose frozen prices are no longer on record. Pass the same " + "--prices/--driver-map/--instrument-map this session was prepared with; otherwise " + "start a fresh review with prepare --cash rather than re-basing answers onto " + "numbers the user never saw") carried = {name: pending.get(name.replace("-", "_")) for name in CASH_RECOMPUTE_CARRIED} session.save_pending(root, result["session_id"], **carried) if result["session_id"] != args.session_id: @@ -5768,6 +5873,14 @@ def cmd_add_cash(args): recomputed_plan = (session.load_pending(root, result["session_id"]).get("plan") or {}) _emit({"status": "anchored", "session_id": result["session_id"], "superseded_session_id": args.session_id, + # #665: the gate has exactly two verdicts and both are stated. This is + # the allowed one — the anchor reached the account pillar and every + # fact underneath it held. The other is the refusal above, and the + # difference between them is what this command used to get wrong. + "recompute": {"outcome": "anchor_propagated", + "market_frame": "reused", + "source_facts_verified": [label for label, _read + in CASH_RECOMPUTE_SOURCE_FACTS]}, "review_plan": _plan_for_agent(recomputed_plan), "carried_forward": sorted(name for name, value in carried.items() if value is not None), "next_action": ( diff --git a/skills/fomo-kernel/references/data-contract.md b/skills/fomo-kernel/references/data-contract.md index f53b6dc2..ecd95e77 100644 --- a/skills/fomo-kernel/references/data-contract.md +++ b/skills/fomo-kernel/references/data-contract.md @@ -78,7 +78,7 @@ python3 engine/review.py add-cash --session-id \ --cash '{"currency":"USD","amount":8200,"as_of":""}' ``` -This re-enters the same review with the anchor added and reuses that session's frozen prices; it refuses outright if anything but the anchor moved, because the user answered against the card those numbers rendered. Answers, narrative, and frozen question surfaces carry over untouched; the returned session id supersedes the one you passed, and `card_plan.required_honesty_keys` gains the account-basis key, which needs one more sentence in `narrative.honesty`. Rerun `preview` on the returned session and show the recomputed card. If they skip, the card keeps its holdings pillar and its unlock invitation exactly as before, and the review finishes normally — skipping is a real answer, not a failure. +This re-enters the same review with the anchor added, and it reuses that session's frozen prices rather than resolving new ones — the user answered against the card those numbers rendered, so a second observation instant would be a different review wearing the same session (#665). Two outcomes, and the response names which: the anchor propagated into the account pillar (`recompute.outcome: "anchor_propagated"`), or the facts underneath it moved and the command refuses. What counts as moving is the source facts, not the anchor's own downstream effects: the input files, that frozen frame, the recorded book, the diagnosis, the rules offered, and the questions already answered. A transaction file that grew since the card was rendered is refused before anything is written at all. Answers, narrative, and frozen question surfaces carry over untouched; the returned session id supersedes the one you passed, and `card_plan.required_honesty_keys` gains the account-basis key, which needs one more sentence in `narrative.honesty`. Rerun `preview` on the returned session and show the recomputed card. If they skip, the card keeps its holdings pillar and its unlock invitation exactly as before, and the review finishes normally — skipping is a real answer, not a failure. On `first_review` and full-tier `weekly_review`, record which of the three happened (`cash_anchor_checked`, `references/ux-receipt.md`): `found_in_source` before the first question or card, `provided` or `declined` after the card the question was attached to. There is no outcome meaning "the agent decided not to ask" — that is what made a run where the user never got the chance look identical to one where they declined (#357, fifth recurrence). diff --git a/tests/test_market_data.py b/tests/test_market_data.py index 1c09d141..53807781 100644 --- a/tests/test_market_data.py +++ b/tests/test_market_data.py @@ -582,6 +582,120 @@ def test_e_an_open_ended_request_is_not_served_from_a_closed_window(): "stale close becomes today's weight") +# ────────────── E2. the frozen frame: amending, not re-observing ────────────── +# +# `covers` is the right rule for the ordinary cache and the wrong one for a pass +# that amends a review the user has already read. A caller in that position is +# not asking for the current market; it is asking for the frame the card it is +# amending was rendered from. #665: one unpriced symbol anywhere in the universe +# made `covers` refuse forever, so `review.py add-cash` re-priced the whole +# review at a second instant and then reported its own movement to the user as +# facts moving underneath them. + +FROZEN = {"TR_OFFLINE": "0", market_data.FROZEN_ENV: "1"} + + +def test_e2_a_frozen_pass_reuses_the_recorded_frame_and_asks_nothing(): + with provider() as p: + first = p.resolve(_request()) + market_data.reset_memo() # a different process has no memo + again = p.resolve(_request(), env=FROZEN) + assert len(p.calls) == 1, ( + f"a frozen pass must reach no provider at all; it made {len(p.calls) - 1} extra " + f"request(s): {p.calls[1:]}") + assert again.source == first.source and again.priced == first.priced, ( + f"the frozen pass must answer with the recorded frame: {again.source} {again.gaps}") + assert again.to_json() == first.to_json(), ( + "byte-identical is the whole claim — a card amended against anything else is a second " + "review wearing the first one's session id") + + +def test_e2_a_frozen_pass_reuses_a_frame_the_coverage_rule_would_refuse(): + """The #665 mechanism itself, at the level it happens. + + DEAD comes back present-but-empty, exactly as a delisted or misspelled symbol + does. `covers` then refuses to serve this request from its own stored bundle + — deliberately, so a transient outage gets its retry — and every later pass + on the same book re-resolves. That retry is right for a fresh review and + wrong for one being amended, and the difference is the posture, not the + bundle. + """ + request = _request(instruments=["NVDA", "DEAD"], benchmarks=[], currencies=[]) + with provider() as p: + stored = p.resolve(request) + assert not stored.covers(request), ( + "precondition: an unpriced symbol is exactly what makes the ordinary cache refuse " + "to serve the request that produced it") + market_data.reset_memo() + ordinary = p.resolve(request) + assert len(p.calls) == 2, ( + "precondition: without the frozen posture this request re-resolves every time — " + f"that is the bug, and it must still be reproducible here: {p.calls}") + assert ordinary.source == "yahoo" + market_data.reset_memo() + frozen = p.resolve(request, env=FROZEN) + assert len(p.calls) == 2, ( + f"the frozen pass must not have added a third request: {p.calls}") + assert frozen.to_json() == stored.to_json(), ( + "the frozen pass must hand back the same frame, gaps and all: a review is amended " + f"against what it was computed from, not against a better answer: {frozen.gaps}") + + +def test_e2_a_frozen_pass_with_no_recorded_frame_states_it_and_still_asks_nothing(): + """Fail-closed, and silent on the wire either way. + + The frame being gone — a day boundary, a reset root, a session prepared + somewhere else — is a real change of facts, so it degrades exactly as an + unreachable provider does and the caller's own gate refuses. What it must + never do is quietly fetch a fresh one: that is the failure this posture + exists to remove, and 'no frame on record' is the moment it would be most + tempting. + """ + with provider() as p: + bundle = p.resolve(_request(), env=FROZEN) + assert not p.calls, f"a frozen pass must never reach the provider: {p.calls}" + assert not bundle.usable and bundle.source == "unavailable", bundle.source + assert [gap["code"] for gap in bundle.gaps] == ["frozen_frame_gone"], bundle.gaps + + +def test_e2_a_frozen_pass_does_not_read_a_different_days_frame(): + with provider() as p: + p.resolve(_request()) + market_data.reset_memo() + tomorrow = p.resolve(_request(), env=FROZEN, today=NEXT) + assert not tomorrow.usable, ( + "yesterday's closes must not be served as this session's frozen frame; a day boundary " + f"is a changed price frame and the caller refuses on it: {tomorrow.source}") + assert len(p.calls) == 1, f"and it must still ask nothing: {p.calls}" + + +def test_e2_an_offline_frozen_pass_degrades_exactly_as_offline_alone_does(): + """The posture order, which is load-bearing in the unglamorous direction. + + A machine that may not reach a provider has nothing to freeze. If the frozen + branch ran first, an offline review would report `frozen_frame_gone` where an + identical offline review reports `network_disabled` — two runs on the same + machine describing it differently because of which subcommand invoked them, + and a gate comparing the frame would then refuse a session for a reason that + is not about the market at all. + """ + with provider() as p: + offline = p.resolve(_request(), env={"TR_OFFLINE": "1"}) + both = p.resolve(_request(), env={"TR_OFFLINE": "1", market_data.FROZEN_ENV: "1"}) + assert not p.calls, f"neither may reach the provider: {p.calls}" + assert both.to_json() == offline.to_json(), ( + f"offline degradation must not depend on the frozen posture: " + f"{[g['code'] for g in both.gaps]} vs {[g['code'] for g in offline.gaps]}") + + +def test_e2_the_frozen_posture_is_read_in_one_place_and_declared(): + assert "frozen_frame_gone" in market_data.GAP_CODES, \ + "a gap code must be declared before it can be emitted" + assert market_data.frame_frozen({market_data.FROZEN_ENV: "1"}) + for spelling in ("0", "", "no", "off"): + assert not market_data.frame_frozen({market_data.FROZEN_ENV: spelling}), spelling + + # ────────────────────────── F. supplied envelope ────────────────────────── def _envelope(path, closes, fx=None, splits=None, as_of="2026-07-29"): @@ -1060,6 +1174,30 @@ def test_h_the_offline_posture_has_exactly_one_reader(): "posture cannot be re-decided per route.") +def test_h_the_frozen_posture_has_exactly_one_reader_too(): + """Same rule, same reason, for #665's posture. + + `review.py` exports the variable through `market_data.FROZEN_ENV` rather than + a literal of its own precisely so this holds: a second module spelling the + name out is a second policy about when a review may be re-observed, and the + whole point of the posture is that no route gets to decide that for itself. + """ + import ast + readers = {} + for name, path in _engine_modules(): + with open(path, encoding="utf-8") as handle: + source = handle.read() + if market_data.FROZEN_ENV not in source: + continue + tree = ast.parse(source, filename=path) + for node in ast.walk(tree): + if isinstance(node, ast.Constant) and node.value == market_data.FROZEN_ENV: + readers.setdefault(name, []).append(node.lineno) + assert set(readers) == {"market_data.py"}, ( + f"{market_data.FROZEN_ENV} is spelled out outside the resolver: {readers}. Reach it " + "through market_data.FROZEN_ENV instead.") + + def test_i_the_resolver_never_picks_a_state_root_for_its_caller(): """#627: `root` is required, so no call site can inherit someone else's. diff --git a/tests/test_review_v2.py b/tests/test_review_v2.py index 6cb1e586..6dfad47e 100644 --- a/tests/test_review_v2.py +++ b/tests/test_review_v2.py @@ -2051,7 +2051,12 @@ def test_add_cash_refuses_when_more_than_the_anchor_moved(): "--cash", '{"currency":"USD","amount":8200,"as_of":"2026-07-30"}', env=env) assert run.returncode != 0, run.stdout error = json.loads(run.stdout)["error"] - assert "changed more than the anchor" in error and "engine_state" in error, error + # The refusal names which of the gate's two verdicts it reached (#665). + # "the facts moved" is the one that refuses; the other -- the anchor + # propagating into the account pillar -- is what this command is for, and + # naming only "something changed" is what made the two indistinguishable. + assert "the facts moved" in error, error + assert "the valuation frame the card was priced from" in error, error assert sorted(os.listdir(pathlib.Path(root) / ".pending")) == [plan["session_id"]], \ "a refused recompute must leave no anchored, finalizable session behind" @@ -2077,6 +2082,268 @@ def test_add_cash_carries_a_declared_price_dead_end_forward(): assert "reachable from this host" in recovery["checked"], recovery +# ── the card-beat answer has to be recordable on a priced review (#665) ── +# +# Everything above runs price-degraded, and that is why it stayed green through +# the defect: with nothing retrieved there is no second retrieval to disagree +# with the first. On a real user's review the engine fetches, and `add-cash` +# fetched again — a second observation instant, therefore different closes, +# therefore every derived number moved, and the recompute reported its own +# movement as "the facts moved". The user answered the question the card asked +# them and the answer could not be recorded (#665). +# +# The provider below is what makes that reproducible offline. Two properties, +# both taken from the real thing: it answers with different closes every pass +# (`market_data`'s own docstring records ^VIX moving between two calls seconds +# apart), and one benchmark comes back unpriced, which is what makes the +# same-day cache refuse to serve the request that produced it. + +_MOVING_PROVIDER = ''' +# Injected as usercustomize (never sitecustomize -- Homebrew ships its own and +# shadowing it removes site-packages), so the real CLI subprocess runs against a +# deterministic stand-in for a live market. +import datetime as dt +import os +import sys +sys.path.insert(0, os.environ["ENGINE_DIR"]) + +UNPRICED = {"XLY"} # present-but-empty, exactly as a dead symbol returns + + +def _fake_download(symbols, start, end=None): + import pandas as pd + log = os.environ["PROVIDER_LOG"] + with open(log, "a") as handle: + handle.write(",".join(sorted(symbols)) + "\\n") + with open(log) as handle: + nth = len([line for line in handle if line.strip()]) + days = [dt.date(2026, 7, 27), dt.date(2026, 7, 28), dt.date(2026, 7, 29)] + index = pd.DatetimeIndex([dt.datetime(d.year, d.month, d.day) for d in days]) + data = {} + for symbol in symbols: + seed = sum(ord(c) for c in symbol) + # The nudge is the whole point: a second pass is a second instant. + base = 20.0 + (seed % 400) + 0.37 * (nth - 1) + if symbol in UNPRICED: + data[("Close", symbol)] = [float("nan")] * len(days) + elif symbol.endswith("=X"): + data[("Close", symbol)] = [32.0 + 0.01 * (nth - 1)] * len(days) + else: + data[("Close", symbol)] = [base - 1.0, base - 0.5, base] + data[("Stock Splits", symbol)] = [float("nan")] * len(days) + frame = pd.DataFrame(data, index=index) + frame.columns = pd.MultiIndex.from_tuples(frame.columns) + return frame + + +import market_data +market_data._download = _fake_download +# The other half of the provider seam. Without it `_from_yahoo` answers +# `provider_missing` above the fake and never calls it -- which is every CI +# runner, none of which install yfinance (#621). +market_data._provider_available = lambda: True +''' + + +def _priced_env(tmp): + """A CLI environment that really resolves market data, against a live-like fake.""" + sitedir = pathlib.Path(tmp) / "provider-site" + sitedir.mkdir(exist_ok=True) + (sitedir / "usercustomize.py").write_text(_MOVING_PROVIDER, encoding="utf-8") + env = dict(os.environ) + env.pop("TR_OFFLINE", None) # the point here is resolution, not degradation + env["ENGINE_DIR"] = str(ENGINE_DIR) + env["PROVIDER_LOG"] = str(pathlib.Path(tmp) / "provider.log") + env["PYTHONPATH"] = os.pathsep.join( + [str(sitedir), str(ENGINE_DIR), env.get("PYTHONPATH", "")]).rstrip(os.pathsep) + return env + + +def _provider_calls(env): + try: + with open(env["PROVIDER_LOG"], encoding="utf-8") as handle: + return [line.strip() for line in handle if line.strip()] + except OSError: + return [] + + +def _prepared_on_a_priced_review(tmp, root, csv=None): + env = _priced_env(tmp) + run = _run("prepare", csv or _OFFLINE_MOCK, "--root", root, "--language", "en", env=env) + assert run.returncode == 0, run.stdout + run.stderr + plan = json.loads(run.stdout)["review_plan"] + assert plan["input"]["price_feed"]["provenance"]["mode"] == "engine_fetch", \ + f"fixture must actually be engine-priced: {plan['input']['price_feed']}" + assert plan["input"]["cash_anchor"]["status"] == "absent", plan["input"]["cash_anchor"] + return env, plan + + +def _preview(root, plan, tmp, env, tag): + answers = pathlib.Path(tmp) / f"answers-{tag}.json" + answers.write_text(json.dumps(_answers_for_plan(plan)), encoding="utf-8") + narrative = pathlib.Path(tmp) / f"narrative-{tag}.json" + narrative.write_text(json.dumps(_narrative_for_plan(plan)), encoding="utf-8") + return _run("preview", "--root", root, "--session-id", plan["session_id"], + "--answers", answers, "--narrative", narrative, env=env) + + +def test_the_card_beat_cash_answer_is_recordable_on_an_engine_priced_review(): + """#665's owning outcome, walked the way the product prescribes it. + + prepare -> the card is previewed and shown -> the user answers the cash + question that card asked -> add-cash -> the amended card. Every step is the + real CLI, the prices are the engine's own, and the provider moves between + passes because a real one does. + + The load-bearing assertion is the request count. The amended card must be + priced from the frame the user was reading, and the only way to know that is + that the recompute asked for nothing: a second resolution that happened to + agree would prove nothing, and one that disagreed is the defect. + """ + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) / "coach" + env, plan = _prepared_on_a_priced_review(tmp, root) + frozen_frame = json.loads( + (pathlib.Path(session_engine.pending_dir(str(root), plan["session_id"])) + / "plan.json").read_text(encoding="utf-8"))["engine_state"]["price_snapshot"] + + shown = _preview(root, plan, tmp, env, "first") + assert shown.returncode == 0, shown.stdout + shown.stderr + assert json.loads(shown.stdout)["private_card"], "the card the user is asked at" + calls_before = len(_provider_calls(env)) + assert calls_before >= 1, "the review must really have fetched its own prices" + + added = _run("add-cash", "--root", root, "--session-id", plan["session_id"], + "--cash", '{"currency":"USD","amount":0,"as_of":"2026-07-29"}', env=env) + assert added.returncode == 0, ( + "a declared balance answered at the card beat must be recordable — this is the " + f"session the command exists for:\n{added.stdout}{added.stderr}") + out = json.loads(added.stdout) + assert out["recompute"]["outcome"] == "anchor_propagated", out["recompute"] + assert len(_provider_calls(env)) == calls_before, ( + "the recompute re-resolved the market. The user is being shown an amended version " + "of a card they already read; a second observation instant is a different review " + f"wearing the same session: {_provider_calls(env)[calls_before:]}") + + amended = session_engine.load_pending(str(root), out["session_id"])["plan"] + assert amended["engine_state"]["price_snapshot"] == frozen_frame, ( + "the amended card must be priced from the same frame the user was reviewing") + assert amended["input"]["cash_anchor"] == {"status": "anchored", "source": "anchored"} + acct = amended["engine_card"]["acct_perf"] + assert acct.get("acct_twr") is not None and acct.get("cash_drag") is not None, ( + f"the account pillar is what answering buys; it is still empty: {acct}") + assert amended["engine_card"]["cash"]["source"] == "anchored" + + # And the amended card renders, which is the beat the user actually gets. + again = _preview(root, amended, tmp, env, "second") + assert again.returncode == 0, again.stdout + again.stderr + assert json.loads(again.stdout)["private_card"] + + +def test_a_source_that_really_moved_still_refuses_and_leaves_nothing_behind(): + """The other half, and the one that keeps the fix from being a rubber stamp. + + An input file edited between the card and the answer is not the anchor + propagating — it is a different review, and the answers already given were + made against numbers that no longer exist. Refused before any write: no + anchored pending session to finalize, and a ledger byte-identical to the one + the refusal started with. + """ + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) / "coach" + csv = pathlib.Path(tmp) / "trades.csv" + csv.write_text(_OFFLINE_MOCK.read_text(encoding="utf-8"), encoding="utf-8") + env, plan = _prepared_on_a_priced_review(tmp, root, csv=csv) + ledger_path = pathlib.Path(root) / "ledger.jsonl" + ledger_before = ledger_path.read_bytes() + + with open(csv, "a", encoding="utf-8") as handle: + handle.write("TSLA,5,300.00,BUY,BOUGHT TESLA,2024-12-04,2024-12-06,0," + "-1500.00,0,0,,Trade\n") + + run = _run("add-cash", "--root", root, "--session-id", plan["session_id"], + "--cash", '{"currency":"USD","amount":8200,"as_of":"2026-07-29"}', env=env) + assert run.returncode != 0, run.stdout + assert ledger_path.read_bytes() == ledger_before, ( + "the refusal must land before any write. The recompute re-enters the ingest, so a " + "gate that only reads the recomputed plan has already appended the rows it is about " + "to refuse -- and the user's next command would then run against a book neither " + "review was computed over") + error = json.loads(run.stdout)["error"] + assert "the facts moved" in error, error + assert "the transaction file has grown by 1 row(s)" in error, ( + f"the refusal must name which fact moved, not only that something did: {error}") + assert sorted(os.listdir(pathlib.Path(root) / ".pending")) == [plan["session_id"]], \ + "a refused recompute must leave no anchored, finalizable session behind" + pending = pathlib.Path(session_engine.pending_dir(str(root), plan["session_id"])) + assert not list(pending.glob("card-*")), \ + f"and no card artifact for a review nobody accepted: {list(pending.iterdir())}" + + +def test_a_frame_no_longer_on_record_refuses_instead_of_fetching_a_fresh_one(): + """The fail-closed half of reusing the frame, and the more tempting failure. + + The frame this session was built on can genuinely be gone — a day boundary, + a reset root, a session prepared on another machine. Quietly resolving a new + one would look like a fix and would be the original defect: the user would be + handed an amended card priced at an instant they never saw, with the account + pillar computed against it. So the recompute asks for nothing, the review + degrades, and the gate refuses naming the frame. + """ + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) / "coach" + env, plan = _prepared_on_a_priced_review(tmp, root) + ledger_before = (pathlib.Path(root) / "ledger.jsonl").read_bytes() + calls_before = len(_provider_calls(env)) + shutil.rmtree(pathlib.Path(root) / "cache") # the frame is no longer on record + + run = _run("add-cash", "--root", root, "--session-id", plan["session_id"], + "--cash", '{"currency":"USD","amount":8200,"as_of":"2026-07-29"}', env=env) + assert run.returncode != 0, run.stdout + error = json.loads(run.stdout)["error"] + assert "the facts moved" in error, error + assert "the valuation frame the card was priced from" in error, error + assert len(_provider_calls(env)) == calls_before, ( + "the recompute went and fetched a replacement frame. That is the defect wearing a " + f"repair: {_provider_calls(env)[calls_before:]}") + assert sorted(os.listdir(pathlib.Path(root) / ".pending")) == [plan["session_id"]] + assert (pathlib.Path(root) / "ledger.jsonl").read_bytes() == ledger_before + + +def test_replaying_the_same_cash_anchor_changes_nothing(): + """Idempotence, at the level the user can retry at. + + An agent that loses the response and re-runs the command must not produce a + second review, a second ledger row, or a different answer. The anchored + session is already anchored, so the replay is refused with a stable message + and the session on disk is byte-identical afterwards. + """ + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) / "coach" + env, plan = _prepared_on_a_priced_review(tmp, root) + anchor = '{"currency":"USD","amount":1200,"as_of":"2026-07-29"}' + added = _run("add-cash", "--root", root, "--session-id", plan["session_id"], + "--cash", anchor, env=env) + assert added.returncode == 0, added.stdout + added.stderr + anchored_id = json.loads(added.stdout)["session_id"] + pending = pathlib.Path(session_engine.pending_dir(str(root), anchored_id)) + before = {path.name: path.read_bytes() for path in sorted(pending.iterdir())} + ledger_before = (pathlib.Path(root) / "ledger.jsonl").read_bytes() + calls_before = len(_provider_calls(env)) + + replay = _run("add-cash", "--root", root, "--session-id", anchored_id, + "--cash", anchor, env=env) + assert replay.returncode != 0, replay.stdout + assert "already carries a cash anchor" in json.loads(replay.stdout)["error"] + assert {path.name: path.read_bytes() for path in sorted(pending.iterdir())} == before, \ + "a replay must not rewrite the session it is replaying" + assert (pathlib.Path(root) / "ledger.jsonl").read_bytes() == ledger_before + assert len(_provider_calls(env)) == calls_before, \ + "and it must not reach the market on the way to refusing" + assert sorted(os.listdir(pathlib.Path(root) / ".pending")) == [anchored_id], \ + "no second pending session survives the replay" + + def test_add_cash_refuses_a_session_that_never_takes_an_anchor(): """A snapshot states cash inline in its own envelope, so there is no second place to supply one -- and the plan already says `not_applicable`. The diff --git a/tests/test_split_basis.py b/tests/test_split_basis.py index bcf3690e..00c1ba3e 100644 --- a/tests/test_split_basis.py +++ b/tests/test_split_basis.py @@ -531,14 +531,17 @@ def test_the_split_map_changes_the_books_own_identity(): _ROUTES_NOT_DRIVEN = { # `add-cash` (#357) performs no book read of its own: it re-enters # `_prepare_session` — the `prepare` route driven above — with one extra - # input, and then refuses unless the recomputed `engine_state` (holdings, - # splits, valuation frame: everything but `cash`) is byte-identical to the - # session it is amending. Whatever `prepare` establishes about the split - # basis, this route reproduces exactly or does not answer at all, so there - # is no second basis for it to get wrong. This is a mechanism, not a - # call-graph argument: it is pinned by - # tests/test_review_v2.py::test_add_cash_refuses_when_more_than_the_anchor_moved. - "add-cash": "delegates to prepare and refuses any engine_state drift outside cash", + # input, and it does so on a frozen market frame, so the splits and the + # valuation frame it computes over are the ones `prepare` already resolved + # rather than a second resolution (#665). It then refuses unless the source + # facts come back identical, `the valuation frame the card was priced from` + # — holdings, splits, price snapshot, valuation frame — among them. Whatever + # `prepare` establishes about the split basis, this route reproduces exactly + # or does not answer at all, so there is no second basis for it to get + # wrong. This is a mechanism, not a call-graph argument: it is pinned by + # tests/test_review_v2.py::test_add_cash_refuses_when_more_than_the_anchor_moved + # and ::test_a_frame_no_longer_on_record_refuses_instead_of_fetching_a_fresh_one. + "add-cash": "delegates to prepare on a frozen frame and refuses any source-fact drift", }