diff --git a/docs/maintainer-guide.md b/docs/maintainer-guide.md index 931a759f..9e79c8b1 100644 --- a/docs/maintainer-guide.md +++ b/docs/maintainer-guide.md @@ -154,7 +154,7 @@ When multiple sessions are active: | A card that names a reward with no way to claim it (#623) | Three different defects wearing one shape, and the fix differs per class — which is why `copy/*.json`'s `block_missing` cannot be treated as one surface. **An invitation** (`snapshot_unlock`, `rule_structural`) names the one answer this user's book cannot reach and nothing else; its rule is `references/decision-framing.md`'s "Earning the next piece of evidence" (#617), routed to from `references/card-policy.md` rather than restated, because two surfaces disagreeing about what an invitation owes is its own defect. Locked by `tests/test_doc_language.py`'s `CARD_INVITATION_REQUIRED_PHRASES` (per-phrase mutation proof) and by `tests/test_review_v2.py`'s retired-catalogue check across all three locales — the retired fragments are what is pinned, because a presence test on the new wording goes green the moment a feature list is appended back onto it. The counterweight matters as much as the rule: a complete behavioral review renders **no** invitation at all, and that absence is asserted, since a manufactured invitation is the same defect as a manufactured disclosure. **A disclosure with a product path** (`annualized_reconciliation`) names the path inline, in the user's own terms, beside the number it explains — never as a command and never promoted into the closing block, which would put two invitations on one card. **A disclosure with no user action at all** is the other 12 strings, correct as they stand and deliberately untouched. Branch tests assert the catalog entry, never a fragment of its wording; `tests/copy_corpus.py`'s golden owns what the sentences say, and `block_missing` is an unclaimed register there — claiming it would demand a scene for all 17 keys. | | A price-degraded card is a stated dead end, never a skipped step (#623 class 2) | `review._price_feed_status`'s `recovery` block is the single statement — `attempted` comes from whether an envelope or an explicit declaration actually arrived, never inferred from `provenance.mode`, because an envelope too broken to frame anything still leaves that mode `unavailable` and reading the attempt off it would call a real attempt a skipped one. Its readers: `review._refuse_a_card_built_on_a_skipped_price_recovery` (on the shared `_draft_bundle` path, so `finalize` called directly cannot walk around `preview`) ↔ `prepare --prices-unavailable` and its entry in `_fingerprint` ↔ `references/price-feed.md` ↔ `flows/first-review.md` step 0 and `flows/weekly-review.md` step 0 ↔ `review.cmd_add_cash` (which replays the declaration off the plan rather than re-typing it) ↔ `tests/test_review_v2.py`. Two rules. **The refusal is not the hard block #357 ruled out**: the user is asked for nothing and nothing waits on them — it is a gate on a step `flows/first-review.md` step 0 already required, clearable either by doing it or by declaring in one flag that it was done and found nothing. **The declaration arrives on a second `prepare`**, so it must be in the fingerprint (the #289/#369/#412 class, fourth instance) or the rerun resumes the undeclared pending session and the only thing separating a skipped step from an honest dead end is dropped; the session id legitimately does not move, because a statement about the outside world changes no engine number. | | The price recovery kit, and the two lanes that degrade in opposite directions (#629) | `review._price_feed_status` is the single builder of the kit — `provenance`, `request`, the `recovery` block above, and the instruction. Two routes call it and neither restates it: `cmd_prepare` (through the plan's `input.price_feed`) and `review._consider_price_feed_status`, which supplies only this route's own inputs and delegates. Only two things differ between them, and both are parameters rather than a second sentence: `command` (which subcommand takes the envelope back) and `request_path` (where the caller finds the manifest in the payload it is holding). `review._declared_prices_unavailable` is likewise the single reader of `--prices-unavailable`, now accepted by both commands, so one cannot accept a declaration the other refuses. Readers: `references/price-feed.md` ("Two lanes, two opposite rules" — the single statement of the asymmetry) ↔ `references/freeform-answers.md` (the agent-facing bound, and why the lookup is not the multi-tool production rule 8 forbids) ↔ `references/trade-consequence.md` ("When the engine could not price the book") ↔ `SKILL.md` rule 8 / `AGENTS.md` boundary 8 ↔ `tests/test_consider.py` section O. **The asymmetry is the point and must stay written down**: the identical `--prices-unavailable` declaration delivers the degraded card on the review lane and *refuses* on `consider`. A retrospective card's cost weights describe what the user actually paid; a forward concentration decision computed on cost describes a book that no longer exists — on `mock/sample_momentum.csv` the largest position moves more than thirteen points and the second and third by size invert. `test_o_the_recovery_kit_is_the_prepare_builder_rather_than_a_second_one` is the anti-fork gate: it rewrites `prepare`'s own sentence with those two substitutions and requires `consider`'s to start with it, so a hand-written second manifest reddens. Deliberately **not** gated: whether a host actually performs the lookup, the same instruction-only footing `docs/development-guide.md` §4 admits for the recommendation ban. | -| The cash anchor, and when it is asked for (#357, #507 knife 1) | `review.py`'s `_cash_anchor_status` is the single statement of whether a review has an accounting anchor — every other surface reads `review_plan.input.cash_anchor` rather than re-deriving it from `state["cash"]`. Its readers: `schemas/review-plan.schema.json` (`input.cash_anchor`, `additionalProperties: false`) ↔ `review.py` `cmd_add_cash` / `_cash_recompute_drift` ↔ `tools/ux_receipt.py`'s `CASH_OUTCOMES` and the outcome-dependent ordering gate ↔ `references/data-contract.md` (the one full statement of the beat) ↔ `references/ux-receipt.md` ↔ `references/interaction-delivery.md` ↔ `flows/first-review.md` and `flows/weekly-review.md` (each one short route-specific pointer — the two verbatim-identical preflight paragraphs #358 shipped were an unregistered hand mirror and are gone) ↔ `docs/qa-runbook.md` gate 3 ↔ `SKILL.md` "Fixed lifecycle" step 4 ↔ `tests/test_review_v2.py` + `tests/test_interaction_trajectory.py`. Four rules. **The engine states the gap in both directions**: with an anchor it already demanded a disclosure (`acct_perf_basis` in `required_honesty_keys`) and without one it demanded nothing at all, so the single condition the agent had to notice unprompted was the only one the plan never mentioned — five recurrences, three on real data. `not_applicable` is a positive claim with a `reason`, never an absent key, which is what makes the light-tier promise mechanical instead of depending on a flow paragraph asking before a tier check three lines later (#358 finding 1). **The timing is the opposite of `input.price_feed.request`'s, and `ask_after` says so**: a price gap is recovered before the user sees anything because every price-dependent number is degraded; a missing anchor degrades one pillar and leaves the rest byte-identical, so it is asked at the card beat where the user can see what answering buys (owner ruling 2026-07-30 against a `prepare` hard block; #507 principle 1). **The recompute proves it was the same review rather than asserting it**, and #665 is what that costs when the proof is aimed at the wrong thing. `add-cash` re-enters `_prepare_session` — never a second implementation of run-engine-ingest-plan — and the two halves below are what make that a recompute rather than a second review. **It reuses the frame instead of re-resolving it** (`args.amending_session` → `market_data.FROZEN_ENV` → `market_data.frame_frozen` → `_from_frozen_frame`, an *exact-request* same-day reuse that never reaches a provider on any outcome). The same-day cache was carrying that claim before and could not: `MarketDataBundle.covers` deliberately refuses to serve a request naming a symbol the stored bundle failed to price, so a transient outage gets its retry — and one unpriced symbol anywhere in the universe therefore made this command re-price the whole review at a second instant. **And the gate compares source facts, not derived objects.** `CASH_RECOMPUTE_SOURCE_FACTS` names them positively — the input files, the frozen frame, the recorded book, the diagnosis, the rules offered, the question identities already answered — because the predecessor asked the opposite question (exclude a hand-maintained set of cash-derived keys from `engine_state`/`engine_card`, then compare `state_snapshot` and `question_queue` **whole**) and that shape cannot be kept true: the re-entry's own downstream effects arrived as "drift", the refusal named four surfaces a cash anchor cannot reach, and a card-beat answer became unrecordable. Two rules for adding a row: it must be an input or a frozen observation rather than a projection of one, and it must be *measured* invariant under `--cash` on the same book (across five mock books an anchor moves `engine_state.cash`, `engine_card`'s `cash`/`acct_perf`/`honesty_ledger`, and `card_plan.required_honesty_keys`, and nothing else). The refusal names which of the gate's two verdicts it reached — "the facts moved", never merely that something did — and its counterpart `recompute.outcome: "anchor_propagated"` is emitted on success so the allowed outcome is stated too. A transaction file that grew is refused **before any write**, inside `_verify_and_ingest_frozen_trades` (`amending=True`, on the engine's own `fresh` dedup): a gate that only reads the recomputed plan has already appended the rows it is about to refuse. **No receipt outcome means "the agent decided not to ask"**: `skipped` was that value, and #357's fifth recurrence recorded it correctly and in order while the user was never offered the question, so the gate passed and the experience was identical to forgetting. | +| The cash anchor, and when it is asked for (#357, #507 knife 1) | `review.py`'s `_cash_anchor_status` is the single statement of whether a review has an accounting anchor — every other surface reads `review_plan.input.cash_anchor` rather than re-deriving it from `state["cash"]`. Its readers: `schemas/review-plan.schema.json` (`input.cash_anchor`, `additionalProperties: false`) ↔ `review.py` `cmd_add_cash` / `_cash_recompute_drift` ↔ `tools/ux_receipt.py`'s `CASH_OUTCOMES` and the outcome-dependent ordering gate ↔ `references/data-contract.md` (the one full statement of the beat) ↔ `references/ux-receipt.md` ↔ `references/interaction-delivery.md` ↔ `flows/first-review.md` and `flows/weekly-review.md` (each one short route-specific pointer — the two verbatim-identical preflight paragraphs #358 shipped were an unregistered hand mirror and are gone) ↔ `docs/qa-runbook.md` gate 3 ↔ `SKILL.md` "Fixed lifecycle" step 4 ↔ `tests/test_review_v2.py` + `tests/test_interaction_trajectory.py`. Four rules. **The engine states the gap in both directions**: with an anchor it already demanded a disclosure (`acct_perf_basis` in `required_honesty_keys`) and without one it demanded nothing at all, so the single condition the agent had to notice unprompted was the only one the plan never mentioned — five recurrences, three on real data. `not_applicable` is a positive claim with a `reason`, never an absent key, which is what makes the light-tier promise mechanical instead of depending on a flow paragraph asking before a tier check three lines later (#358 finding 1). **The timing is the opposite of `input.price_feed.request`'s, and `ask_after` says so**: a price gap is recovered before the user sees anything because every price-dependent number is degraded; a missing anchor degrades one pillar and leaves the rest byte-identical, so it is asked at the card beat where the user can see what answering buys (owner ruling 2026-07-30 against a `prepare` hard block; #507 principle 1). **The recompute proves it was the same review rather than asserting it**, and #665 is what that costs when the proof is aimed at the wrong thing. `add-cash` re-enters `_prepare_session` — never a second implementation of run-engine-ingest-plan — and the two halves below are what make that a recompute rather than a second review. **It reuses the frame instead of re-resolving it** (`args.amending_session` → `market_data.FROZEN_ENV` → `market_data.frame_frozen` → `_from_frozen_frame`, an *exact-request* same-day reuse that never reaches a provider on any outcome). The same-day cache was carrying that claim before and could not: `MarketDataBundle.covers` deliberately refuses to serve a request naming a symbol the stored bundle failed to price, so a transient outage gets its retry — and one unpriced symbol anywhere in the universe therefore made this command re-price the whole review at a second instant. **And the gate compares source facts, not derived objects.** `CASH_RECOMPUTE_SOURCE_FACTS` names them positively — the input files, the frozen frame, the recorded book, the diagnosis, the rules offered, the question identities already answered — because the predecessor asked the opposite question (exclude a hand-maintained set of cash-derived keys from `engine_state`/`engine_card`, then compare `state_snapshot` and `question_queue` **whole**) and that shape cannot be kept true: the re-entry's own downstream effects arrived as "drift", the refusal named four surfaces a cash anchor cannot reach, and a card-beat answer became unrecordable. Two rules for adding a row: it must be an input or a frozen observation rather than a projection of one, and it must be *measured* invariant under `--cash` on the same book (across five mock books an anchor moves `engine_state.cash`, `engine_card`'s `cash`/`acct_perf`/`honesty_ledger`, and `card_plan.required_honesty_keys`, and nothing else). The refusal names which of the gate's two verdicts it reached — "the facts moved", never merely that something did — and its counterpart `recompute.outcome: "anchor_propagated"` is emitted on success so the allowed outcome is stated too. A transaction file that grew is refused **before any write**, inside `_verify_and_ingest_frozen_trades` (`amending=True`, on the engine's own `fresh` dedup): a gate that only reads the recomputed plan has already appended the rows it is about to refuse. **No receipt outcome means "the agent decided not to ask"**: `skipped` was that value, and #357's fifth recurrence recorded it correctly and in order while the user was never offered the question, so the gate passed and the experience was identical to forgetting. #662 adds a second accepted input format without touching any of the above: `trade_recap.resolve_cash_anchor_input` is the single place a `percent_of_total` anchor is accepted and converted to the absolute-amount shape `cash_position` has always taken — called from `main()` against the same frozen `held_mv` the card was priced from, never a second market resolution, before `cash_position` ever sees the anchor, so the percentage is an input format and never a second stored kind. The denominator is the account's total value (cash plus position value), so the algebra is `p/(100-p) * held_mv`, never `p/100 * held_mv`, which would silently answer the position value alone. Two rules of its own: a non-finite or non-positive `held_mv` refuses rather than converting against a garbage denominator, and a multi-currency book accepts the percentage only as a single anchor in the aggregate currency (`currency_meta.aggregate_currency`), never inside the per-currency list — the smallest honest cut for a denominator no per-currency bucket can claim alone; the list itself, and every absolute-amount anchor, are untouched. `cmd_add_cash`'s response carries the disclosed derivation as `anchor_conversion` (percent, position value, currency, formula, amount) only when a percentage was converted, so an absolute-amount response is unchanged. Readers: `references/data-contract.md`'s cash-anchor paragraph ↔ `flows/first-review.md` step 6 / `flows/weekly-review.md` step 13 ↔ `tests/test_engine_units.py` (the algebra and the fail-closed cases) ↔ `tests/test_review_v2.py` (the CLI wiring) ↔ `tests/test_tr_json_contract.py`'s `STATE_KEYS` (the new `cash_anchor_conversion` state field). | | Behavior verdict store (#446 cut 1) | `horizon.py`'s existing `horizon_contradiction()` judgment ↔ `review.py` `_horizon_markers_all` (the untruncated computation; `_horizon_markers` is only its `HORIZON_MARKER_LIMIT`-bounded slice for the card's attention budget — the limit is a *display* cap, never a storage cap) ↔ `review.py` `_horizon_verdict_rows` (assembled only from what `_build_plan` already stamped into the plan — `engine_state`, `state_snapshot.thesis_states`, `state_snapshot.recent_exits` — never re-derived from disk or from a second, differently-sourced `active_cycle_ids`) ↔ `engine/verdicts.py` (`build_horizon_verdict`, the pure `replay()` contract, `RULE_PARAMS`/`RULE_PARAM_DIGEST`) ↔ `schemas/behavior-verdict.schema.json` (no `input` sub-object — every field is engine-assigned, the same reason `condition-check.schema.json` refuses `user_response` on its own envelope) ↔ `session.py`'s projection appending to `/verdicts.jsonl`, the same firewall `conditions.py:66-72` gives `conditions.jsonl` — a verdict never enters `problems.check_rules`' mechanical reconciliation ↔ `coach.py`'s `DATA_FILES` ↔ `tests/test_verdicts.py` (+ `tests/test_review_v2.py::test_all_json_schemas_parse`, `tests/test_coach_data_cli.py`). A verdict recomputes to the same `outcome` from exactly its five frozen scalar fields — `replay(rule.id, rule.version, subject.value, observed.value, observed.closed)` — forever, including after the thesis row `subject.row_id` names is revised (a revision writes a new `event_id`; the old one, and every verdict pointing at it, is never rewritten) or a future profile label `profile_label_id` references is superseded (that field is a back-reference only, load-bearing for nothing; `replay`'s signature carries no parameter for a labels store at all). The one way replay could fail silently — a threshold edited without a version bump — is blocked mechanically: `RULE_PARAM_DIGEST` pins `RULE_PARAMS`'s hash as a literal (never recomputed live, which would make the check vacuous), and a companion test cross-checks `RULE_PARAMS`'s live version against `horizon.py`'s own `EXIT_FAST`/`HELD_LONG` constants, so an edit landing in either file is caught. | | What `planned_entry` obliges the agent to collect, and what the honesty gate demands (#667) | `question_surface._INITIAL_THESIS_REQUIREMENTS["planned_entry"]` (the declared `answer_contract.requirements_by_choice`, read by the agent before it answers) and `review.py` `_validate_thesis_completeness`'s `inferred_planned` check (the enforced rule, read after the answer arrives) describe the same fact from two hand-written literals rather than one derivation — the same shape `new_evidence`'s `_ADD_REQUIREMENTS` / `thesis.build_decision_events` already accept for this reason, so this is not a new class of drift, only a new instance of one. If the validator's rule ever changes (a new required `thesis_updates` field, a different accepted `maturity` set), the declared requirement must change with it or the contract silently promises less than the gate demands again. `tests/test_question_surfaces.py::test_planned_entry_declares_its_thesis_capture_requirement_and_others_stay_empty` pins today's value; nothing mechanically ties the two sides together. | | A non-recoverable `consider` refusal's usable_facts packet (#674) | `review.py`'s `ReviewError.payload_extra` is the general mechanism (any raiser may attach a small deterministic extra, merged into `main()`'s emitted JSON beside `status`/`error`); `_usable_facts_snapshot` is the one function that populates it today, reading `last_state.json` through the existing `_previous_state` helper and filtering it to `CONSIDER_REFUSAL_CONCENTRATION_KEYS` (whole-book concentration) and `CONSIDER_REFUSAL_COMMITMENT_KEYS` (the frozen rule) — never recomputing, per the owner's ruling that this leaf may translate and contrast already-computed facts but not act as a second consequence engine. `_consider_rows`'s ledger-basis block is the only call site, wrapping exactly the three genuinely non-recoverable refusals (structural corruption, an unscopable integrity warning, no usable holding left) and deliberately excluding both the recoverable single-holding refusal `consequence.consequence` raises later (#673, unaffected) and `_canonical_consider_before`'s own defensive-floor sizing refusal (a fourth, likely-unreachable site this leaf does not touch — a candidate for a future cut, not this one). Readers: `references/trade-consequence.md` "When the whole book refuses" (the agent-facing contract: lead with a decision tension, cite only payload facts, frame at least two user-nominated options, never name one to sell, fall back to `references/decision-framing.md` when the packet is `null`) ↔ `evals/run_episodes.py`'s `usable_facts_grounding` check, whose own `CONSIDER_REFUSAL_CONCENTRATION_KEYS` mirrors review.py's constant by value rather than by import (review.py pulls in the rest of the engine by bare sibling import, so it cannot be `_load_module`-loaded the way `conditions.py`/`card_renderer.py` are) — locked together by a drift test in `tests/test_episode_checkers.py` ↔ `evals/episodes/EP-010-non-recoverable-refusal-narrated-instead-of-framed.json` ↔ `tests/test_consider.py`'s own suite of fixtures forcing each refusal shape. There is no schema file for the CLI's bare `{status, error}` envelope today, so `usable_facts` is documented and tested but not JSON-schema-validated — consistent with every other `ReviewError` message, none of which have one either. | diff --git a/skills/fomo-kernel/engine/review.py b/skills/fomo-kernel/engine/review.py index 8b7fbecb..800ffaa1 100644 --- a/skills/fomo-kernel/engine/review.py +++ b/skills/fomo-kernel/engine/review.py @@ -3950,10 +3950,20 @@ def _cash_anchor_status(state, route, cadence): "in that same message ask the user for the account's current cash balance in " + (", ".join(unanchored) or "the account's own currency") + " — stating what it unlocks and that skipping keeps the holdings-only view. " - "If they answer, run add-cash --session-id --cash '{\"currency\":\"\"," - "\"amount\":,\"as_of\":\"\"}' and continue on the session it " - "returns; it reuses this session's frozen prices rather than fetching new ones, " - "and refuses if the facts underneath the card moved. Never guess a balance")} + "Accept either an absolute currency amount or a percentage of the account's " + "total value (cash plus current position market value — never position value " + "alone). If they answer with a percentage, state that denominator in plain " + "words and get it confirmed once before converting — never more than one " + "clarification round-trip for this ask, and never compute the dollar figure " + "yourself. Then run add-cash --session-id --cash " + "'{\"currency\":\"\",\"amount\":,\"as_of\":\"\"}' for an " + "absolute amount, or --cash '{\"currency\":\"\",\"percent_of_total\":<0-100>," + "\"as_of\":\"\"}' for a percentage (#662) — the engine converts against " + "this session's own frozen position value and returns the derivation in " + "anchor_conversion, which you must show the user rather than silently apply. " + "Continue on the session add-cash returns; it reuses this session's frozen " + "prices rather than fetching new ones, and refuses if the facts underneath the " + "card moved. Never guess a balance")} def _build_plan(card, state, engine_meta, root, paths, route, language, fingerprint, nonce, persist, @@ -5969,23 +5979,36 @@ def cmd_add_cash(args): # `preview`/`finalize` would happily commit. shutil.rmtree(session.pending_dir(root, args.session_id), ignore_errors=True) recomputed_plan = (session.load_pending(root, result["session_id"]).get("plan") or {}) - _emit({"status": "anchored", "session_id": result["session_id"], - "superseded_session_id": args.session_id, - # #665: the gate has exactly two verdicts and both are stated. This is - # the allowed one — the anchor reached the account pillar and every - # fact underneath it held. The other is the refusal above, and the - # difference between them is what this command used to get wrong. - "recompute": {"outcome": "anchor_propagated", - "market_frame": "reused", - "source_facts_verified": [label for label, _read - in CASH_RECOMPUTE_SOURCE_FACTS]}, - "review_plan": _plan_for_agent(recomputed_plan), - "carried_forward": sorted(name for name, value in carried.items() if value is not None), - "next_action": ( - "the account pillar is now computed. Every required answer, thesis and frozen " - "question surface carried over unchanged, and card_plan.required_honesty_keys " - "gained the account-basis key — write one sentence for it in narrative.honesty, " - "then rerun preview and finalize on this session id")}) + # #662: present only when this --cash payload was a percentage (trade_recap's + # resolve_cash_anchor_input stamped it into engine_state); the absolute-amount + # path leaves this None, so the response below carries no new key at all -- + # byte-identical to before the percentage format existed. + anchor_conversion = (recomputed_plan.get("engine_state") or {}).get("cash_anchor_conversion") + response = {"status": "anchored", "session_id": result["session_id"], + "superseded_session_id": args.session_id, + # #665: the gate has exactly two verdicts and both are stated. This is + # the allowed one — the anchor reached the account pillar and every + # fact underneath it held. The other is the refusal above, and the + # difference between them is what this command used to get wrong. + "recompute": {"outcome": "anchor_propagated", + "market_frame": "reused", + "source_facts_verified": [label for label, _read + in CASH_RECOMPUTE_SOURCE_FACTS]}, + "review_plan": _plan_for_agent(recomputed_plan), + "carried_forward": sorted(name for name, value in carried.items() + if value is not None), + "next_action": ( + "the account pillar is now computed. Every required answer, thesis and " + "frozen question surface carried over unchanged, and " + "card_plan.required_honesty_keys gained the account-basis key — write one " + "sentence for it in narrative.honesty, then rerun preview and finalize on " + "this session id" + + (" — and show anchor_conversion's derivation to the user first: they " + "answered a percentage, so the stored dollar amount must not reach them " + "silently." if anchor_conversion else ""))} + if anchor_conversion: + response["anchor_conversion"] = anchor_conversion + _emit(response) def cmd_render(args): @@ -7939,8 +7962,11 @@ def build_parser(): add_cash.add_argument("--session-id", required=True) add_cash.add_argument("--root") add_cash.add_argument("--cash", required=True, - help="TR_CASH JSON string: one {currency,amount,as_of} anchor, " - "or a list of them for a multi-currency account") + help="TR_CASH JSON string: one {currency,amount,as_of} anchor, one " + "{currency,percent_of_total,as_of} anchor converted against this " + "session's own frozen position value and disclosed in the " + "response's anchor_conversion (#662), or a list of absolute-" + "amount anchors for a multi-currency account") add_cash.add_argument("--prices", help="the same agent-supplied price envelope this session was " "prepared with; omit when prepare fetched its own") diff --git a/skills/fomo-kernel/engine/trade_recap.py b/skills/fomo-kernel/engine/trade_recap.py index b202ee5b..05ed229d 100644 --- a/skills/fomo-kernel/engine/trade_recap.py +++ b/skills/fomo-kernel/engine/trade_recap.py @@ -7,7 +7,7 @@ 用法：python3 trade_recap.py [trades.csv ...] (預設吃 ../mock/mock_trades.csv) 隱私：本檔不含任何真實帳戶路徑;預設只跑 mock 資料。用戶自己的 CSV 由參數傳入,留在本機。 """ -import csv, os, re, sys, statistics, datetime as dt +import csv, math, os, re, sys, statistics, datetime as dt from collections import Counter, defaultdict, deque import instruments as instrument_policy import market_context as market_context_engine @@ -262,6 +262,115 @@ def _cash_balance_one_ccy(flows, anchor, prev_end): return sum(cf["amount"] for cf in flows), "csv_sum", False +class CashAnchorInputError(ValueError): + """A ``--cash`` payload could not be resolved into a storable anchor (#662). + + Raised only for the percentage input format: the absolute-amount + ``{currency, amount, as_of}`` shape is unchanged since #171 and never + reaches this class. Caught at the process frame beside + ``MissingAggregateCurrencyRate`` so a malformed or unconvertible + percentage fails the whole run rather than silently reaching + ``cash_position`` as an anchor it cannot use (no ``amount`` key, read as + no anchor at all). + """ + + +def resolve_cash_anchor_input(raw_anchor, held_mv, aggregate_currency): + """Resolve a ``--cash`` payload into the absolute-amount shape ``cash_position`` accepts. + + ``raw_anchor`` is the parsed ``--cash`` JSON, exactly as it reaches + ``cash_position`` today: one ``{currency, amount, as_of}`` anchor, or a + list of them for a multi-currency account. #662 adds a second, single- + anchor input format the user may answer the cash-anchor ask with instead + of an absolute amount: ``{currency, percent_of_total, as_of}``. This is + the one place that format is accepted and converted — the agent never + does this arithmetic itself, and ``cash_position`` never sees a + percentage, so the stored anchor is always an absolute amount regardless + of which format the user answered in (the percentage is an input format, + never a second stored kind). + + The denominator a percentage answer names is this account's total value — + cash plus current position market value — never the position value alone + (owner ruling 2026-08-01, #662). Solving for cash given that denominator + is why the algebra is ``p/(100-p)`` and not ``p/100``: a p% cash answer + means cash is p% of (cash + held_mv), so:: + + cash = (p/100) * (cash + held_mv) + cash * (1 - p/100) = (p/100) * held_mv + cash = p / (100 - p) * held_mv + + A 30% answer against a 70,000 position value is 30,000 against a 100,000 + total, never the 21,000 a p/100 misreading of the denominator (position + value alone) would produce. + + ``held_mv`` must be this session's own frozen position market value — the + caller (``main``) computes it from the same frozen prices the review was + rendered from, never a freshly resolved one, so a percentage answered on + an amended review converts against the numbers the user actually saw. + Fails closed rather than converting against a garbage denominator: a + non-finite or non-positive ``held_mv`` refuses, the same zero/invalid- + denominator rule this book keeps everywhere else (AGENTS.md boundary 6). + + ``aggregate_currency`` is the currency ``held_mv`` is measured in — USD + for a mixed-currency book, the book's own single currency otherwise + (``trade_recap.usd_view``). A multi-currency cash book cannot say which + currency's bucket a whole-account percentage belongs to, so the smallest + honest cut is a single anchor in that same aggregate currency; a + percentage anchor whose stated ``currency`` disagrees is refused rather + than silently relabelled or silently trusted. + + Returns ``(resolved, derivation)``. ``resolved`` is byte-identical to + ``raw_anchor`` whenever it carried no percentage — the absolute-amount + path, unchanged. ``derivation`` is ``None`` on that path and otherwise the + disclosure the caller must surface rather than keep to itself: the + percent, the position value and currency it was measured against, the + formula, and the resulting amount. + """ + items = raw_anchor if isinstance(raw_anchor, list) else [raw_anchor] + percent_items = [item for item in items + if isinstance(item, dict) and "percent_of_total" in item] + if not percent_items: + return raw_anchor, None + if isinstance(raw_anchor, list) or len(percent_items) > 1: + raise CashAnchorInputError( + "percent_of_total is accepted for a single cash anchor only, never inside a " + "multi-currency list; answer each currency's balance with an absolute amount instead") + anchor = percent_items[0] + if "amount" in anchor: + raise CashAnchorInputError( + "--cash carries both amount and percent_of_total; supply exactly one") + percent = anchor.get("percent_of_total") + if isinstance(percent, bool) or not isinstance(percent, (int, float)) or not (0 < percent < 100): + raise CashAnchorInputError( + f"percent_of_total must be a number strictly between 0 and 100, got {percent!r}") + if not anchor.get("as_of"): + raise CashAnchorInputError("a percent_of_total anchor still needs as_of, like an " + "absolute-amount anchor") + currency = (anchor.get("currency") or "").strip().upper() + aggregate = (aggregate_currency or "").strip().upper() + if not currency: + raise CashAnchorInputError("a percent_of_total anchor still needs currency, like an " + "absolute-amount anchor") + if currency != aggregate: + raise CashAnchorInputError( + f"percent_of_total is measured against this review's position value in " + f"{aggregate_currency}, not {currency}; state the percentage in {aggregate_currency} " + "or answer with an absolute amount in that currency instead") + if (not isinstance(held_mv, (int, float)) or isinstance(held_mv, bool) + or not math.isfinite(held_mv) or held_mv <= 0): + raise CashAnchorInputError( + "percent_of_total needs this review's position market value to convert against, and " + f"it is not usable ({held_mv!r}); answer with an absolute amount instead") + amount = round(percent / (100.0 - percent) * held_mv, 2) + resolved = {key: value for key, value in anchor.items() if key != "percent_of_total"} + resolved["amount"] = amount + derivation = {"percent_of_total": percent, "position_value": round(held_mv, 2), + "currency": currency, + "formula": "amount = percent_of_total / (100 - percent_of_total) * position_value", + "amount": amount} + return resolved, derivation + + def cash_position(cash_flows, held_mv, anchor=None, prev_end=None, fx=None): """帳戶現金地基（#171 PR-1；多幣別現金桶）。 per-currency 各算餘額（錨點+其後現金流 or csv_sum），用 fx 聚合成 USD total—— @@ -2210,7 +2319,7 @@ def build_state(rows, rts, held, dims, overview, ab, rx, currency_meta=None, portfolio_structure=None, price_snapshot=None, market_context=None, max_pos_override=None, price_provenance=None, price_request=None, prices=None, valuation_frame=None, splits=None, - splits_window=None): + splits_window=None, cash_anchor_conversion=None): """把這次復盤收斂成一張薄 JSON 狀態,給「下次對帳上次規矩」用(非給人看的卡)。 只在 main() 偵測 TR_STATE_OUT 時呼叫並寫出;不設 → 完全不執行,引擎行為零變。 設計依 requirements §4/§10: @@ -2341,6 +2450,10 @@ def build_state(rows, rts, held, dims, overview, ab, rx, currency_meta=None, "positions": holdings, }, "cash": cash, # #171 PR-1:帳戶現金地基(balance/weight/source/reliable/recent_net_deposit)。None=未提供;source=csv_sum+reliable=False=無錨點靠 Σamount 近似(honesty 揭露) + # #662:僅在這次傳入的 --cash 是百分比(percent_of_total)時非 None——換算揭露 + # (percent/position_value/currency/formula/amount),供 cmd_add_cash 轉呈給 agent。 + # 不參與 review._cash_recompute_drift 比對(該 gate 的鍵表本就不含 cash)。 + "cash_anchor_conversion": cash_anchor_conversion, "price_snapshot": price_snapshot, # #191 PR B:review-time prices for deterministic exit/swap comparison; frozen in the session plan # #550:這次實際套用的分割事件。帳本存的是「當時成交的股數」(正確,不該改), # 所以任何跨分割累加股數的讀者都必須拿到同一份分割表,否則 90 股(分割前)減 @@ -2872,7 +2985,8 @@ def main(): } # 帳戶現金地基（#171）：現金流 + 現金餘額錨點 → cash_position（多幣別 per-currency 各算再 fx 聚合）。 - # held_mv = 持倉市值（聚合幣別 USD，無現價用成本近似，同 dim_diversify）= cash_weight 分母。 + # held_mv = 持倉市值（聚合幣別 USD，無現價用成本近似，同 dim_diversify）= cash_weight 分母， + # 也是 #662 百分比現金輸入換算絕對金額所用的同一份分母（用凍結價格算出，非重新抓價）。 import json # per-currency（每筆帶 currency）；聚合由 cash_position 內部做。帶 rows 進去讓「來源 # 沒有 Amount 欄」時能用 qty×price 估出交易的現金足跡（#375），而不是整條路打死。 @@ -2883,6 +2997,15 @@ def main(): cash_anchor = json.loads(_ca) if _ca else None except (ValueError, TypeError): cash_anchor = None + # #662: a percentage anchor ({currency, percent_of_total, as_of}) is + # resolved into an absolute amount right here, before cash_position ever + # sees it -- the stored anchor is always an absolute amount, and the + # agent never does this arithmetic itself. cash_anchor_conversion is None + # on the (unchanged) absolute-amount path and is otherwise the disclosure + # the CLI layer surfaces to the agent. + cash_anchor, cash_anchor_conversion = ( + resolve_cash_anchor_input(cash_anchor, held_mv, currency_meta["aggregate_currency"]) + if cash_anchor is not None else (cash_anchor, None)) # 多幣別：cash_position 內部 per-currency 各算餘額再用 fx 聚合（台美各帳戶各自錨點）；單幣 fx=None → 因子 1.0。 cash_data = cash_position(cash_flows, held_mv, anchor=cash_anchor, prev_end=prev_end, @@ -3023,7 +3146,8 @@ def main(): price_provenance=price_provenance, price_request=price_request, prices=px, valuation_frame=frame, splits=splits, splits_window={"start": bundle.window["start"], - "rebase_origin": bundle.rebase_origin}) + "rebase_origin": bundle.rebase_origin}, + cash_anchor_conversion=cash_anchor_conversion) # prev_end(#270 解過的值,上次 review 的 date_end,已排除同週重跑自我別名)→ # behavior 型問題事件只取其後的新交易(weekly 增量);None = 初診全期補齊,問題帳統計冷啟動。 outdir = os.path.dirname(os.path.abspath(path)) or "." @@ -3039,8 +3163,12 @@ def main(): # of the card, the state write and the ledger ingest, so nothing has been # produced yet. Caught at the process frame rather than at the call site so # any later aggregation path in `main()` inherits the same single error line. + # #662's CashAnchorInputError joins it here for the same reason: a + # percentage anchor that cannot be converted (bad range, no usable + # position value) must fail the whole run rather than reach cash_position + # as a silently unusable anchor. try: main() - except MissingAggregateCurrencyRate as exc: + except (MissingAggregateCurrencyRate, CashAnchorInputError) as exc: print(f"❌ {exc}", file=sys.stderr) sys.exit(1) diff --git a/skills/fomo-kernel/flows/first-review.md b/skills/fomo-kernel/flows/first-review.md index c9bf627c..26583c75 100644 --- a/skills/fomo-kernel/flows/first-review.md +++ b/skills/fomo-kernel/flows/first-review.md @@ -36,7 +36,7 @@ One choice does not tolerate the inferred default: `planned_entry` asserts a rea Ask the user to choose one candidate rule, write their own, or skip. Present each candidate's engine-authored `grounding` sentence verbatim when the payload carries one — that is what ties a generic rule to this user's real positions — and never invent a grounding for a candidate that has none. When `card_plan.candidate_comparison` is present, show that one sentence once alongside the candidates: it explains why the others ranked lower on this period's severity ranking, not which rule is objectively right for this user, so do not rephrase it into an endorsement. -When `input.cash_anchor.status` is `absent` or `partial`, ask for the account's current cash balance in the same message, in the currencies it names — stating what answering unlocks and that skipping keeps the holdings-only view. Do not settle for pointing at the card's own unlock invitation: that sentence is not an interaction point, and a finished card that names a gap the user cannot act on is the miss this step exists to close. Record `provided` or `declined` (`references/ux-receipt.md`); if they answered, run `add-cash`, rerun `preview` on the session it returns, and show that card before the commitment is written. +When `input.cash_anchor.status` is `absent` or `partial`, ask for the account's current cash balance in the same message, in the currencies it names — accepting either an absolute amount or a percentage of the account's total value (cash plus current positions, stated in those plain words so the denominator is never assumed), confirmed once before converting if they answer with a percentage (`references/data-contract.md`) — stating what answering unlocks and that skipping keeps the holdings-only view. Do not settle for pointing at the card's own unlock invitation: that sentence is not an interaction point, and a finished card that names a gap the user cannot act on is the miss this step exists to close. Record `provided` or `declined` (`references/ux-receipt.md`); if they answered, run `add-cash` (the engine converts and discloses a percentage's derivation — never do that arithmetic yourself), rerun `preview` on the session it returns, and show that card before the commitment is written. If the rule they write names a quantity the engine does not compute, it is stored rather than refused — look the quantity up in that same exchange, show the value back, and send it as a condition slot (`references/condition-slots.md`). diff --git a/skills/fomo-kernel/flows/weekly-review.md b/skills/fomo-kernel/flows/weekly-review.md index 84d6ae1d..db79e592 100644 --- a/skills/fomo-kernel/flows/weekly-review.md +++ b/skills/fomo-kernel/flows/weekly-review.md @@ -34,6 +34,6 @@ The queue is the engine's ranking, so do not change route, kind, priority, requi **12. Focus the narrative on movement against the previous rule and the largest new leak.** Cover every `card_plan.required_honesty_keys` entry in `narrative.honesty`. Frame coverage gaps the engine never asked about as neutral facts per `authoring_contract.narrative.unprompted_gaps`. Optionally add `synthesis` — a closing block connecting facts across sections into one point of view; omit it if it only restates a number. Do not produce a full dashboard. -**13. After preview, one beat: the card, the rule choice, and the cash question if one is owed.** One message — the complete card inline first (`references/card-delivery.md`), the choices under it; record the card presentation, then the rule choice (`references/ux-receipt.md`). The user still sees the real card before committing; they no longer wait through a second round for candidates the engine had already computed. Present each candidate's engine-authored `grounding` verbatim when the payload carries one, and never invent one. When `card_plan.candidate_comparison` is present, show that sentence once alongside the candidates — it explains why the others ranked lower this period, not which rule is objectively right, so do not rephrase it into an endorsement. When `input.cash_anchor.status` is `absent` or `partial`, ask for the account's cash balance in the same message, stating what it unlocks and what skipping keeps; record `provided` or `declined`, and on an answer run `add-cash` and rerun `preview` on the session it returns before the commitment is written. If the user states their own single-position cap, record it with `review.py set-cap`. If the rule they write names a quantity the engine does not compute, it is stored rather than refused: look the quantity up in that same exchange, show the value back, and send it as a condition slot (`references/condition-slots.md`). Finalize atomically; update legacy state only through projections. +**13. After preview, one beat: the card, the rule choice, and the cash question if one is owed.** One message — the complete card inline first (`references/card-delivery.md`), the choices under it; record the card presentation, then the rule choice (`references/ux-receipt.md`). The user still sees the real card before committing; they no longer wait through a second round for candidates the engine had already computed. Present each candidate's engine-authored `grounding` verbatim when the payload carries one, and never invent one. When `card_plan.candidate_comparison` is present, show that sentence once alongside the candidates — it explains why the others ranked lower this period, not which rule is objectively right, so do not rephrase it into an endorsement. When `input.cash_anchor.status` is `absent` or `partial`, ask for the account's cash balance in the same message — an absolute amount or a percentage of the account's total value (cash plus current positions, named in those plain words), confirmed once before converting if they answer with a percentage (`references/data-contract.md`) — stating what it unlocks and what skipping keeps; record `provided` or `declined`, and on an answer run `add-cash` (which converts and discloses a percentage's derivation itself — never do that arithmetic yourself) and rerun `preview` on the session it returns before the commitment is written. If the user states their own single-position cap, record it with `review.py set-cap`. If the rule they write names a quantity the engine does not compute, it is stored rather than refused: look the quantity up in that same exchange, show the value back, and send it as a condition slot (`references/condition-slots.md`). Finalize atomically; update legacy state only through projections. Do not ask for an already confirmed motive every week. Prepare requeues one only for a new cycle, new behavior, or an inferred answer that remains the largest contradiction. diff --git a/skills/fomo-kernel/references/data-contract.md b/skills/fomo-kernel/references/data-contract.md index bb9cd291..a20be8fd 100644 --- a/skills/fomo-kernel/references/data-contract.md +++ b/skills/fomo-kernel/references/data-contract.md @@ -71,14 +71,19 @@ When no balance appears anywhere, **the engine says so and the ask waits for the `ask_after` is the whole point, and it is the opposite of `input.price_feed.request` sitting beside it. A price gap is recovered *before* the user is shown anything, because every price-dependent number is degraded without it. A missing cash anchor degrades one pillar and leaves every other number identical, so asking first buys nothing and spends the user's attention before they have seen anything (#507 principle 1). Ask in the same message as the card, alongside the rule choice: state what answering unlocks and that skipping keeps the holdings-only view, so the ask is informed rather than blind. +The ask accepts either an absolute currency amount or a percentage of the account's total value — cash plus current position market value, never the position value alone (#662). State that denominator in plain words in the same ask, so the user is never left guessing which one is assumed, and if they answer with a percentage, get that denominator confirmed once before converting — never more than one free-form clarification round-trip for this ask, the friction the three-round-trip trigger of #662 exists to close. The engine, never the agent, does the conversion: pass the percentage straight through to `add-cash` and read the disclosed amount back from the response (below); do not compute the dollar figure yourself and pass off your own arithmetic as the user's answer. + If they answer, recompute in place: ```bash python3 engine/review.py add-cash --session-id \ --cash '{"currency":"USD","amount":8200,"as_of":""}' +# or, for a percentage of the account's total value: +python3 engine/review.py add-cash --session-id \ + --cash '{"currency":"USD","percent_of_total":30,"as_of":""}' ``` -This re-enters the same review with the anchor added, and it reuses that session's frozen prices rather than resolving new ones — the user answered against the card those numbers rendered, so a second observation instant would be a different review wearing the same session (#665). Two outcomes, and the response names which: the anchor propagated into the account pillar (`recompute.outcome: "anchor_propagated"`), or the facts underneath it moved and the command refuses. What counts as moving is the source facts, not the anchor's own downstream effects: the input files, that frozen frame, the recorded book, the diagnosis, the rules offered, and the questions already answered. A transaction file that grew since the card was rendered is refused before anything is written at all. Answers, narrative, and frozen question surfaces carry over untouched; the returned session id supersedes the one you passed, and `card_plan.required_honesty_keys` gains the account-basis key, which needs one more sentence in `narrative.honesty`. Rerun `preview` on the returned session and show the recomputed card. If they skip, the card keeps its holdings pillar and its unlock invitation exactly as before, and the review finishes normally — skipping is a real answer, not a failure. +This re-enters the same review with the anchor added, and it reuses that session's frozen prices rather than resolving new ones — the user answered against the card those numbers rendered, so a second observation instant would be a different review wearing the same session (#665). A percentage anchor converts against that same frozen position value, never a freshly resolved one, and the response's `anchor_conversion` discloses the percent, the position value and currency it was measured against, the formula, and the resulting amount — show it to the user rather than applying it silently. The algebra follows from the denominator: with cash counted in the account's total, a p% answer means `cash = p/(100-p) * position_value`, not `p/100 * position_value` (30% against a 70,000 position value is 30,000 on a 100,000 total, never the 21,000 that misreads the denominator as the position value alone). A multi-currency book accepts a percentage as a single anchor in the account's own aggregate currency only (`currency_meta.aggregate_currency`) — per-currency absolute anchors are unaffected and keep working exactly as before. A book with no usable position value — no position currently held — refuses a percentage rather than converting against nothing; supply an absolute amount instead. Two outcomes, and the response names which: the anchor propagated into the account pillar (`recompute.outcome: "anchor_propagated"`), or the facts underneath it moved and the command refuses. What counts as moving is the source facts, not the anchor's own downstream effects: the input files, that frozen frame, the recorded book, the diagnosis, the rules offered, and the questions already answered. A transaction file that grew since the card was rendered is refused before anything is written at all. Answers, narrative, and frozen question surfaces carry over untouched; the returned session id supersedes the one you passed, and `card_plan.required_honesty_keys` gains the account-basis key, which needs one more sentence in `narrative.honesty`. Rerun `preview` on the returned session and show the recomputed card. If they skip, the card keeps its holdings pillar and its unlock invitation exactly as before, and the review finishes normally — skipping is a real answer, not a failure. On `first_review` and full-tier `weekly_review`, record which of the three happened (`cash_anchor_checked`, `references/ux-receipt.md`): `found_in_source` before the first question or card; `declined` after the card the question was attached to, since nothing recomputes and that card stands as settled; `provided` *before* the recorded preview pair instead, since providing always recomputes and the settled card — the one this trace's exactly-once preview pair must record — is the one rendered after that recompute, not the one that asked (#663). There is no outcome meaning "the agent decided not to ask" — that is what made a run where the user never got the chance look identical to one where they declined (#357, fifth recurrence). diff --git a/tests/test_engine_units.py b/tests/test_engine_units.py index 2a16357d..358471fd 100644 --- a/tests/test_engine_units.py +++ b/tests/test_engine_units.py @@ -1649,6 +1649,148 @@ def test_cash_position_single_dict_anchor_backward_compat(): assert cp["by_currency"]["USD"]["balance"] == 6000.0 +# --- #662: a --cash payload may be an absolute amount or a percentage of the - +# account's total value (cash + current position market value), and the engine +# -- never the agent -- owns the conversion. resolve_cash_anchor_input is the +# one place a percentage is accepted and turned into the absolute-amount shape +# cash_position has always taken; these tests pin the algebra, the disclosure, +# the absolute-amount regression path, and the fail-closed cases. + +def test_resolve_cash_anchor_input_converts_percent_with_the_correct_denominator(): + """The load-bearing algebra (#662 owner ruling 2026-08-01). + + The denominator a percentage answer names is the account's TOTAL value -- + cash plus position value -- never the position value alone. Solving cash = + p% * (cash + position_value) for cash gives p/(100-p) * position_value, not + the p/100 * position_value a reader who forgets cash is IN the denominator + would reach. 30% against a 70,000 position is 30,000 (so the 100,000 total + is exactly 30% cash) -- never 21,000, which is what 30% of the position + value alone (the wrong denominator) would produce. + """ + resolved, derivation = tr.resolve_cash_anchor_input( + {"currency": "USD", "percent_of_total": 30, "as_of": "2026-07-30"}, + held_mv=70000.0, aggregate_currency="USD") + assert resolved["amount"] == 30000.0, resolved["amount"] + assert resolved["amount"] != 21000.0, "30% of the position value alone is the wrong reading" + total = resolved["amount"] + 70000.0 + assert _approx(total, 100000.0), total + assert _approx(resolved["amount"] / total, 0.30), resolved["amount"] / total + # The absolute-amount shape cash_position has always accepted: currency, + # amount, as_of -- and nothing named percent_of_total left behind. + assert resolved == {"currency": "USD", "amount": 30000.0, "as_of": "2026-07-30"}, resolved + + +def test_resolve_cash_anchor_input_discloses_the_derivation(): + """The engine must show its work, not just the number (#662). + + The agent never does this arithmetic itself, so the disclosure has to + carry everything a reader needs to check it: the percent asked about, the + position value it was measured against (and that value's currency), the + formula, and the resulting stored amount. + """ + _resolved, derivation = tr.resolve_cash_anchor_input( + {"currency": "USD", "percent_of_total": 30, "as_of": "2026-07-30"}, + held_mv=70000.0, aggregate_currency="USD") + assert derivation is not None, ( + "a percentage anchor was converted but the derivation was not disclosed -- the agent " + "would apply the stored amount without ever seeing how it was computed") + assert derivation["percent_of_total"] == 30 + assert derivation["position_value"] == 70000.0 + assert derivation["currency"] == "USD" + assert derivation["amount"] == 30000.0 + assert "100" in derivation["formula"] and "percent_of_total" in derivation["formula"] + + +def test_resolve_cash_anchor_input_is_a_no_op_for_an_absolute_amount(): + """Regression: the pre-#662 shape must be untouched, not merely equal. + + Returning the exact same object (not a copy, not a re-serialization) is + the strongest statement that the absolute-amount path was never touched by + the new percentage branch.""" + raw = {"currency": "USD", "amount": 8200, "as_of": "2026-07-30"} + resolved, derivation = tr.resolve_cash_anchor_input(raw, held_mv=70000.0, + aggregate_currency="USD") + assert resolved is raw, "the absolute-amount anchor must pass through unchanged" + assert derivation is None, "no derivation to disclose when nothing was converted" + + +def test_resolve_cash_anchor_input_multi_currency_list_is_a_no_op(): + """Regression: a per-currency list -- #171's multi-account shape -- is + untouched when none of its entries name a percentage.""" + raw = [{"currency": "USD", "amount": 100.0, "as_of": "2026-07-30"}, + {"currency": "TWD", "amount": 2000.0, "as_of": "2026-07-30"}] + resolved, derivation = tr.resolve_cash_anchor_input(raw, held_mv=70000.0, + aggregate_currency="USD") + assert resolved is raw + assert derivation is None + + +def test_resolve_cash_anchor_input_refuses_a_non_positive_held_mv(): + """Fails closed rather than converting against a garbage denominator + (#662) -- zero, negative, non-finite, and missing all refuse the same + way a missing/incompatible valuation refuses elsewhere (AGENTS.md + boundary 6), instead of silently producing a zero or nonsensical amount.""" + anchor = {"currency": "USD", "percent_of_total": 30, "as_of": "2026-07-30"} + for bad_held_mv in (0.0, -1.0, float("nan"), float("inf"), None): + try: + tr.resolve_cash_anchor_input(anchor, held_mv=bad_held_mv, aggregate_currency="USD") + except tr.CashAnchorInputError: + pass + else: + raise AssertionError(f"held_mv={bad_held_mv!r} must refuse, never convert") + + +def test_resolve_cash_anchor_input_refuses_a_percent_outside_zero_to_hundred(): + for bad_percent in (0, 100, 100.0, -5, 150, True): + anchor = {"currency": "USD", "percent_of_total": bad_percent, "as_of": "2026-07-30"} + try: + tr.resolve_cash_anchor_input(anchor, held_mv=70000.0, aggregate_currency="USD") + except tr.CashAnchorInputError: + pass + else: + raise AssertionError(f"percent_of_total={bad_percent!r} must refuse") + + +def test_resolve_cash_anchor_input_refuses_both_amount_and_percent(): + anchor = {"currency": "USD", "amount": 1.0, "percent_of_total": 30, "as_of": "2026-07-30"} + try: + tr.resolve_cash_anchor_input(anchor, held_mv=70000.0, aggregate_currency="USD") + except tr.CashAnchorInputError as exc: + assert "amount" in str(exc) and "percent_of_total" in str(exc) + else: + raise AssertionError("carrying both amount and percent_of_total must refuse") + + +def test_resolve_cash_anchor_input_refuses_inside_a_multi_currency_list(): + """The smallest honest cut for a multi-currency book (#662): a percentage + names the account's total, which cannot be split across an unspecified set + of per-currency buckets, so it is only accepted as a single anchor -- never + inside the list a multi-currency account otherwise answers with.""" + mixed = [{"currency": "USD", "percent_of_total": 30, "as_of": "2026-07-30"}, + {"currency": "TWD", "amount": 2000.0, "as_of": "2026-07-30"}] + try: + tr.resolve_cash_anchor_input(mixed, held_mv=70000.0, aggregate_currency="USD") + except tr.CashAnchorInputError as exc: + assert "list" in str(exc) + else: + raise AssertionError("a percentage inside a multi-currency list must refuse") + + +def test_resolve_cash_anchor_input_refuses_a_currency_that_is_not_the_aggregate(): + """A percentage is measured against held_mv, which is itself denominated in + the book's aggregate currency (USD for a mixed book, trade_recap.usd_view). + Accepting a mismatched currency label would silently mislabel that amount + -- the exact class of bug #649 fought at the holdings aggregate -- so this + refuses instead of relabelling or trusting the caller's stated currency.""" + anchor = {"currency": "TWD", "percent_of_total": 30, "as_of": "2026-07-30"} + try: + tr.resolve_cash_anchor_input(anchor, held_mv=70000.0, aggregate_currency="USD") + except tr.CashAnchorInputError as exc: + assert "USD" in str(exc) and "TWD" in str(exc) + else: + raise AssertionError("a percentage stated in a non-aggregate currency must refuse") + + def test_honesty_ledger_cash_reliability_trigger(): """#171 呈現層:cash_reliability 只在『有可誤導的 weight 但不可信』時進 ledger。 reliable(錨點)→ 不觸發;weight=None(算不出,不上卡)→ 不觸發(無可誤導數字); diff --git a/tests/test_review_v2.py b/tests/test_review_v2.py index 7201c83b..e784d366 100644 --- a/tests/test_review_v2.py +++ b/tests/test_review_v2.py @@ -2344,6 +2344,124 @@ def test_replaying_the_same_cash_anchor_changes_nothing(): "no second pending session survives the replay" +# --- #662: the cash-anchor ask accepts an absolute amount or a percentage --- +# +# A user answering "30%" used to cost three free-form clarification round +# trips (#662's trigger). Owner disposition 2026-08-01: accept both formats; +# the denominator a percentage names is the account's TOTAL value (cash plus +# current position market value), stated in plain words and confirmed once; +# the engine -- never the agent -- converts and discloses the derivation. + +def test_add_cash_converts_a_percentage_against_this_sessions_frozen_position_value(): + """#662 proofs 1 and 2, walked through the real CLI on an engine-priced + review. The percentage converts against this session's own frozen + position value (no second market resolution), the stored amount is + exactly p/(100-p) * position_value -- never p/100, which would be the + position value alone rather than the total account value -- and the + response discloses that derivation instead of applying it silently. + """ + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) / "coach" + env, plan = _prepared_on_a_priced_review(tmp, root) + calls_before = len(_provider_calls(env)) + + added = _run("add-cash", "--root", root, "--session-id", plan["session_id"], + "--cash", '{"currency":"USD","percent_of_total":20,"as_of":"2026-07-29"}', + env=env) + assert added.returncode == 0, added.stdout + added.stderr + out = json.loads(added.stdout) + assert out["recompute"]["outcome"] == "anchor_propagated", out["recompute"] + assert len(_provider_calls(env)) == calls_before, ( + "converting a percentage must not re-resolve prices -- it uses this session's own " + f"frozen position value: {_provider_calls(env)[calls_before:]}") + + assert "anchor_conversion" in out, ( + "a percentage was converted but the response discloses nothing about it", out) + conversion = out["anchor_conversion"] + assert conversion["percent_of_total"] == 20 + assert conversion["currency"] == "USD" + position_value = conversion["position_value"] + assert position_value > 0, "the fixture must really have a nonzero position value" + # The algebra (#662 owner ruling), recomputed independently here rather + # than trusted from the module under test: 20% of the ACCOUNT'S TOTAL + # (cash + positions) is 20/80 of the position value alone, not 20/100. + expected = round(20 / 80 * position_value, 2) + assert conversion["amount"] == expected, (conversion["amount"], expected, position_value) + assert conversion["amount"] != round(0.20 * position_value, 2), ( + "20% of the position value alone is the wrong denominator (#662)") + + amended = session_engine.load_pending(str(root), out["session_id"])["plan"] + cash = amended["engine_state"]["cash"] + assert cash["balance"] == conversion["amount"], cash + assert cash["source"] == "anchored" + # The denominator claim proven end to end: after storing the converted + # amount, cash really is 20% of cash + positions on the amended card. + assert abs(cash["weight"] - 0.20) < 1e-9, cash["weight"] + assert amended["engine_card"]["cash"]["balance"] == conversion["amount"] + + +def test_add_cash_percent_leaves_the_absolute_amount_response_unchanged(): + """Regression (#662 proof 3): an absolute-amount payload's response must + carry no new key at all -- not even a null one -- now that a percentage + format exists beside it, and the stored figure is untouched.""" + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) / "coach" + env, plan = _prepared_on_a_priced_review(tmp, root) + added = _run("add-cash", "--root", root, "--session-id", plan["session_id"], + "--cash", '{"currency":"USD","amount":8200,"as_of":"2026-07-29"}', env=env) + assert added.returncode == 0, added.stdout + added.stderr + out = json.loads(added.stdout) + assert "anchor_conversion" not in out, ( + "an absolute-amount response must not grow a new key", out) + amended = session_engine.load_pending(str(root), out["session_id"])["plan"] + assert amended["engine_state"]["cash_anchor_conversion"] is None + assert amended["engine_state"]["cash"]["balance"] == 8200.0 + + +def test_add_cash_percent_refuses_without_a_usable_position_value(): + """#662 proof 4: fails closed rather than converting against a garbage + denominator. A book with no currently held position has a position value + of zero, so a percentage cannot be resolved, and the refusal must leave no + pending session or ledger row behind -- exactly like every other add-cash + refusal in this file. The same book still takes a plain absolute amount: + the refusal is specific to the percentage format needing a denominator, + not to this book being otherwise unable to anchor cash at all. + """ + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) / "coach" + csv = pathlib.Path(tmp) / "fully_exited.csv" + csv.write_text( + "Symbol,Quantity,Price,Action,Description,TradeDate,SettledDate,Interest,Amount," + "Commission,Fee,CUSIP,RecordType\n" + "AAA,10,100.00,BUY,BOUGHT AAA,2024-01-02,2024-01-04,0,-1000.00,0,0,,Trade\n" + "AAA,10,110.00,SELL,SOLD AAA,2024-06-02,2024-06-04,0,1100.00,0,0,,Trade\n", + encoding="utf-8") + env = _offline_env(tmp) + run = _run("prepare", csv, "--root", root, "--language", "en", env=env) + assert run.returncode == 0, run.stdout + run.stderr + plan = json.loads(run.stdout)["review_plan"] + assert plan["input"]["cash_anchor"]["status"] == "absent", plan["input"]["cash_anchor"] + ledger_before = (pathlib.Path(root) / "ledger.jsonl").read_bytes() + + refused = _run("add-cash", "--root", root, "--session-id", plan["session_id"], + "--cash", '{"currency":"USD","percent_of_total":30,"as_of":"2024-06-02"}', + env=env) + assert refused.returncode != 0, refused.stdout + error = json.loads(refused.stdout)["error"] + assert "position market value" in error and "not usable" in error, error + assert sorted(os.listdir(pathlib.Path(root) / ".pending")) == [plan["session_id"]], \ + "a refused conversion must leave no anchored, finalizable session behind" + assert (pathlib.Path(root) / "ledger.jsonl").read_bytes() == ledger_before + + recovered = _run("add-cash", "--root", root, "--session-id", plan["session_id"], + "--cash", '{"currency":"USD","amount":500,"as_of":"2024-06-02"}', env=env) + assert recovered.returncode == 0, recovered.stdout + recovered.stderr + amended = session_engine.load_pending( + str(root), json.loads(recovered.stdout)["session_id"])["plan"] + assert amended["engine_state"]["cash"]["balance"] == 500.0 + assert amended["engine_state"]["cash"]["weight"] == 1.0 + + def test_add_cash_refuses_a_session_that_never_takes_an_anchor(): """A snapshot states cash inline in its own envelope, so there is no second place to supply one -- and the plan already says `not_applicable`. The diff --git a/tests/test_tr_json_contract.py b/tests/test_tr_json_contract.py index 7eb068e8..3390e6e4 100644 --- a/tests/test_tr_json_contract.py +++ b/tests/test_tr_json_contract.py @@ -54,6 +54,7 @@ "currency_meta", # #51/#129 PR-2a(optional 附加欄,單幣 USD 時內容多為 None) "portfolio_structure", # skill v2 ETF P0:同 card 的確定性結構判讀 "cash", # #171 PR-1:帳戶現金地基(balance/weight/source/reliable/recent_net_deposit;None=未提供現金錨點) + "cash_anchor_conversion", # #662:僅當這次 --cash 是 percent_of_total 才非 None——換算揭露(percent/position_value/currency/formula/amount),供 review.py cmd_add_cash 轉呈 agent "price_snapshot", "valuation_frame", "market_context", # #500 receipt is private state only "splits", # #550:這次真的套用的分割事件;帳本存名目股數,跨分割累加股數的讀者(revisit.detect_exits)要拿同一份,否則減碼被讀成清倉 "splits_window", # #605:上面那份表**從哪天起**是完整的。批次取得只帶窗口內的分割,而 refresh 與 catch-up 閘門按契約不重抓——一份蓋不住自己錨點的表會把 90 股讀成 900 股改動,故窗口跟著表凍,讓讀者查得出夠不夠