diff --git a/docs/maintainer-guide.md b/docs/maintainer-guide.md index c25e8725..2af602bf 100644 --- a/docs/maintainer-guide.md +++ b/docs/maintainer-guide.md @@ -139,14 +139,14 @@ When multiple sessions are active: | Pre-trade evaluation (Layer 2, `review.py consider`) | `schemas/trade-evaluation.schema.json` (`premise` is `$ref`-ed straight from `trade-premise.schema.json`, `context` from `schemas/decision-context.schema.json`, and `agent_case` from `schemas/answer-provenance.schema.json` — never restated) ↔ `review.py`'s `cmd_consider` / `_rows_from_ledger` (the ledger-reconstruction fallback for a caller with no CSV in hand) / `_evaluation_id` / `_append_evaluation_row` / `_validate_decision_context` / `_validate_agent_case` ↔ `engine/answer_provenance.py`'s `validate_agent_case` (#414; the semantic gate `cmd_consider` calls after `consequence`/`rule_collisions` exist and before either is persisted or returned — #479 Wave B) ↔ `coach.py`'s `DATA_FILES` (so `data-export`/`data-reset` see `trade_evaluations.jsonl` like every other stored file — #452 is exactly the bug that shipped when a file skipped this step) ↔ `references/trade-consequence.md` ↔ `tests/test_consider.py`. Every stored field is a frozen value, never a pointer into mutable state the ledger keeps growing past — the frozen-subject design ratified in issue #446's specification comment. `--resolve` appends a new row carrying the same `evaluation_id` rather than rewriting the old one; `_fold_evaluations` (mirroring `conditions.fold_slots`'s supersede-by-chain precedent) is the only reader that decides which row is current. `review.CONSIDER_DECISIONS` and the schema's `decision` enum are locked together by a drift test in `tests/test_consider.py`, the same discipline the question-kind enum row above holds; so are `EVALUATION_EVIDENCE_REFS_CAP` / `EVALUATION_CONTEXT_TEXT_MAX` / `EVALUATION_EVIDENCE_REF_MAX` and the context schema's own `maxItems`/`maxLength`. Three rules the DecisionContext (#479 Wave A) adds. **Absent means absent**: `_evaluation_id` omits the `context` key from its seed and `cmd_consider` omits it from the row on the identical `is not None` test — a seed carrying `"context": null` moves the hash of *every* context-free call, so an existing user's next plain re-ask mints a new id instead of converging on the row already on disk, duplicating it, with nothing else in the suite pinning an id value. `test_a_context_free_evaluation_id_is_exactly_what_it_was_before_context_existed` pins the literal digest this function returned at `main@52df7f9`; that value must never move. **Identity, never arithmetic**: the context seeds the id so the same premise re-asked with a different `why_now` is a distinct evaluation, and `consequence`/`rule_collisions` are computed from the premise and the book before the seed is taken — the paired test asserts both halves at once, because seeding on the context is only legitimate while the arithmetic is byte-identical. **Every bound refuses, none truncates**: a shortened reason or a clipped evidence list is a statement the user never made, so `_validate_decision_context` raises with the limit named rather than repairing the envelope. `linked` stays out of `CONSIDER_DECISIONS` and out of the `decision` enum — that tuple is `--decision`'s argparse `choices`, so admitting it would let a user assert a link the engine never made (#490 derives link status at read time from its own stream and never writes it back here). Two rules #479 Wave B's provenance-gate integration adds, on top of #414's already-frozen validator. **The gate reads exactly what gets stored, and reads it once**: `cmd_consider` calls `validate_agent_case` with the same `consequence_stored`/`collisions` objects the row itself carries, never a separately assembled copy, and passes `user_statements=(context["reason"], context["why_now"])` — the exact, unparaphrased strings, never a summary — when a `--decision-context` was supplied, `()` otherwise; `agent_case` still never enters `_evaluation_id`'s seed, matching the reasoning the `context` row above already states: it is the agent's interpretation, not the subject being evaluated. `_validate_agent_case` (the cheap structural precheck that still runs first) was narrowed to require only that a claim carry `claim`/`provenance`, not *exactly* those two fields — the full field set is provenance-dependent (`anchor`/`worsens` for `engine_fact`, `source`/`as_of` for `public_fact`) and is now checked in exactly the one place that also enforces it, `answer_provenance.py`. **Reconciling the two claim shapes was Wave B's call, made once, not left open**: `trade-evaluation.schema.json`'s `agent_case` property is a bare `$ref` to `answer-provenance.schema.json` rather than a second, narrower claim `$defs` of its own — the old inline shape (`additionalProperties: false`, only `claim`/`provenance`) could not express what `engine_fact`/`public_fact` claims are required to carry at all, so leaving it unreconciled would have made both of those provenance kinds permanently unusable through the CLI even after the validator was wired in. What `consequence.DISCLOSURES` may contain is settled in one place and read three times (#598/#599/#600): the constant itself, `trade-evaluation.schema.json`'s stored enum, and `evaluation-challenge.schema.json`'s live one, locked by two drift tests in `tests/test_consider.py`. The stored enum is `DISCLOSURES` **plus** `RETIRED_DISCLOSURES` and nothing else — a key no path emits stays valid on a row written before it stopped being emitted, the replay posture #549's `declared_partial` already has — while the challenge enum is `DISCLOSURES` alone, because that block is emitted fresh and never stored, so offering a retired key there would tell an agent it may owe a disclosure nothing can produce. Three rules about what the keys themselves are for. **A blind spot is disclosed with its size, never only its existence**: `unclassified_book`/`etf_not_decomposed` name the *book's* illegible positions where `unmapped_driver` had only ever looked at the premise's own ticker, and each carries an identity list beside it (`unclassified_holdings`, `undecomposed_etfs`) on #515's exact division of labour — the key says THAT the concentration figures were measured over part of the book, the list says WHICH positions and at what weight. Both feed `answer_provenance.required_coverage`, which reads `disclosures` generically, so adding a key is what makes an answer owe the fact; the enforcement is free and the naming is the whole decision. The lists are stamped by `consequence.book_legibility`, called from `portfolio_state` rather than from `consequence()`, so a state carries its concentration readings and the positions those readings were measured *without* in the same dict — `consequence()` forwards `after`'s pair instead of deriving a second one, and `review._canonical_consider_before` is the only other call site, recomputing because it is the one place a state's denominator is replaced after the state was built. Placing this in `consequence()` alone was the shipped defect: a probe workflow that read the book and printed `ai_pct` received the reading with none of its limitations, offline suite green — the same failure shape as a reader of the book not being handed the split map, and fixed the same way. **The two lists are disjoint and the split is the remedy, not tidiness**: a fund the instrument map does not recognize is an unclassified single name, where `--driver-map` is the fix, and declaring it a fund moves it to the limitation that is true, where no fix exists yet — naming one position twice would point the user at a remedy that cannot work. Nothing here is look-through; #599 still owns prorating a fund's constituents across sector/AI buckets. **A currency gap is not a disclosure at all**: `portfolio_state` refuses a book whose held currency has no rate, because `usd_view` resolves a missing one as 1.0 and a ~31:1 currency summed at face value does not make the aggregate incomplete, it inverts which holding is the largest — AGENTS.md boundary 6's fail-closed rule, and #497's canonical-lane treatment finally reaching the legacy CSV lane that never had it. `_canonical_consider_before` converts that `ConsequenceError` rather than letting it escape, and `portfolio_state` carries no `fx_gaps` companion any more: past the refusal it could only ever be empty. One rule about the price day the basis freezes (#618). **A date is recorded only where one was observed, per instrument, and the summary is derived from the instruments rather than declared over them**: `basis.price_observations` is stamped by `review._price_observation_record` from the parsed feed's own `observed_date`, scoped to the same `priced_universe` the recovery kit is built over, and is *absent* — never null, never a placeholder, never today's — whenever `valuation_basis` is `unpriced`, which is what keeps an offline answer from growing a date it never had. Its `as_of` is `max(by_ticker)` and not `feed["as_of"]`: the envelope's declared frame date is an upper bound `price_feed.parse` enforces on its rows, so a supplied envelope may legitimately declare today over yesterday's closes, and copying it forward would put the summary ahead of every number it summarizes — the same confusion at frame level that per-ticker observation exists to prevent (#583 §2), and the definition `market_data.to_price_feed_envelope` already takes for its own `as_of`. A priced call's `evaluation_id` moves because of this field and that is correct — the id seeds on the frozen answer and the answer genuinely differs — while a context-free *unpriced* call's must not, which `test_a_context_free_evaluation_id_is_exactly_what_it_was_before_context_existed`'s literal digest is what pins. | | Pre-trade evaluation → review reconciliation | `review.py`'s `_evaluation_reconciliation` (calls `_fold_evaluations`, and reads real dated trade events through `_ledger_trade_events` — **never** `_rows_from_ledger`, whose synthesized anchor row raises on a position declared with shares alone and would break `prepare` for an ordinary snapshot user) ↔ `evaluation_reconciliation` in `schemas/review-plan.schema.json` (declared, `additionalProperties: false` at every level, deliberately not dropped into schema-open `state_snapshot`; optional rather than `required`, matching the `engine_version`/`authoring_contract` replay-compatibility precedent) ↔ `EVALUATION_RECONCILE_CAP` and its `summary.beyond_cap` disclosure ↔ `references/trade-consequence.md` ↔ `tests/test_consider.py`. Two invariants. **The engine states a fact, never a cause**: a matching trade may be reported, but `decision` is the user's word and moves only through `consider --resolve` — an engine-written decision would manufacture an adjudication nobody made, the prohibition `condition-check.schema.json` states about `user_response`. **A capped list discloses what it dropped**, or a bounded read reads as a complete one. This is the consumer that keeps `trade_evaluations.jsonl` from being a written-never-read store (#429). **It is no longer the only reader outside `consider`**, and the second one deliberately shares none of its filters: `_evaluation_recall` (#636) reads the same store to quote the user's own `context` words back into the `initial_thesis` question stem, and ignores `decision` entirely — a resolved evaluation is still something the user said, so this row's open-only scope is the wrong one for recall, and this surface's own schema declares itself a fact surface rather than a question. Its bounds live with it: `_cycle_entry` matches `trade_recap.CYCLE_ID_RE` instead of splitting on `#`, and `_recalled_entry_statement` fails closed on any cycle after the first, because a cycle id carries no lower bound and a re-entry would otherwise quote the previous position's reason. A third reader of this store must state which of those two filter sets it takes and why. | | 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` — #485 Slice C, #531) | `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.build_adoption` 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. 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. | +| 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. | | 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. | | Whether a held currency may enter a cross-currency aggregate (#612) | `trade_recap.held_currency_fx_gaps` is the single predicate, and `trade_recap.usd_view` — the one place amounts are converted before every aggregate reader (weights, concentration, rankings, `what_if`, the diagnosis order, the persisted reconciliation metrics) — is where it refuses. Putting it in the primitive rather than at each consumer is what makes a lane added later inherit it: #602 had already made `consequence.portfolio_state` refuse the same book, and the review lane never inherited that, which is the whole defect. `consequence` now reads the predicate instead of restating it, and the identity factor it replaced was not an approximation but a different number — one ~840,000 TWD position read as ~97% of a book that is really ~47% of it. Two properties are load-bearing and each has its own compatibility test: a **single-currency** book, including a pure non-USD one, aggregates itself self-consistently and is never a gap; and the predicate's domain is `cur_map`, which holds only *held* currencies, so the display currency `fx_request_currencies` widens the request with can never trigger it — a missing display rate stays a rendering degradation. The refusal reaches the CLI as one controlled error raised upstream of the card, the state write and the ledger ingest, quoting #611's machine-readable `fx_unavailable` gap code rather than parsing provider prose. Its readers: `references/price-feed.md`'s `fx` field rule (the repair the message names) ↔ `tests/test_engine_units.py`'s refusal and compatibility pair ↔ `tests/test_review_v2.py`'s four-state CLI walk ↔ `tests/test_consider.py` section on the pre-trade lane. The harness half: a mixed-currency persona has no card at all without a rate, so `tests/persona_sweep.py`'s `persona_prices` supplies the committed `mock/sample_.prices.json` beside it — the product's own repair path, not a harness-only bypass — and `trade_recap`'s `data_integrity["fx_gaps"]` producer is gone because after the refusal its only possible value was empty (#429); the key still lives, written by `snapshot_adapter`, whose lane suppresses weights rather than refusing. | -| 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.build_adoption`'s `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) 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). | +| 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. | diff --git a/docs/prd-ledger.md b/docs/prd-ledger.md index e711257b..07fd2b65 100644 --- a/docs/prd-ledger.md +++ b/docs/prd-ledger.md @@ -38,7 +38,17 @@ A fourth engine-assigned shape records a disappearance the user confirmed as a s Like `adjustment`, it stays out of holding derivation: the accompanying new anchor is what moves the book, and applying both would double-count. Its purpose is to be *readable* — `revisit.absence_exits` turns it into a real exit row (`kind: "full"`, `exit_price: null`), so the confirmation the user gave is not a written-never-read field (#429's failure class). The shares, currency, market and cost basis that row carries are copied from the recorded book as it stood immediately before the row was appended, never inferred from a price. Positions the user keeps because the new view simply did not capture them are carried into the new anchor with `"carried": true` on the position row; the flag is written only when true, because every snapshot payload is content-addressed and an unconditional `false` would rewrite every existing `snapshot_id`. -The mirror-image answer rides the same anchor row (#531). A position that *appeared* in the declaration is confirmed too, and the user's rough answer to "how long have you held this" becomes `"since"` plus `"since_basis"` on that position — the conversion from months to a date happens in `book_refresh._months_before`, never in the agent. The two keys are one fact and `snapshot_adapter` validates them as a pair: `since` cannot exist without the `since_basis: "user_estimate"` stamp beside it, which is how "a reconstructed start date is never rendered as an exact date" holds at the storage layer rather than as a rule every renderer has to remember. `since_basis: "unknown"` carries no date at all and `ledger.derive_holdings` turns it into the two-segment `ticker#unknown` cycle that already exists for an undatable open. Like `carried`, both are written only when they apply, for the same content-addressing reason; and like `carried`, neither enters `portfolio_basis._normalized_anchor`, so two declarations of the same book still share one `state_version` regardless of how each row's start was learned. A later refresh copies an existing stamp forward onto the same position (`book_refresh._carry_forward_provenance`) so the answer is not spent on one review — but only while the record still traces that position to its anchor, because a position sold and bought back is a different cycle the ledger can date itself. +The mirror-image answer rides the same anchor row (#531). A position that *appeared* in the declaration is confirmed too, and the user's rough answer to "how long have you held this" becomes `"since"` plus `"since_basis"` on that position — the conversion from months to a date happens in `book_refresh._months_before`, never in the agent. The two keys are one fact and `snapshot_adapter` validates them as a pair: `since` cannot exist without a `since_basis` stamp beside it saying where that date came from, which is how "a reconstructed start date is never rendered as an exact date" holds at the storage layer rather than as a rule every renderer has to remember. `since_basis: "unknown"` carries no date at all and `ledger.derive_holdings` turns it into the two-segment `ticker#unknown` cycle that already exists for an undatable open. Like `carried`, both are written only when they apply, for the same content-addressing reason; and like `carried`, neither enters `portfolio_basis._normalized_anchor`, so two declarations of the same book still share one `state_version` regardless of how each row's start was learned. + +Two further bases, `"trade_event"` and `"snapshot_anchor"`, are what make a position survive its own book being re-declared (#539/#536). A declaration says what is held and never since when, so an adopted position whose start it did not restate falls back to that declaration's `as_of` — reminting `cycle_id` and orphaning every memory keyed to it, the user's thesis first. `book_refresh.carry_recorded_starts` stamps each still-held position with the start the record already holds, and it is the single primitive both adoption lanes reach: `build_adoption` for the refresh lane, `review._carried_declaration` for the review lane, which is also the only path through which engine-assigned provenance may enter a book at all. + +**What is carried is the basis, not a marker saying the date was carried** (owner ruling, 2026-07-31). A single "this came from the record" value would describe how the date was transported and destroy why it is believed, collapsing an exact start the ledger watched open together with a lower bound inferred from the day a declaration first listed the position — and the loss would be permanent, because after an adoption `origin` describes the new snapshot writer rather than the original evidence. So `since_basis` is an evidence vocabulary and it travels unchanged: a position the record still traces to its anchor keeps that anchor's own stamp, whatever it says, and only a position the anchor does not describe — one carried for the first time, or one whose current cycle the ledger opened since — is labelled fresh, `trade_event` when the ledger watched it open and `snapshot_anchor` when a declaration first put it on the books. That branch order is load-bearing rather than incidental: an adopted trade-proven position reads `origin == "snapshot"` like every other row, so a basis recomputed from origin would demote it on the very next declaration. + +Reading the start off the *derived* book is what keeps a sold-and-rebought position honest without a further rule: its derived start *is* the rebuy, so there is no previous cycle to inherit from and no estimate can follow the user's words onto a cycle they were not about. Two guards on writing at all: a start later than the declaration's own `as_of` means the declaration predates the record, which the lane owning that refusal states plainly, and a `snapshot_anchor` start equal to `as_of` restates the default and would change the anchor's content address without changing a fact. The equality guard stops there — for the other bases the stamp carries something the default does not. + +No surface consumes the distinction yet; #532 owns making provenance visible and actionable. What this change owes is that the distinction is still *there* to consume, through any number of adoptions and a projection repair. + +`"cycle_seq"` rides with it, and only with it. A `cycle_id` is `ticker#since#seq`, so a position flattened and bought back on one day differs from the cycle it replaced in the sequence alone — carrying the date without it would give the live position the sold cycle's exact identity, and with it that cycle's thesis, standing conditions and closed status. `snapshot_adapter` refuses a sequence with no dated start beside it, and refuses one on `since_basis: "unknown"` because a two-segment `ticker#unknown` cycle has no sequence to state. Like `carried`, it is written only past the first cycle, for the same content-addressing reason. `ledger.cycle_sequence` is the single inverse of the composition in `derive_holdings`, and `_anchored_cycle_seq` degrades a bad value to 1 with a named `integrity` issue rather than refusing the whole book, the contract `_anchored_cycle_start` already keeps. `ledger.append_events` can stamp a `recorded_at` date alongside the schema version `v` (#472): *when this system learned the fact*, as distinct from the event's own `date`/`as_of` (*when the thing happened*). A trade imported weeks late still carries its true trade date, but `recorded_at` marks the later day it actually entered the ledger — the field a future rule backtest needs to replay history without look-ahead bias, since without it a late import reads, to the replay, as if it had been known on its trade date. diff --git a/skills/fomo-kernel/engine/book_refresh.py b/skills/fomo-kernel/engine/book_refresh.py index 8eaebcb2..7567f6be 100644 --- a/skills/fomo-kernel/engine/book_refresh.py +++ b/skills/fomo-kernel/engine/book_refresh.py @@ -493,39 +493,135 @@ def _appearance_stamp(row, answer, as_of): return row -def _carry_forward_provenance(row, recorded, derived): - """Keep an already-answered cycle start attached across later refreshes. - - Without this the stamp survives exactly one refresh: the next declaration is - an ordinary envelope with no provenance on it, so the position's start would - snap back to the new anchor's date and the user's answer would have bought - exactly one review. The user is not asked again, and should not be — the - position is not appearing, it is simply still held. - - Two gates, and both are about not carrying a stamp onto a different cycle. - The stamp is read from the last declaration's own row (the only place that - says a start was answered rather than assumed; a ``trades_derived`` - restatement never stamps one, so reading the newest recorded row instead - would spend the user's answer the first time they imported a CSV). And it is applied only while - the record still traces the position back to that anchor - (``origin == "snapshot"``): a position sold and bought back reads - ``origin == "trades"``, and its real recorded open date must win over a - stamp describing the cycle before it. +def _carried_start(row, recorded, derived, as_of): + """Attach the cycle start the record already holds for one position (#539). + + A declaration says what is held, never since when. So an adopted position + whose start this declaration did not restate must be given the start the + book already had, or ``ledger._anchored_cycle_start`` falls back to the new + anchor's own date, ``cycle_id`` remints, and every memory keyed to that + cycle — the user's thesis first — reads as belonging to nothing. + + What is carried is the *basis*, not a marker saying the date was carried + (owner ruling, 2026-07-31). A single "this came from the record" stamp would + describe how the date was transported and destroy why it is believed — an + exact start the ledger watched open and a lower bound inferred from the day + a declaration first listed the position would become the same value, and + nothing could tell them apart afterwards. Not even ``origin``: once the book + is adopted, that describes the new snapshot writer rather than the original + evidence. So the four bases are the vocabulary and they travel unchanged. + + The cases, in the order they are decided. + + A row that already carries a stamp is left alone: it came from + ``_appearance_stamp`` a few lines up, which is the position the user was + just asked about, and this declaration *is* its first statement. + + A position the record still traces to its anchor keeps that anchor's own + stamp, whatever it says. The stamp is read from the last declaration's own + row — the only place that states a start rather than assuming one; a + ``trades_derived`` restatement never stamps one, so reading the newest + recorded row instead would spend the user's #531 answer the first time they + imported a CSV. This is also the branch that keeps a trade-proven start + trade-proven: once adopted, a position opened by a real buy sits in the + anchor and reads ``origin == "snapshot"`` like any other, so a basis + recomputed from origin here would quietly demote it to a lower bound on the + second declaration. + + Everything else still held takes the start the *derived* book states, with + the basis that book's own evidence supports: ``trade_event`` for a cycle the + ledger watched open, ``snapshot_anchor`` for one a declaration first put on + the books. Reading it from the derived book is what makes this safe without + a further gate — the derived start of a rebought position is its rebuy date, + so there is no previous cycle to inherit from, and no old estimate can + follow the user's words onto a cycle they were not about. It is also a + fixpoint: carrying it again yields the same date and the same basis, so + replay, ``repair-projections`` and an idempotent finalize retry are stable + by construction. + + A ticker absent from the derived book is untouched. It is appearing, and + #531 owns what happens to it. + + Two guards on writing at all. A start later than this declaration's own + ``as_of`` means the declaration predates the record, which is refused a few + steps on by the lane that owns that refusal; carrying one here would replace + a plain "older than the current ledger anchor" with a validation error about + a field the user never supplied. And a ``snapshot_anchor`` start equal to + ``as_of`` restates exactly what ``_anchored_cycle_start`` already defaults + to, so writing it would change the anchor's content address without changing + a fact — which is what an idempotent replay of the same declaration is + checked by. The equality guard is deliberately not extended to the other + bases: for them the stamp carries something the default does not. """ - ticker = row.get("ticker") - stamp = (recorded.get(ticker) or {}).get("since_basis") - if stamp not in lg.SINCE_BASES: + if row.get("since_basis"): return row - if (derived.get(ticker) or {}).get("origin") != "snapshot": + ticker = row.get("ticker") + held = derived.get(ticker) or {} + if not held: return row row = dict(row) - row["since_basis"] = stamp row.pop("since", None) - if stamp == "user_estimate": - row["since"] = recorded[ticker].get("since") + stamp = (recorded.get(ticker) or {}).get("since_basis") + if stamp in lg.SINCE_BASES and held.get("origin") == "snapshot": + if stamp == "unknown": + # No date to guard: the cycle is `ticker#unknown` either way, and + # that identity is already stable across declarations. + row["since_basis"] = "unknown" + return row + since, basis = recorded[ticker].get("since"), stamp + else: + since = held.get("since") + basis = ("trade_event" if held.get("origin") == "trades" + else "snapshot_anchor") + # The date and the basis are decided together, deliberately: a start read off + # the derived book labelled `user_estimate` would attribute a ledger fact to + # the user, which is the same dishonesty as the reverse. + if not since or str(since) > str(as_of): + return row + if str(since) == str(as_of) and basis == "snapshot_anchor": + return row + row["since"], row["since_basis"] = since, basis + # The start alone is not the identity. Two cycles opened on one day differ + # only here, so carrying the date without the sequence would give the live + # position the id of the one that was sold (#539). + seq = lg.cycle_sequence((held.get("cycle_id") or "")) + if seq and seq > 1: + row["cycle_seq"] = seq return row +def carry_recorded_starts(envelope, events, *, splits=None, today=None): + """Validate one adopted book with every continuously-held start carried (#539). + + The single primitive both adoption lanes call, and the single place engine + provenance may enter a book. The refresh lane calls it on the envelope it + assembled from its own answers; the review lane calls it in ``cmd_prepare`` + on the declaration it is about to build a plan from, before + ``snapshot_adapter.prepare`` derives anything, so the plan's cycle ids and + the ledger anchor come from one stamped object rather than two derivations. + + It is one function rather than one call per lane because the composition is + the part that drifts: ``derived`` and ``recorded`` are assembled here, so a + caller cannot supply a book read through a different window (#444's rule — + a single reader is still two readers when its input is assembled twice). + + Returns ``normalize_book``'s ``(snapshot, anchor)``. The envelope may be a + raw one or a previously normalized one; only the declared fields are read. + """ + positions = list(envelope.get("positions") or []) + if positions: + derived = lg.holdings_as_of(events or [], envelope["as_of"], splits=splits) + recorded = _declared_map(lg.latest_anchor(events or [], declared_only=True) or {}) + positions = [_carried_start(row, recorded, derived, envelope["as_of"]) + for row in positions] + stamped = {key: value for key, value in envelope.items() + if key in snapshot_adapter.ENVELOPE_KEYS and value is not None} + stamped["positions"] = positions + return snapshot_adapter.normalize_book( + stamped, today=today or dt.date.fromisoformat(envelope["as_of"]), + allow_engine_provenance=True) + + def _carried_row(ticker, fact): """One position carried forward from the record, copied and never invented.""" row = {"ticker": ticker, "shares": fact.get("shares"), @@ -552,10 +648,10 @@ def build_adoption(receipt, events, snapshot, anchor, answers, *, today=None, sp carried rows merged back in, the difference the ledger records must be the difference that actually happened. - This is also the only place ``allow_engine_provenance`` is turned on. The - envelope assembled here is not a caller's file: every ``since`` in it was - computed a few lines above by ``_appearance_stamp`` or copied off the - recorded anchor by ``_carry_forward_provenance``. + Validation and engine provenance both run through ``carry_recorded_starts``, + shared with the review lane: the envelope assembled here is not a caller's + file, and every ``since`` in the adopted book was either computed a few lines + above by ``_appearance_stamp`` or carried off the record by that primitive. """ classifications = _validated_answers(receipt.get("pending_confirmations") or [], answers) resupply = sorted(ticker for ticker, value in classifications.items() @@ -565,7 +661,6 @@ def build_adoption(receipt, events, snapshot, anchor, answers, *, today=None, sp "tickers": resupply} derived = lg.holdings_as_of(events, snapshot["as_of"], splits=splits) - recorded = _declared_map(lg.latest_anchor(events, declared_only=True) or {}) by_ticker = {row["ticker"]: row for row in receipt.get("pending_confirmations") or []} carried, sold, appeared = [], [], {} for ticker, answer in sorted(classifications.items()): @@ -579,8 +674,7 @@ def build_adoption(receipt, events, snapshot, anchor, answers, *, today=None, sp fact = derived.get(ticker) if not fact: raise RefreshError(f"{ticker} is no longer in the recorded book to carry forward") - carried.append(_carry_forward_provenance( - _carried_row(ticker, fact), recorded, derived)) + carried.append(_carried_row(ticker, fact)) elif answer["classification"] == "sold": sold.append(ticker) @@ -588,17 +682,16 @@ def build_adoption(receipt, events, snapshot, anchor, answers, *, today=None, sp for row in anchor.get("positions") or []: row = _supplied_row(row) answer = appeared.get(row.get("ticker")) - supplied.append(_appearance_stamp(row, answer, snapshot["as_of"]) if answer - else _carry_forward_provenance(row, recorded, derived)) + supplied.append(_appearance_stamp(row, answer, snapshot["as_of"]) if answer else row) envelope = {"as_of": snapshot["as_of"], "positions": supplied + carried} if snapshot.get("cash") is not None: envelope["cash"] = snapshot["cash"] if snapshot.get("fx"): envelope["fx"] = snapshot["fx"] try: - _adopted_snapshot, adopted_anchor = snapshot_adapter.normalize_book( - envelope, today=today or dt.date.fromisoformat(snapshot["as_of"]), - allow_engine_provenance=True) + _adopted_snapshot, adopted_anchor = carry_recorded_starts( + envelope, events, splits=splits, + today=today or dt.date.fromisoformat(snapshot["as_of"])) except snapshot_adapter.SnapshotError as exc: raise RefreshError(f"the adopted book failed validation: {exc}") from exc diff --git a/skills/fomo-kernel/engine/ledger.py b/skills/fomo-kernel/engine/ledger.py index 3f6e8398..8c80323d 100644 --- a/skills/fomo-kernel/engine/ledger.py +++ b/skills/fomo-kernel/engine/ledger.py @@ -79,17 +79,28 @@ # 的人一定同時讀得到「這是估的」——「推算出來的開倉日不得被當成精確日期呈現」在儲存層 # 就成立,不必靠每個 renderer 自律。since_basis="unknown"(使用者說不知道)不帶 since, # derive_holdings 下面把它變成既有的 ticker#unknown cycle。 +# cycle_seq(#539)是 cycle_id 的第三段:同一天內清掉再買回的兩個 cycle 只差這個序號, +# 所以帶了 since 卻不帶序號的採納會把還持有的部位配上「已經賣掉那個 cycle」的 id。 SNAPSHOT_POSITION_KEYS = frozenset({ "ticker", "shares", "avg_cost", "market_value", "market", "currency", "carried", - "since", "since_basis", + "since", "since_basis", "cycle_seq", }) # 引擎指派、永遠不收 agent 供給的持倉欄位(SKILL.md 不可協商規則 1:數字來自引擎產物)。 # snapshot_adapter.normalize_envelope 預設拒收它們;只有 book_refresh 採納自己剛問到的 # 答案時才開鎖,而那一條路上的值是引擎自己算出來的。 -ENGINE_ASSIGNED_POSITION_KEYS = frozenset({"since", "since_basis"}) -# since_basis 的合法值。"user_estimate" = 從使用者給的月數換算(近似,±半個月); -# "unknown" = 使用者說不知道,不編日期。 -SINCE_BASES = ("user_estimate", "unknown") +ENGINE_ASSIGNED_POSITION_KEYS = frozenset({"since", "since_basis", "cycle_seq"}) +# since_basis 說的是「這個日期憑什麼被相信」,不是「它是怎麼被搬過來的」 +# (#539 owner ruling 2026-07-31)。四個值各是一種證據強度,採納時原樣帶過去,不合併: +# "trade_event" = 帳本親眼看著這個 cycle 開的,精確日期。 +# "snapshot_anchor" = 某次宣告第一次把它記進帳本的那天,是下界,不是買進日。 +# "user_estimate" = 從使用者給的月數換算(近似,±半個月)。 +# "unknown" = 使用者說不知道,不編日期。 +# 後兩個是舊有的;前兩個是 #539 新增。一檔從沒被問過「抱多久」的持倉(第一次宣告裡 +# 的每一檔都是)本來沒有任何蓋章可帶,於是每次採納都被退回成最新宣告日,cycle_id 跟著 +# 重鑄,使用者寫過的 thesis 被重問一次。但用單一個「從帳本帶過來的」值蓋掉全部會抹掉 +# 精確度差異,而且事後補不回來:採納之後 origin 描述的是新的 snapshot 寫入者,不再是 +# 這個起點原本的證據。所以帶的是原本那個值本身。 +SINCE_BASES = ("user_estimate", "unknown", "trade_event", "snapshot_anchor") # A snapshot row's ``source`` says how the book it states was learned (#549). # It is recorded, never used to decide whether the row counts as the recorded @@ -368,6 +379,9 @@ def _anchored_cycle_start(position, anchor_date, ticker, integrity): 預設仍是錨點日 —— 一筆宣告出來的持倉,帳本只知道它「至少從這天起在帳上」。 #531 之後,refresh 問過「大約抱多久」的持倉會帶 since/since_basis 蓋章: ``user_estimate`` 用蓋章的日期,``unknown`` 保持錨點日但把 cycle 標成 unknown。 + #539 之後,採納一份新宣告時引擎會把帳本本來就記著的起點連同它原本的證據強度 + (``trade_event`` / ``snapshot_anchor``)一起帶過來。這裡對三個帶日期的值讀法 + 完全相同 —— 差別是「這個日期憑什麼被相信」,那屬於帳本的誠實,不屬於這裡的算法。 本函式對壞值一律降級回錨點日並記 integrity,不 raise:ledger.jsonl 是可被手改的 append-only 檔案,而 derive_holdings 對壞資料的既有契約是「照走、但看得見」 @@ -398,6 +412,43 @@ def _anchored_cycle_start(position, anchor_date, ticker, integrity): return stated.isoformat(), False +def cycle_sequence(cycle_id): + """The sequence segment of a ``cycle_id``, or None when it carries none. + + The inverse of the one composition below, kept beside it so the two cannot + drift, and the only reader: a second place that split this string would be + the divergent-derivation shape this repository keeps closing. ``#unknown`` + and anything unparseable answer None — a caller with no sequence to carry + must be told so rather than handed a guess. + """ + parts = str(cycle_id or "").split("#") + if len(parts) != 3: + return None + try: + seq = int(parts[2]) + except (TypeError, ValueError): + return None + return seq if seq > 0 else None + + +def _anchored_cycle_seq(position, ticker, integrity): + """一列錨點持倉的 cycle 序號,預設 1(#539)。 + + ``since`` 只釘住 cycle 的起點;同一天清倉再買回的兩個 cycle 起點相同,只差序號。 + 採納一份新宣告時若只帶 since 不帶序號,還持有的那個 cycle 會拿到「已經賣掉那一段」 + 的 id,連同它的 thesis、conditions 和結案狀態一起繼承過來 —— 比重鑄更糟。 + + 與 ``_anchored_cycle_start`` 同一個契約:壞值降級回預設並記 integrity,不 raise。 + """ + raw = position.get("cycle_seq") + if raw is None: + return 1 + if isinstance(raw, bool) or not isinstance(raw, int) or raw < 1: + integrity.append({"issue": "bad_cycle_seq", "ticker": ticker}) + return 1 + return raw + + def _moved_basis(position, events, after, upto): """Record the newest split this rebase actually applied to ``position``. @@ -504,7 +555,9 @@ def derive_holdings(events, splits=None, as_of=None): "cycle_unknown": cycle_unknown, "add_count": 0, # A declaration states shares in its own as_of basis (#558). "basis_date": anchor_date} - seq_base[t] = 1 # cycle 序號單一事實源:seq_base(清倉後仍保留,重建 +1) + # cycle 序號單一事實源:seq_base(清倉後仍保留,重建 +1)。錨點列帶得動它時 + # 就沿用,否則 1 —— 一份沒有序號的宣告本來就說不出它是第幾段(#539)。 + seq_base[t] = _anchored_cycle_seq(p, t, integrity) trades = [] for ev in events: diff --git a/skills/fomo-kernel/engine/review.py b/skills/fomo-kernel/engine/review.py index 75885ee8..9ff18f5e 100644 --- a/skills/fomo-kernel/engine/review.py +++ b/skills/fomo-kernel/engine/review.py @@ -457,6 +457,35 @@ def _virtual_review_basis(inputs, batches, state): return overlay, receipt +def _carried_declaration(root, snapshot_path): + """The declaration with each still-held cycle start carried from the record. + + #539: a declaration says what is held, never since when, so an ordinary + second declaration would give every continuously held position a cycle id + minted from its own ``as_of`` — and the user's thesis, written against the + previous id, would be asked for again as if they had never answered. + + Returns ``None`` for a root with no recorded book, which is onboarding: there + is nothing to carry, and ``prepare`` loads the file the way it always has. + + Deliberately here, before ``snapshot_adapter.prepare`` builds anything: the + stamped envelope is what ``_anchor`` writes to the ledger *and* what + ``_state_positions`` derives the plan's cycle ids from, so the questions the + user is asked and the book that is recorded cannot disagree about identity. + Doing it at finalize instead would fix the ledger after the review had + already re-asked. The refresh lane calls the same primitive from + ``build_adoption`` (#536): one implementation, and the only path through + which engine-assigned provenance enters a book. + """ + events, _skipped = ledger.load_ledger(os.path.join(root, "ledger.jsonl")) + if not events or ledger.latest_anchor(events) is None: + return None + snapshot, _anchor = book_refresh.carry_recorded_starts( + snapshot_adapter.normalize_book(snapshot_path)[0], events, + splits=_recorded_splits(root)) + return snapshot + + def _validate_initial_snapshot_root(root, anchor): """Resolve how a runtime snapshot declaration may enter this coach root. @@ -4139,7 +4168,8 @@ def _prepare_session(args): if args.snapshot_json: try: card, state, adapter_meta = snapshot_adapter.prepare( - paths[0], driver_map=args.driver_map, instrument_map=args.instrument_map + paths[0], driver_map=args.driver_map, instrument_map=args.instrument_map, + snapshot=_carried_declaration(root, paths[0]) ) except (OSError, ValueError, snapshot_adapter.SnapshotError) as exc: raise ReviewError(f"snapshot adapter rejected input: {exc}") from exc diff --git a/skills/fomo-kernel/engine/snapshot_adapter.py b/skills/fomo-kernel/engine/snapshot_adapter.py index 8b32dc05..4f32a17d 100644 --- a/skills/fomo-kernel/engine/snapshot_adapter.py +++ b/skills/fomo-kernel/engine/snapshot_adapter.py @@ -43,6 +43,9 @@ class SnapshotError(ValueError): ENVELOPE_KEYS = {"as_of", "positions", "cash", "fx"} +# What normalize_envelope returns, declared beside what it accepts so `prepare` +# can tell a validated envelope from a raw one rather than trusting its caller. +_NORMALIZED_KEYS = ENVELOPE_KEYS | {"merged_rows", "input_rows"} POSITION_KEYS = set(ledger.SNAPSHOT_POSITION_KEYS) # single declaration: ledger.py # Engine-assigned, never agent-supplied. Also declared once, in ledger.py. ENGINE_ASSIGNED_KEYS = set(ledger.ENGINE_ASSIGNED_POSITION_KEYS) @@ -127,6 +130,15 @@ def _normalize_provenance(raw, index, row): downstream reader would be free to print it as an exact day. Enforcing the pair here is what makes "a derived start date is never rendered as an exact date" true at the storage layer rather than a habit each renderer must keep. + + The stamp says what the date is worth, and the set of answers lives in + ``ledger.SINCE_BASES`` rather than here: ``trade_event`` is exact (the ledger + watched that cycle open), ``snapshot_anchor`` is the day a declaration first + put the position on the books and is a lower bound, ``user_estimate`` is the + user's own approximation, and ``unknown`` carries no date at all. The three + dated bases are validated identically — the distinction is stored, not + enforced, because it is a claim about evidence rather than about shape, and + #539 keeps them separate precisely because nothing can recover it later. """ basis = raw.get("since_basis") since = raw.get("since") @@ -135,6 +147,10 @@ def _normalize_provenance(raw, index, row): raise SnapshotError( f"positions[{index}].since requires since_basis; a reconstructed " "start date may not travel without the stamp saying it is one") + if raw.get("cycle_seq") is not None: + raise SnapshotError( + f"positions[{index}].cycle_seq requires the dated start it " + "sequences; a cycle segment cannot travel on its own") return if basis not in ledger.SINCE_BASES: allowed = ", ".join(ledger.SINCE_BASES) @@ -143,13 +159,41 @@ def _normalize_provenance(raw, index, row): if since is not None: raise SnapshotError( f"positions[{index}].since_basis is 'unknown' but a since was supplied") + if raw.get("cycle_seq") is not None: + raise SnapshotError( + f"positions[{index}].since_basis is 'unknown', which is a " + "two-segment cycle with no sequence to state") row["since_basis"] = "unknown" return if since is None: raise SnapshotError( - f"positions[{index}].since_basis is 'user_estimate' but no since was derived") + f"positions[{index}].since_basis is {basis!r} but no since was derived") row["since"] = _strict_date(since, f"positions[{index}].since").isoformat() - row["since_basis"] = "user_estimate" + row["since_basis"] = basis + _normalize_cycle_seq(raw, index, row) + + +def _normalize_cycle_seq(raw, index, row): + """Attach the cycle's sequence segment, which only a dated start can carry. + + A `cycle_id` is `ticker#since#seq`, and #539 carries the start across + declarations. The sequence has to travel with it: a position flattened and + reopened on the same day differs from the cycle it replaced in this segment + alone, so carrying the date without the sequence hands a live position the + identity of one that was sold — with that cycle's thesis, conditions and + closed status attached. Worse than the reminting it was fixing. + + Refused without a dated pair, deliberately. `since_basis: "unknown"` yields + the two-segment `ticker#unknown` cycle, which has no sequence to state, and + a bare sequence with no start would claim a reopen the row cannot date. + """ + seq = raw.get("cycle_seq") + if seq is None: + return + if isinstance(seq, bool) or not isinstance(seq, int) or seq < 1: + raise SnapshotError( + f"positions[{index}].cycle_seq must be a positive integer") + row["cycle_seq"] = seq def _normalize_position(raw, index, *, allow_engine_provenance=False): @@ -456,6 +500,12 @@ def _anchor(snapshot): position["since_basis"] = row["since_basis"] if row.get("since"): position["since"] = row["since"] + # Only past the first (#539). Like `carried`, every snapshot payload + # is content-addressed, so an unconditional key would rewrite every + # existing `snapshot_id` — and the overwhelmingly common cycle is + # the first, where the sequence states nothing the default does not. + if (row.get("cycle_seq") or 1) > 1: + position["cycle_seq"] = row["cycle_seq"] positions.append(position) event = { "type": "snapshot", @@ -579,14 +629,28 @@ def _honesty(snapshot, currencies, fx_gaps, unclassified, portfolio_structure): return entries -def prepare(path, driver_map=None, instrument_map=None, today=None): +def prepare(path, driver_map=None, instrument_map=None, today=None, *, snapshot=None): """Return ``(card, state, meta)`` for one normalized local snapshot. No history-only metric is synthesized. When a complete, consistently valued portfolio cannot be put in one currency, ``weights_available`` is false and the adapter emits no sizing/diversification dimension. + + ``snapshot`` is an envelope that has already been through + ``normalize_envelope``, supplied instead of loading ``path``. Its one + producer is ``book_refresh.carry_recorded_starts``, which stamps each + continuously-held position with the cycle start the record already held + (#539) — and which is the review lane's only way to obtain one, because + engine-assigned provenance may not enter through the file path an agent + writes. Passing it here rather than post-processing the result is what + gives this route one computation with two consumers: ``_anchor`` carries the + stamp into the ledger event, and ``_state_positions`` derives the plan's + cycle ids from that same anchor. """ - snapshot = _load(os.path.abspath(os.path.expanduser(path)), _today(today)) + if snapshot is None: + snapshot = _load(os.path.abspath(os.path.expanduser(path)), _today(today)) + elif set(snapshot) != _NORMALIZED_KEYS: + raise SnapshotError("prepare's snapshot argument must be a normalized envelope") basis, native_values = _valuation(snapshot) global_values, currencies, fx_gaps = _global_values(snapshot, native_values) weights_available = bool(global_values) diff --git a/skills/fomo-kernel/flows/book-refresh.md b/skills/fomo-kernel/flows/book-refresh.md index 9d5e0d87..3bc48f84 100644 --- a/skills/fomo-kernel/flows/book-refresh.md +++ b/skills/fomo-kernel/flows/book-refresh.md @@ -56,7 +56,8 @@ Report what was recorded, in the user's terms: - a `sold` position left the book. Its exit is now tracked like any other, and it will come back at the 30/60/90 checkpoints. Say plainly that **no sale price was recorded**, so it does not count toward win rate, payoff, or exit discipline — supplying the actual trade later is the way to make it count. (It does not upgrade automatically today; see #519.) - a `not_captured` position stayed, carried forward from the record. If the same position is missing again next time, this flow will ask again — there is no answer memory. Saying so once is honest; nagging about it is not. -- a `confirmed` appearance entered the book with the start date the engine derived from the months given. Report it as the approximation it is — "about eighteen months" — never as a specific day; the engine stores it stamped as an estimate for exactly that reason. If they said they did not know, say the position is recorded and simply will not appear in holding-period readings until a real trade record exists. This answer is not asked again: the position is held from now on, not appearing. +- a `confirmed` appearance entered the book with the start date the engine derived from the months given. Report it as the approximation it is — "about eighteen months" — never as a specific day; the engine stores it stamped `user_estimate` for exactly that reason. If they said they did not know, say the position is recorded and simply will not appear in holding-period readings until a real trade record exists. This answer is not asked again: the position is held from now on, not appearing. +- every other position keeps the start the record already held for it, with the stamp that says what that date is worth (#539). Nothing to report — it is the absence of a change, and the point is that the user's earlier answers and theses stay attached to the same position rather than being asked for again. If you do report one, the stamp is what decides how: `trade_event` is exact, because the ledger watched that cycle open, and may be stated as a day; `snapshot_anchor` is the day a declaration first put the position on the books, so it is a lower bound — "on the books since at least March" — and never the day it was bought; `user_estimate` stays the approximation it always was. - `reconciliation: "reconciled"` means the adopted book matched the record exactly, so nothing was rewritten. That is a clean result, not a failure. ## How this flow relates to the shared runtime contracts diff --git a/skills/fomo-kernel/references/data-contract.md b/skills/fomo-kernel/references/data-contract.md index 39d18b79..f53b6dc2 100644 --- a/skills/fomo-kernel/references/data-contract.md +++ b/skills/fomo-kernel/references/data-contract.md @@ -48,7 +48,7 @@ Snapshot envelope: - Every source records the book at the time it arrives, and its `source` records which kind of source it was without that deciding anything (#549). A holdings view records `user_declared`; a transaction import records `trades_derived` after the trades it summarizes. Ledger-derived current holdings stay canonical either way — a `trades_derived` row is that derivation written down, so it is never replayed as if it replaced the trades. Later transaction-history imports may unlock supported history-dependent diagnostics; they do not by themselves reconcile a newer broker view, which is what the refresh lane below is for. - Canonical means it is also the denominator. A review reconciles what it derived from the file it was given against that recorded book, and account-wide readings — position sizing, diversification and sector exposure, ETF structure, unrealized P&L — are stated only when the file covers it. Handing over just this period's trades is ordinary use and is not an error: the review still runs, the behavioural half (exits, holding time, averaging down, payoff) still reads from those trades, and the account-wide half is withdrawn for that period and said so on the card rather than being restated from a partial view. Re-supplying the full history restores it. Nothing here asks the user to reformat anything (#630). - A second or subsequent complete snapshot goes through the same `--snapshot-json` prepare call and routes to reconciliation instead of onboarding. The engine compares it with ledger-derived holdings as of the declared `as_of` and freezes the verdict into `engine_state.snapshot_reconciliation`: a `status` of `reconciled` or `adjusted`, plus a narrow fact diff — per-ticker shares, market, currency, avg_cost, one-sided tickers, and per-currency cash, all in original currency. The diff states values, never causes: a missing trade, a transfer, a split, a fee, and a data error all look identical here, so show it without explaining it. Finalize records the outcome (`reconciled` keeps the current anchor; `adjusted` appends one adjustment event preserving history and adopts the newer declaration). Identities are engine-assigned and content-addressed, so a finalize replay is a no-op. A declaration older than the recorded book, a root that has recorded no book at all, and a ledger that changed between prepare and finalize are all rejected — rerun prepare rather than editing artifacts. A declaration carrying a difference only the user can settle is also refused and routed to `refresh` (next bullet): the engine puts every second declaration to the book-update lane first and refuses exactly the ones that lane would raise a confirmation for, so recording precedes discussing (#530). Every other difference reviews normally and is adopted as above. -- Updating the recorded book is its own flow, and once a book exists a newer view goes through it whenever anything in it needs the user's answer. `review.py refresh --snapshot-json ` takes the same envelope, returns the same narrow diff, and is decoupled from the review lifecycle: no card, no review question budget, no session (`flows/book-refresh.md`, `schemas/book-refresh.schema.json`). It asks about exactly three things before adopting anything, and every raised item goes into one question, never one per ticker. A position the record holds and the new view omits, which the ledger does not already explain, is always asked about — the engine cannot tell a sale from a screenshot that missed it, and the two answers lead to different states: `sold` removes the position and records that it is absent as of that date **without inventing a fill**, while `not_captured` carries the position forward with `carried:true`. A position the new view holds and the record does not is confirmed too (#531): it destroys nothing, but it arrives with no provenance and the engine cannot recover it later, so the answer states roughly how many months it has been held plus a cost when the view carries none. The engine, not the agent, converts those months into a cycle start, and stores it stamped as an estimate so no surface can print it as an exact day; "I don't know" is an ordinary answer and keeps the existing `ticker#unknown` cycle, which drops that holding from holding-period diagnostics rather than inventing a date. A large share change on a position that is a large part of the book asks to be confirmed or re-supplied. Everything else — small changes, cash differences, market and currency differences — is adopted and disclosed with no question. Preparing a refresh writes nothing; adopting one refuses if the recorded book changed in between. +- Updating the recorded book is its own flow, and once a book exists a newer view goes through it whenever anything in it needs the user's answer. `review.py refresh --snapshot-json ` takes the same envelope, returns the same narrow diff, and is decoupled from the review lifecycle: no card, no review question budget, no session (`flows/book-refresh.md`, `schemas/book-refresh.schema.json`). It asks about exactly three things before adopting anything, and every raised item goes into one question, never one per ticker. A position the record holds and the new view omits, which the ledger does not already explain, is always asked about — the engine cannot tell a sale from a screenshot that missed it, and the two answers lead to different states: `sold` removes the position and records that it is absent as of that date **without inventing a fill**, while `not_captured` carries the position forward with `carried:true`. A position the new view holds and the record does not is confirmed too (#531): it destroys nothing, but it arrives with no provenance and the engine cannot recover it later, so the answer states roughly how many months it has been held plus a cost when the view carries none. The engine, not the agent, converts those months into a cycle start, and stores it stamped `user_estimate` so no surface can print it as an exact day; "I don't know" is an ordinary answer and keeps the existing `ticker#unknown` cycle, which drops that holding from holding-period diagnostics rather than inventing a date. Every position that was already on record keeps the start the record held for it (#539), through both this lane and the review lane's adoption: a declaration says what is held and never since when, so letting the start default to each new declaration's `as_of` would remint the position's cycle id and re-ask the thesis the user wrote against the previous one. The stamp travels with the date and says what the date is worth, because after an adoption nothing else can: `trade_event` is exact — the ledger watched that cycle open — while `snapshot_anchor` is the day a declaration first put the position on the books and is therefore a lower bound, not a purchase date. State each as what it is; an exact start may be given as a day, a lower bound may not. A sold-and-rebought position is a different cycle and inherits nothing — its own cycle sequence is carried alongside the start, so a position flattened and bought back on one day is never handed the identity, thesis or standing conditions of the cycle it replaced. A large share change on a position that is a large part of the book asks to be confirmed or re-supplied. Everything else — small changes, cash differences, market and currency differences — is adopted and disclosed with no question. Preparing a refresh writes nothing; adopting one refuses if the recorded book changed in between. - A snapshot alone does not support claims about prior adds, exits, holding behavior, win rate, payoff, alpha, or motives. ETF policy: broad-market, regional, bond, and commodity ETFs are diversified allocations. Sector, thematic, and leveraged ETFs remain concentrated risk. Treat an unknown ticker conservatively as equity. Missing expense ratio or tracking error belongs in the honesty ledger and must never be filled with zero. diff --git a/skills/fomo-kernel/schemas/book-refresh.schema.json b/skills/fomo-kernel/schemas/book-refresh.schema.json index 9b560898..904da47f 100644 --- a/skills/fomo-kernel/schemas/book-refresh.schema.json +++ b/skills/fomo-kernel/schemas/book-refresh.schema.json @@ -2,7 +2,7 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "$id": "https://fomo-kernel.local/schemas/book-refresh.schema.json", "title": "Fomo Kernel Book Refresh", - "description": "The independent book-refresh lane (#485 Slice C, owner ruling 2026-07-28): updating the recorded book from a newer holdings view is its own conversation, decoupled from post-trade review and from pre-trade evaluation. `receipt` is what `review.py refresh --snapshot-json ...` emits and writes nothing; `input` is the agent-facing answers envelope `--answers` accepts. The readable contract; skills/fomo-kernel/engine/book_refresh.py holds the enforcement, because the offline suite carries no jsonschema dependency. Nothing here is stored under the coach root: the durable record of a refresh is the ledger rows it appends ([adjustment?, position_absence*, snapshot]), which ride the already-registered ledger.jsonl. An appearance's answer is recorded inside that snapshot row, as `since` plus `since_basis` on the position (ledger.SNAPSHOT_POSITION_KEYS): they are one fact validated as a pair, so a reconstructed start date can never be read without the stamp saying it was reconstructed. Making that stamp visible and actionable to the user is #532's job, not this lane's.", + "description": "The independent book-refresh lane (#485 Slice C, owner ruling 2026-07-28): updating the recorded book from a newer holdings view is its own conversation, decoupled from post-trade review and from pre-trade evaluation. `receipt` is what `review.py refresh --snapshot-json ...` emits and writes nothing; `input` is the agent-facing answers envelope `--answers` accepts. The readable contract; skills/fomo-kernel/engine/book_refresh.py holds the enforcement, because the offline suite carries no jsonschema dependency. Nothing here is stored under the coach root: the durable record of a refresh is the ledger rows it appends ([adjustment?, position_absence*, snapshot]), which ride the already-registered ledger.jsonl. An appearance's answer is recorded inside that snapshot row, as `since` plus `since_basis` on the position (ledger.SNAPSHOT_POSITION_KEYS): they are one fact validated as a pair, so a start date can never be read without the stamp saying where it came from. Every adopted position carries that pair, not only an answered appearance (#539): a position the record already held keeps the start the record held for it, with the basis that says what that date is worth (`trade_event` exact, `snapshot_anchor` a lower bound, `user_estimate` an approximation), plus `cycle_seq` past the first cycle — because a declaration says what is held and never since when, so left to default each new declaration would restate the start as its own `as_of` and remint the cycle every memory is keyed to, and carrying the start without the sequence would hand a position bought back on the day it was sold the identity of the cycle it replaced. Making that stamp visible and actionable to the user is #532's job, not this lane's.", "type": "object", "$defs": { "classification": { diff --git a/tests/test_book_refresh.py b/tests/test_book_refresh.py index 1167c60e..69c4a47e 100644 --- a/tests/test_book_refresh.py +++ b/tests/test_book_refresh.py @@ -246,6 +246,173 @@ def test_a_stamped_start_survives_the_next_refresh(): "silently spend the user's answer on one review") +def test_a_position_nobody_ever_asked_about_keeps_its_cycle_too(): + """#539: the stamp was never the fact worth carrying, and most rows have none. + + The appearance question is only asked for a position that appears *after* the + book exists, so every position in the user's first declaration is + permanently unstampable. Carrying only a stamp therefore reached exactly the + rows that needed it least: a position the user was asked about kept its + cycle, and the ones they were never asked about -- the whole opening book -- + reminted on every adoption, taking every thesis written against them with it. + + What is carried now is the start the record already holds, which for these + rows is the date the book first saw them. That is the same lower bound the + default produces on the first declaration; the defect was that each later + declaration replaced it with a newer, weaker one. + """ + with tempfile.TemporaryDirectory() as tmp: + root = _root(tmp) + events, _ = lg.load_ledger(os.path.join(root, "ledger.jsonl")) + assert lg.derive_holdings(events)["holdings"]["ACME"]["cycle_id"] == \ + "ACME#2026-06-30#1" + # WIDGET moves below both thresholds, so nothing is asked and the adopted + # book really differs from the record -- without that the refresh + # reconciles clean, keeps the old anchor, and proves nothing. + second = _snapshot(tmp, [{"ticker": "ACME", "shares": 100, "avg_cost": 12.0, + "market": "US", "currency": "USD"}, + dict(KEPT[0], shares=48), KEPT[1]], as_of="2026-07-25") + receipt = _cli(root, second) + assert receipt["pending_confirmations"] == [] + assert _cli(root, second, {"refresh_id": receipt["refresh_id"], + "answers": []})["status"] == "adopted" + events, _ = lg.load_ledger(os.path.join(root, "ledger.jsonl")) + holdings = lg.derive_holdings(events)["holdings"] + assert holdings["ACME"]["cycle_id"] == "ACME#2026-06-30#1", ( + "a position held right through two declarations is one position; " + "reminting it to ACME#2026-07-25#1 is what re-asks its thesis") + assert holdings["WIDGET"]["shares"] == 48.0, "the new view's numbers are adopted" + assert holdings["WIDGET"]["cycle_id"] == "WIDGET#2026-06-30#1", ( + "a share count changing is not a position restarting") + + +def test_a_start_the_ledger_can_prove_is_not_overwritten_by_a_declaration(): + """#539, second symptom: the same defect where the engine knew the answer. + + A position opened by a real trade after the anchor carries the date it was + actually bought. Before this, the next declaration replaced that with its own + `as_of` -- discarding a proven fact in favour of a bookkeeping one, and + reminting the cycle while doing it. Nobody is asked anything here; the record + already holds the truth and adoption has only to keep it. + """ + with tempfile.TemporaryDirectory() as tmp: + root = _root(tmp) + with open(os.path.join(root, "ledger.jsonl"), "a", encoding="utf-8") as handle: + handle.write(json.dumps({"type": "trade", "date": "2026-07-05", + "ticker": "NEWCO", "action": "buy", "qty": 10, + "price": 5.0, "market": "US", + "currency": "USD"}, sort_keys=True) + "\n") + events, _ = lg.load_ledger(os.path.join(root, "ledger.jsonl")) + before = lg.derive_holdings(events)["holdings"]["NEWCO"] + assert before["origin"] == "trades" and before["cycle_id"] == "NEWCO#2026-07-05#1" + second = _snapshot(tmp, [{"ticker": "ACME", "shares": 100, "avg_cost": 12.0, + "market": "US", "currency": "USD"}, + dict(KEPT[0], shares=48), KEPT[1], + {"ticker": "NEWCO", "shares": 10, "avg_cost": 5.0, + "market": "US", "currency": "USD"}], + as_of="2026-07-25") + receipt = _cli(root, second) + assert receipt["pending_confirmations"] == [], ( + "the record already holds NEWCO, so it is not an appearance") + assert _cli(root, second, {"refresh_id": receipt["refresh_id"], + "answers": []})["status"] == "adopted" + events, _ = lg.load_ledger(os.path.join(root, "ledger.jsonl")) + assert lg.derive_holdings(events)["holdings"]["NEWCO"]["cycle_id"] == \ + "NEWCO#2026-07-05#1", ( + "the buy date is the start; a declaration states what is held, " + "never since when, so it may not overwrite one") + stamped = {row["ticker"]: row.get("since_basis") + for row in lg.latest_anchor(events)["positions"]} + assert stamped["NEWCO"] == "trade_event", ( + "and it is labelled by the evidence: the ledger watched this cycle " + "open, so the date is exact rather than a declaration's lower bound") + assert stamped["ACME"] == "snapshot_anchor" + + # The second declaration is where a transport-only marker would have lost + # it. NEWCO now sits in the anchor and derives `origin == "snapshot"` like + # every other row, so a basis recomputed from origin here would silently + # demote a trade-proven date to a bookkeeping one -- and nothing + # downstream could ever tell that it had been exact. + third = _snapshot(tmp, [{"ticker": "ACME", "shares": 100, "avg_cost": 12.0, + "market": "US", "currency": "USD"}, + dict(KEPT[0], shares=47), KEPT[1], + {"ticker": "NEWCO", "shares": 10, "avg_cost": 5.0, + "market": "US", "currency": "USD"}], + as_of="2026-07-30") + again = _cli(root, third) + assert _cli(root, third, {"refresh_id": again["refresh_id"], + "answers": []})["status"] == "adopted" + events, _ = lg.load_ledger(os.path.join(root, "ledger.jsonl")) + row = [p for p in lg.latest_anchor(events)["positions"] + if p["ticker"] == "NEWCO"][0] + assert (row["since"], row["since_basis"]) == ("2026-07-05", "trade_event"), ( + "a start's evidence survives every later adoption; it is not " + "recoverable afterwards, because by then `origin` describes the " + "snapshot writer rather than how the start was learned") + + +def test_a_carried_start_never_hands_a_live_position_a_sold_cycles_identity(): + """The start alone is not the identity, and the gap is worse than reminting. + + A `cycle_id` is `ticker#since#seq`. Two cycles opened on the same day -- + a position flattened and bought back within one session, which is ordinary + behavior -- differ in the sequence and nothing else. So an adoption that + carries the date and not the sequence does not merely misname the live + position: it gives it the exact id of the one that was sold, and the folded + thesis for that cycle is closed, possibly falsified, with its own exit + narrative and its own standing conditions. A user who is re-asked for a + thesis has lost work; a user handed a sold cycle's thesis is being told + something untrue about the position they hold. + + Nothing is asked here. The round trip is already in the ledger and the + declaration agrees with it, so this passes through adoption in silence. + """ + with tempfile.TemporaryDirectory() as tmp: + root = _root(tmp) + with open(os.path.join(root, "ledger.jsonl"), "a", encoding="utf-8") as handle: + for row in ({"type": "trade", "date": "2026-07-20", "ticker": "ACME", + "action": "sell", "qty": 100, "price": 14.0}, + {"type": "trade", "date": "2026-07-20", "ticker": "ACME", + "action": "buy", "qty": 80, "price": 13.0}): + handle.write(json.dumps(row, sort_keys=True) + "\n") + events, _ = lg.load_ledger(os.path.join(root, "ledger.jsonl")) + assert lg.derive_holdings(events)["holdings"]["ACME"]["cycle_id"] == \ + "ACME#2026-07-20#2", "the round trip opened a second cycle on one day" + + second = _snapshot(tmp, [{"ticker": "ACME", "shares": 80, "avg_cost": 13.0, + "market": "US", "currency": "USD"}, + dict(KEPT[0], shares=48), KEPT[1]], as_of="2026-07-25") + receipt = _cli(root, second) + assert receipt["pending_confirmations"] == [] + assert _cli(root, second, {"refresh_id": receipt["refresh_id"], + "answers": []})["status"] == "adopted" + events, _ = lg.load_ledger(os.path.join(root, "ledger.jsonl")) + book = lg.derive_holdings(events) + assert book["holdings"]["ACME"]["cycle_id"] == "ACME#2026-07-20#2", ( + "ACME#2026-07-20#1 is the cycle the user sold; adopting a book may " + "not hand its thesis, its conditions and its closed status to the " + "position they are still holding") + assert not book["integrity"] + + # And it settles: carrying a carried sequence changes nothing, so an + # ordinary cadence of declarations cannot walk the identity anywhere. + third = _snapshot(tmp, [{"ticker": "ACME", "shares": 80, "avg_cost": 13.0, + "market": "US", "currency": "USD"}, + dict(KEPT[0], shares=47), KEPT[1]], as_of="2026-07-30") + again = _cli(root, third) + _cli(root, third, {"refresh_id": again["refresh_id"], "answers": []}) + events, _ = lg.load_ledger(os.path.join(root, "ledger.jsonl")) + assert lg.derive_holdings(events)["holdings"]["ACME"]["cycle_id"] == \ + "ACME#2026-07-20#2" + stamped = {row["ticker"]: (row.get("since"), row.get("since_basis")) + for row in lg.latest_anchor(events)["positions"]} + assert stamped["ACME"] == ("2026-07-20", "trade_event") + assert stamped["WIDGET"] == ("2026-06-30", "snapshot_anchor"), ( + "a start the book only knows as a declaration date stays a lower " + "bound however many declarations it survives; surviving is not " + "evidence, and nothing may read it as a purchase date later") + + def test_a_stamp_never_survives_onto_a_different_cycle(): """The carry-forward's own limit, and it is not a hypothetical. @@ -254,6 +421,14 @@ def test_a_stamp_never_survives_onto_a_different_cycle(): days ago as an eighteen-month holding — a number the user never said, in the field this issue exists to make honest. The record's own `origin` settles it: `trades` means the ledger has the real open date. + + #539 sharpened what "settles it" produces. The old carry-forward could only + keep the wrong estimate or keep nothing, and keeping nothing dropped the + position back to the new declaration's own date — still not the eighteen + months, and still not the truth the ledger was holding. Now the real open + date is what gets carried, stamped as the record's rather than the user's. + The invariant under test is unchanged and is asserted twice below: the + estimate does not survive, and the date the user never gave is not restated. """ with tempfile.TemporaryDirectory() as tmp: root = _root(tmp) @@ -289,9 +464,18 @@ def test_a_stamp_never_survives_onto_a_different_cycle(): assert out["status"] == "adopted", out events, _ = lg.load_ledger(os.path.join(root, "ledger.jsonl")) row = [p for p in lg.latest_anchor(events)["positions"] if p["ticker"] == "NEWCO"][0] - assert "since" not in row and "since_basis" not in row, ( + assert row["since_basis"] == "trade_event", ( "the estimate described the cycle that was sold; it must not be " - "reattached to the one the ledger can date itself") + "reattached to the one the ledger can date itself -- and the new " + "cycle's start is labelled by the evidence that supports it") + assert row["since"] == "2026-07-20", ( + "the rebuy is the start the ledger can prove, and it is the only " + "start this position may carry") + assert lg.derive_holdings(events)["holdings"]["NEWCO"]["cycle_id"] == \ + "NEWCO#2026-07-20#2", ( + "a rebought position opens its own cycle -- dated by the ledger " + "rather than by whichever day the user next declared, and " + "numbered so it is not the cycle it replaced") assert lg.derive_holdings(events)["holdings"]["NEWCO"]["cycle_id"] != "NEWCO#2025-01-15#1" @@ -361,8 +545,16 @@ def test_only_the_refresh_lane_may_write_engine_assigned_provenance(): """One privileged call site, and a mechanical count rather than a comment. `allow_engine_provenance` is a bypass of rule 1's gate. It is safe only - while exactly one caller uses it -- the one that assembled the envelope out - of answers the engine itself converted -- so the count is the check. + while exactly one *implementation* uses it -- the one that assembles every + value it passes, out of answers the engine itself converted or starts the + record already held -- so the count is the check. + + #539/#536 made the reading precise rather than looser. Two lanes now adopt a + book, and both reach `carry_recorded_starts`; what this counts is that + neither grew its own way in. A second literal, in this file or any other, + would be a second place a cycle start could be authored, which is the thing + #531 forbids -- and it would be invisible to every other test here, because + each lane's own behavior would look correct in isolation. """ engine_files = sorted(name for name in os.listdir(ENGINE) if name.endswith(".py")) users = {} @@ -848,9 +1040,16 @@ def test_a_stamped_position_keeps_the_canonical_current_book_usable(): assert basis.current_book["holdings"]["OLDCO"]["cycle_id"] == "OLDCO#unknown" projection = portfolio_basis.sizing_projection(basis) assert projection is not None and projection.applicable - stamped = [row for row in lg.latest_anchor(events)["positions"] - if row.get("since_basis")] - assert {row["ticker"] for row in stamped} == {"NEWCO", "OLDCO"} + stamped = {row["ticker"]: row.get("since_basis") + for row in lg.latest_anchor(events)["positions"]} + # #539: every adopted position states where its start came from, and the + # basis says what the date is worth. The two the user was just asked + # about keep their own answers; the three already on record keep the + # start the record held -- which is what stops their cycle ids reminting + # -- labelled as the lower bound a declaration date is, not upgraded. + assert stamped == {"NEWCO": "user_estimate", "OLDCO": "unknown", + "ACME": "snapshot_anchor", "WIDGET": "snapshot_anchor", + "BIGCO": "snapshot_anchor"} for row in basis.current_book["anchor"]["positions"]: assert not {"since", "since_basis"} & set(row), ( "the anchor projection carries book-affecting facts only; " @@ -1040,7 +1239,11 @@ def test_the_classification_enum_matches_the_engine_constant(): assert not unknown, f"{kind} allows an undeclared classification: {sorted(unknown)}" answer = (schema["$defs"]["input"]["properties"]["answers"]["items"]["properties"]) assert answer["held_months"]["maximum"] == br.HELD_MONTHS_MAX - assert set(lg.SINCE_BASES) == {"user_estimate", "unknown"} + # #539: four evidence classes, kept separable rather than collapsed into one + # "carried" marker. What a carried date is worth cannot be recovered later -- + # after adoption `origin` describes the snapshot writer, not the evidence. + assert set(lg.SINCE_BASES) == {"user_estimate", "unknown", + "trade_event", "snapshot_anchor"} assert lg.ENGINE_ASSIGNED_POSITION_KEYS <= lg.SNAPSHOT_POSITION_KEYS, ( "an engine-assigned field still has to be a field a position may carry") diff --git a/tests/test_ledger.py b/tests/test_ledger.py index 3c0177fe..9050009d 100644 --- a/tests/test_ledger.py +++ b/tests/test_ledger.py @@ -152,6 +152,41 @@ def test_an_unknown_start_becomes_the_cycle_id_the_engine_already_has(): assert not out["integrity"] +def test_every_dated_basis_reads_the_same_and_says_something_different(): + """#539: a basis states what a date is worth, and never changes the answer. + + Four evidence classes, three of them dated: a start the ledger watched open + (``trade_event``, exact), the day a declaration first put the position on the + books (``snapshot_anchor``, a lower bound), and the user's own approximation + (``user_estimate``). ``_anchored_cycle_start`` must read all three + identically -- same date, same cycle, no integrity note -- because the + difference between them is the ledger's honesty, not the arithmetic. Any + divergence would make the fix for a reminted cycle land as a *differently* + reminted one, which no test above would catch. + + The stored distinction is the other half, and it is why one carried marker + was refused: after an adoption ``origin`` describes the snapshot writer, so + an exact start and a lower bound that had been collapsed could never be told + apart again. + """ + def _read(basis): + out = lg.derive_holdings([_snap("2026-07-20", [ + {"ticker": "NVDA", "shares": 40, "avg_cost": 152.3, + "since": "2026-06-30", "since_basis": basis}])]) + assert not out["integrity"], basis + return out["holdings"]["NVDA"] + + dated = {basis: _read(basis) + for basis in ("trade_event", "snapshot_anchor", "user_estimate")} + for basis, holding in dated.items(): + assert holding["since"] == "2026-06-30", basis + assert holding["cycle_id"] == "NVDA#2026-06-30#1", basis + assert len({json.dumps(row, sort_keys=True) for row in dated.values()}) == 1, ( + "the three dated bases differ in what they claim about the date's " + "source and in nothing else; a reader that treated them differently " + "would be deciding arithmetic from provenance") + + def test_a_cycle_start_with_no_stamp_is_ignored_and_reported(): """A hand-edited ledger can put anything in the file. A date with no ``since_basis`` beside it is precisely the false precision the pairing rule diff --git a/tests/test_review_v2.py b/tests/test_review_v2.py index 48fcbc12..6cb1e586 100644 --- a/tests/test_review_v2.py +++ b/tests/test_review_v2.py @@ -1194,7 +1194,12 @@ def test_a_differing_second_declaration_is_recorded_first_then_reviewed(): derived = ledger_engine.derive_holdings(events)["holdings"] assert derived["PLTR"]["shares"] == 30, "holdings derive from the adopted anchor" assert derived["SPY"]["shares"] == 3, "post-adoption trades still apply on top" - assert derived["PLTR"]["cycle_id"] == "PLTR#2026-07-15#1" + # #539: adopting a newer view of a position that was never sold does not + # restart it. PLTR was on the books on 2026-07-10 and was added to on the + # 12th; the cycle the user's thesis is written against is still that one, + # so the declaration's own date does not become its start. + assert derived["PLTR"]["cycle_id"] == "PLTR#2026-07-10#1" + assert derived["SPY"]["cycle_id"] == "SPY#2026-07-10#1" # ...and only now is there a review. The same declaration that was # refused above reconciles clean against the book it just updated, so it @@ -1218,6 +1223,114 @@ def test_a_differing_second_declaration_is_recorded_first_then_reviewed(): "an identical finalize replay appends neither a second mark nor a second anchor" +def test_a_second_declaration_does_not_ask_for_theses_it_already_has(): + """#539's acceptance, end to end through the lane a user actually walks. + + The user declares their holdings and writes a thesis for each position -- + the most effortful thing a review asks of them. They declare again a few + weeks later, having sold nothing, and the review asks for the same theses + again: the snapshot-derived cycle ids were minted from each declaration's + own `as_of`, so the theses on file belonged to the previous generation of + ids and `missing_thesis_positions` was honestly reporting that none of the + current ids had one. + + Nothing here touches the question layer. `missing_thesis_positions` is still + `positions where cycle_id not in active`; what changed is that a position + nobody sold keeps its cycle id, so it finds the thesis that was always there. + """ + positions = [{"ticker": "SPY", "shares": 2, "avg_cost": 600, + "market": "US", "currency": "USD"}, + {"ticker": "PLTR", "shares": 20, "avg_cost": 30, + "market": "US", "currency": "USD"}] + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) / "coach" + first, _path = _snapshot_prepare( + tmp, root, payload={"as_of": "2026-06-30", "positions": positions}, + name="first.json") + assert sorted(row["cycle_id"] for row in first["missing_thesis_positions"]) == \ + ["PLTR#2026-06-30#1", "SPY#2026-06-30#1"], ( + "the opening declaration has no theses yet, so both are asked for") + assert _finalize_snapshot_session(tmp, root, first, "first").returncode == 0 + + moved = [dict(positions[0]), dict(positions[1], shares=21)] + second, _later = _snapshot_prepare( + tmp, root, payload={"as_of": "2026-07-28", "positions": moved}, + name="second.json") + assert second["missing_thesis_positions"] == [], ( + "they answered this in the first review and sold nothing since; " + "asking again is the product telling them it does not remember") + assert sorted(row["cycle_id"] for row in + second["state_snapshot"]["thesis_states"]) == \ + ["PLTR#2026-06-30#1", "SPY#2026-06-30#1"], ( + "and the theses are the same ones, still on the same cycles") + assert _finalize_snapshot_session(tmp, root, second, "second").returncode == 0 + events, _skipped = ledger_engine.load_ledger(str(root / "ledger.jsonl")) + assert sorted(row["cycle_id"] for row + in ledger_engine.derive_holdings(events)["holdings"].values()) == \ + ["PLTR#2026-06-30#1", "SPY#2026-06-30#1"], ( + "the recorded book agrees with the plan the user was shown") + + +def test_a_review_lane_adoption_keeps_the_holding_duration_the_user_answered(): + """#536: the review lane used to spend an answer the refresh lane collected. + + The user updates their book, is asked about a position that appeared, and + says they have held it about fourteen months. The refresh lane records that. + Later they hand over a holdings view whose only change is small -- an + ordinary path, adopted without ceremony -- and get a review. Before this, the + review lane rebuilt the anchor from the declared envelope, which structurally + cannot carry provenance, so the answered start reverted to a bookkeeping date + and every holding-period reading for that position quietly became wrong. They + were never told and never re-asked, because the position is held, not + appearing. + + The carry-forward has one implementation now, reached by both lanes, which is + this issue's third acceptance criterion. + """ + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) / "coach" + opening = [{"ticker": "SPY", "shares": 2, "avg_cost": 600, + "market": "US", "currency": "USD"}] + plan, _path = _snapshot_prepare( + tmp, root, payload={"as_of": "2026-06-30", "positions": opening}, + name="opening.json") + assert _finalize_snapshot_session(tmp, root, plan, "opening").returncode == 0 + + appeared = opening + [{"ticker": "NEWCO", "shares": 10, "avg_cost": 5.0, + "market": "US", "currency": "USD"}] + with_newco = _snapshot_json(tmp, payload={"as_of": "2026-07-15", + "positions": appeared}, + name="with-newco.json") + receipt = _refresh(str(root), str(with_newco)) + assert [row["ticker"] for row in receipt["pending_confirmations"]] == ["NEWCO"] + adopted = _refresh(str(root), str(with_newco), + {"refresh_id": receipt["refresh_id"], + "answers": [{"ticker": "NEWCO", "classification": "confirmed", + "held_months": 14}]}) + assert adopted["status"] == "adopted" + events, _skipped = ledger_engine.load_ledger(str(root / "ledger.jsonl")) + assert ledger_engine.derive_holdings(events)["holdings"]["NEWCO"]["cycle_id"] == \ + "NEWCO#2025-05-15#1", "the refresh lane records the answer correctly" + + # A small share change: `plan_refresh` raises nothing, so this reaches the + # review lane rather than being routed back (#530). + later = [dict(appeared[0]), dict(appeared[1], shares=11)] + review, _again = _snapshot_prepare( + tmp, root, payload={"as_of": "2026-07-28", "positions": later}, + name="later.json") + assert review["engine_state"]["holdings"]["positions"]["NEWCO"]["cycle_start"] == \ + "2025-05-15", ( + "the plan the user is questioned from must not restart a " + "position they answered for") + assert _finalize_snapshot_session(tmp, root, review, "later").returncode == 0 + events, _skipped = ledger_engine.load_ledger(str(root / "ledger.jsonl")) + holdings = ledger_engine.derive_holdings(events)["holdings"] + assert holdings["NEWCO"]["cycle_id"] == "NEWCO#2025-05-15#1", ( + "and the adopted book keeps it: an answer the product spent " + "silently is the same defect as one it never collected") + assert holdings["SPY"]["cycle_id"] == "SPY#2026-06-30#1" + + def test_second_snapshot_reconciled_marks_ledger_without_new_anchor(): """The #220 clean path: agreement appends only a content-addressed reconciliation mark; the anchor, ordering numbers, and repair stay stable."""