From f0e95372d327b93166a21cf7fb44f1cae8e12371 Mon Sep 17 00:00:00 2001 From: David Duncan Date: Sat, 29 Aug 2026 05:02:09 -0500 Subject: [PATCH 1/2] fix(reporting): resolve account kind from config; never guess LIVE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DEFAULT_ACCOUNT_KINDS was {} with the comment "Load account mappings from environment or config file instead of hardcoding". Stripping broker account ids out of a public repo was right; the replacement loader was never written. So ingest.py fell through to `kinds.get(account, AccountKind.LIVE)` and every account — including a simulated book — was reported as LIVE. Three tests in tests/test_reporting.py have been failing on main since. That default is the dangerous part. LIVE and SIM carry independent risk units, and a simulated book scored against live risk is the one error this layer must not make. A missing config entry should never be able to produce it. - default_account_kinds() resolves $TRADEKIT_ACCOUNT_KINDS first ("ID=LIVE,ID=SIM"), then [accounts] in accounts_config(). Resolved per call, not cached at import, so callers and tests that set the environment are honoured. Malformed pairs and unknown kinds are skipped rather than raising — a typo in one entry must not take down a review. - The LIVE fallback is gone. An unmapped account resolves to None and renders as UNMAPPED (). A visibly wrong label is recoverable; a plausible wrong one is not. AccountPnL.kind is now Optional to make "unknown" a real state rather than something the schema forces you to fake. - render_daily_card still emits a row for an unmapped account. Dropping it would hide real P&L behind a missing config entry. - paths.py gains xdg_config_home() / config_dir() / accounts_config(); paths.py owns every on-disk location, so the new one belongs there too. - config/accounts.toml.example documents the format and says plainly that real ids never get committed. DEFAULT_ACCOUNT_KINDS is kept as an inert, documented alias — it is exported from tradekit.reporting and removing it would break importers. Tests: the three TestIngest failures are fixed by supplying the mapping through an autouse env fixture, which also makes them hermetic — they no longer depend on whether the developer has ~/.config/tradekit/accounts.toml. Seven new tests in TestAccountKindResolution cover env parsing, malformed input, TOML loading, a missing config file, explicit per-record kind precedence, and the two behaviours that matter most: an unmapped account is never LIVE, and it still renders. 90 passed, up from 80 passed / 3 failed. Fixes gap G8 in docs/SPEC.md (#10). --- config/accounts.toml.example | 17 +++++++ src/tradekit/paths.py | 19 ++++++++ src/tradekit/reporting/__init__.py | 4 ++ src/tradekit/reporting/ingest.py | 72 +++++++++++++++++++++++++++--- src/tradekit/reporting/render.py | 13 +++++- src/tradekit/reporting/schema.py | 4 +- tests/test_reporting.py | 64 +++++++++++++++++++++++++- 7 files changed, 184 insertions(+), 9 deletions(-) create mode 100644 config/accounts.toml.example diff --git a/config/accounts.toml.example b/config/accounts.toml.example new file mode 100644 index 0000000..53e1b82 --- /dev/null +++ b/config/accounts.toml.example @@ -0,0 +1,17 @@ +# Account id → book kind. Copy to $XDG_CONFIG_HOME/tradekit/accounts.toml +# (usually ~/.config/tradekit/accounts.toml) and fill in your own ids. +# +# This file is NEVER committed with real ids: broker account identifiers are +# personal, and this repository is public. tradekit reads the mapping from your +# user config or from $TRADEKIT_ACCOUNT_KINDS, never from the source tree. +# +# An account absent from this mapping is reported as UNMAPPED, not LIVE. That is +# deliberate — a simulated book silently scored against live risk is the failure +# this design exists to prevent. +# +# Equivalent environment form, which takes precedence: +# export TRADEKIT_ACCOUNT_KINDS="YOURLIVEID=LIVE,YOURSIMID=SIM" + +[accounts] +# "YOURLIVEID" = "LIVE" +# "YOURSIMID" = "SIM" diff --git a/src/tradekit/paths.py b/src/tradekit/paths.py index a7ac60c..f9e7964 100644 --- a/src/tradekit/paths.py +++ b/src/tradekit/paths.py @@ -21,6 +21,7 @@ $XDG_DATA_HOME ~/.local/share durable app data -> data_dir() $XDG_CACHE_HOME ~/.cache regenerable, unbacked -> cache_dir() $XDG_STATE_HOME ~/.local/state logs, history -> state_dir() + $XDG_CONFIG_HOME ~/.config user configuration -> config_dir() Note ``~/.local/`` is *not* an XDG location; only ``share``, ``state``, ``bin``, and ``lib`` live directly under ``~/.local``. @@ -69,6 +70,24 @@ def xdg_state_home() -> Path: return _env_path("XDG_STATE_HOME") or Path.home() / ".local" / "state" +def xdg_config_home() -> Path: + return _env_path("XDG_CONFIG_HOME") or Path.home() / ".config" + + +def config_dir() -> Path: + """User configuration. Owned by tradekit; safe to relocate.""" + return xdg_config_home() / "tradekit" + + +def accounts_config() -> Path: + """Account id -> book kind mapping. + + Deliberately user-owned and outside the repository: broker account ids are + personal identifiers and this project is public. + """ + return config_dir() / "accounts.toml" + + def _resolve(env_var: str, preferred: Path, *legacy: Path) -> Path: """Resolve a path: explicit override, then whichever location exists. diff --git a/src/tradekit/reporting/__init__.py b/src/tradekit/reporting/__init__.py index 263c36e..781d974 100644 --- a/src/tradekit/reporting/__init__.py +++ b/src/tradekit/reporting/__init__.py @@ -42,8 +42,10 @@ ) from tradekit.reporting.ingest import ( DEFAULT_ACCOUNT_KINDS, + account_pnl_from_falcon, accounts_from_falcon, build_daily_card, + default_account_kinds, ingest_daily, trade_from_dict, ) @@ -94,7 +96,9 @@ # ingest "build_daily_card", "ingest_daily", + "account_pnl_from_falcon", "accounts_from_falcon", + "default_account_kinds", "trade_from_dict", "DEFAULT_ACCOUNT_KINDS", # runits diff --git a/src/tradekit/reporting/ingest.py b/src/tradekit/reporting/ingest.py index 2536295..84c75c3 100644 --- a/src/tradekit/reporting/ingest.py +++ b/src/tradekit/reporting/ingest.py @@ -19,6 +19,10 @@ from __future__ import annotations +import os +import tomllib + +from tradekit.paths import accounts_config from tradekit.reporting.grading import DisciplineScore, Grade, discipline_from_flags from tradekit.reporting.schema import ( AccountKind, @@ -30,9 +34,65 @@ TradeRecord, ) -# Known DAS account ids → book kind. Override per call when ids differ. +ACCOUNT_KINDS_ENV = "TRADEKIT_ACCOUNT_KINDS" + +#: Deprecated. Kept so existing imports keep working; it is no longer consulted. +#: It was an empty dict that silently made every account resolve to LIVE. +#: Use :func:`default_account_kinds`, or pass ``account_kinds=`` explicitly. DEFAULT_ACCOUNT_KINDS: dict[str, AccountKind] = {} -# Load account mappings from environment or config file instead of hardcoding + + +def _parse_account_kinds(raw: str) -> dict[str, AccountKind]: + """Parse ``ID=KIND,ID=KIND``. Unknown kinds and malformed pairs are skipped.""" + out: dict[str, AccountKind] = {} + for pair in raw.split(","): + account, sep, kind = pair.partition("=") + if not sep: + continue + account, kind = account.strip(), kind.strip().upper() + if not account: + continue + try: + out[account] = AccountKind(kind) + except ValueError: + continue + return out + + +def default_account_kinds() -> dict[str, AccountKind]: + """Account id → book kind, resolved from the environment or user config. + + Broker account ids are personal identifiers and this project is public, so + the mapping is never hardcoded here. Resolution order, first hit wins: + + 1. ``$TRADEKIT_ACCOUNT_KINDS`` — ``"1RB16917=LIVE,TR4425=SIM"`` + 2. ``accounts_config()`` — TOML, ``[accounts]`` table of ``id = "LIVE"|"SIM"`` + + Resolved on every call rather than cached at import, so a test or a caller + that sets the environment is honoured. + + Returns an empty mapping when neither source exists. Callers must treat an + unmapped account as *unknown* rather than defaulting it to LIVE — see + :func:`account_pnl_from_falcon`. + """ + raw = os.environ.get(ACCOUNT_KINDS_ENV) + if raw: + return _parse_account_kinds(raw) + + path = accounts_config() + try: + with path.open("rb") as fh: + table = tomllib.load(fh).get("accounts", {}) + except OSError, tomllib.TOMLDecodeError: + return {} + out: dict[str, AccountKind] = {} + for account, kind in table.items(): + try: + out[str(account)] = AccountKind(str(kind).upper()) + except ValueError: + continue + return out + # Tolerant field aliases for the falcon per-account stat object. First match wins. _FALCON_ALIASES: dict[str, tuple[str, ...]] = { @@ -72,14 +132,16 @@ def account_pnl_from_falcon(stat: dict, account_kinds: dict[str, AccountKind] | win/loss counts, the counts are reconstructed from ``win_rate * round_trips`` so the W/L column is still populated; explicit counts always win. """ - kinds = account_kinds or DEFAULT_ACCOUNT_KINDS + kinds = account_kinds if account_kinds is not None else default_account_kinds() account = str(_g(stat, "account", "")).strip() kind_raw = _g(stat, "kind") if kind_raw is not None: kind = AccountKind(str(kind_raw).upper()) else: - kind = kinds.get(account, AccountKind.LIVE) + # No silent LIVE default: an unmapped account guessed as LIVE is how a SIM + # book gets reported against live risk. Unknown stays unknown. + kind = kinds.get(account) round_trips = int(_g(stat, "round_trips", 0) or 0) wins = _g(stat, "wins") @@ -128,7 +190,7 @@ def accounts_from_falcon( def trade_from_dict(d: dict, account_kinds: dict[str, AccountKind] | None = None) -> TradeRecord: """Build a graded :class:`TradeRecord` from the narrative's trade object.""" - kinds = account_kinds or DEFAULT_ACCOUNT_KINDS + kinds = account_kinds if account_kinds is not None else default_account_kinds() account = str(d.get("account", "")).strip() account_kind = d.get("account_kind") or d.get("kind") kind = AccountKind(str(account_kind).upper()) if account_kind else kinds.get(account) diff --git a/src/tradekit/reporting/render.py b/src/tradekit/reporting/render.py index 28ad23b..6a9d4ea 100644 --- a/src/tradekit/reporting/render.py +++ b/src/tradekit/reporting/render.py @@ -40,8 +40,12 @@ def _account_row(a: AccountPnL, config: RiskConfig | None) -> str: peak = f"{_money(a.peak_equity)} @ {a.peak_time}" if a.peak_equity is not None else "—" trough = f"{_money(a.trough_equity)} @ {a.trough_time}" if a.trough_equity is not None else "—" maxdd = f"{_money(a.max_drawdown)} @ {a.max_dd_time}" if a.max_drawdown is not None else "—" + # An unmapped account is labelled UNMAPPED, never LIVE. Reading a SIM book as + # live risk is the failure this guards against; a visibly wrong label is fine, + # a plausible wrong label is not. + label = f"{a.kind.value} ({a.account})" if a.kind else f"UNMAPPED ({a.account})" return ( - f"| **{a.kind.value} ({a.account})** | {a.round_trips} | {wr_str} " + f"| **{label}** | {a.round_trips} | {wr_str} " f"| {_money(a.realized)} | {peak} | {trough} | {maxdd} | {a.streak or '—'} |" ) @@ -58,9 +62,16 @@ def render_daily_card(card: DailyReportCard, config: RiskConfig | None = None) - lines.append("") lines.append("| Account | Round-Trips | Win Rate | Realized | Peak Equity | Trough | Max DD | Streak |") lines.append("|---------|------------:|----------|---------:|-------------|--------|--------|--------|") + rendered: list[int] = [] for kind in (AccountKind.LIVE, AccountKind.SIM): a = card.account(kind) if a: + rendered.append(id(a)) + lines.append(_account_row(a, config)) + # Accounts with no configured kind still get a row. Dropping them would hide + # real P&L behind a missing config entry. + for a in card.accounts: + if id(a) not in rendered: lines.append(_account_row(a, config)) decided_w = sum(a.wins for a in card.accounts) decided_l = sum(a.losses for a in card.accounts) diff --git a/src/tradekit/reporting/schema.py b/src/tradekit/reporting/schema.py index f6c4e94..5eb2a89 100644 --- a/src/tradekit/reporting/schema.py +++ b/src/tradekit/reporting/schema.py @@ -142,7 +142,9 @@ class AccountPnL(BaseModel): """ account: str - kind: AccountKind + # None = the account id is not in the configured mapping. Never guessed: + # a SIM book silently reported as LIVE is a risk-model failure. + kind: AccountKind | None = None round_trips: int = 0 wins: int = 0 losses: int = 0 diff --git a/tests/test_reporting.py b/tests/test_reporting.py index 4e8d88e..5c87b29 100644 --- a/tests/test_reporting.py +++ b/tests/test_reporting.py @@ -1,6 +1,7 @@ """Tests for the canonical reporting layer.""" import json +from pathlib import Path import pytest @@ -17,9 +18,11 @@ RiskConfig, TradePlan, TradeRecord, + account_pnl_from_falcon, accounts_from_falcon, average_grade, build_daily_card, + default_account_kinds, discipline_from_flags, fmt_r, grade_from_score, @@ -225,6 +228,17 @@ def test_render_weekly_reuses_trend_table(self): class TestIngest: + @pytest.fixture(autouse=True) + def _account_kinds(self, monkeypatch): + """Supply the account mapping the way a real deployment does. + + The ids live in the environment, never in the source tree — they are + personal identifiers and this repository is public. Setting them here + also keeps the tests hermetic: they no longer depend on whether the + developer happens to have ~/.config/tradekit/accounts.toml. + """ + monkeypatch.setenv("TRADEKIT_ACCOUNT_KINDS", "1RB16917=LIVE,TR4425=SIM") + def _falcon(self): # Mimics falcon-stats output (alias-tolerant keys on purpose). return [ @@ -283,7 +297,7 @@ def test_accounts_from_falcon_verbatim_and_kinds(self): live = next(a for a in accts if a.account == "1RB16917") sim = next(a for a in accts if a.account == "TR4425") assert live.kind is AccountKind.LIVE - assert sim.kind is AccountKind.SIM # inferred from known id + assert sim.kind is AccountKind.SIM # from the configured mapping # Deterministic numbers carried verbatim, incl. equity shape. assert live.realized == pytest.approx(240.50) assert live.max_drawdown == pytest.approx(-60.0) @@ -308,7 +322,7 @@ def test_build_daily_card_merges_deterministic_and_narrative(self): assert card.market_regime == "Trending" t = card.trades[0] assert t.ticker == "NVDA" and t.grade is Grade.A and t.direction is Direction.LONG - assert t.account_kind is AccountKind.LIVE # inferred from account id + assert t.account_kind is AccountKind.LIVE # from the configured mapping # discipline flags → reproducible total (2 + 2 + 1) assert card.discipline.total == 5 assert card.monitored_not_traded == ["AMD", "ARM"] @@ -404,3 +418,49 @@ def _sample_card(date: str = "2026-06-11") -> DailyReportCard: lessons=["Let the thesis trade breathe to first target"], behavioral_contract="Hold the thesis trade to plan; no premature scaling.", ) + + +class TestAccountKindResolution: + """An unmapped account must never be guessed as LIVE.""" + + def _stat(self, account: str) -> dict: + return {"account": account, "round_trips": 1, "wins": 1, "losses": 0, "realized": 10.0} + + def test_env_mapping_is_parsed(self, monkeypatch): + monkeypatch.setenv("TRADEKIT_ACCOUNT_KINDS", "AAA=LIVE, BBB=sim") + kinds = default_account_kinds() + assert kinds == {"AAA": AccountKind.LIVE, "BBB": AccountKind.SIM} + + def test_malformed_pairs_are_skipped_not_fatal(self, monkeypatch): + monkeypatch.setenv("TRADEKIT_ACCOUNT_KINDS", "AAA=LIVE,,garbage,CCC=NOPE,=SIM,BBB=SIM") + assert default_account_kinds() == {"AAA": AccountKind.LIVE, "BBB": AccountKind.SIM} + + def test_unmapped_account_is_unknown_never_live(self, monkeypatch): + monkeypatch.setenv("TRADEKIT_ACCOUNT_KINDS", "AAA=LIVE") + acct = account_pnl_from_falcon(self._stat("ZZZ")) + assert acct.kind is None, "an unmapped account must not be guessed as LIVE" + + def test_explicit_kind_on_the_record_wins(self, monkeypatch): + monkeypatch.delenv("TRADEKIT_ACCOUNT_KINDS", raising=False) + stat = self._stat("ZZZ") | {"kind": "sim"} + assert account_pnl_from_falcon(stat).kind is AccountKind.SIM + + def test_toml_config_used_when_env_absent(self, monkeypatch, tmp_path): + monkeypatch.delenv("TRADEKIT_ACCOUNT_KINDS", raising=False) + cfg = tmp_path / "accounts.toml" + cfg.write_text('[accounts]\n"AAA" = "LIVE"\n"BBB" = "SIM"\n') + monkeypatch.setattr("tradekit.reporting.ingest.accounts_config", lambda: cfg) + assert default_account_kinds() == {"AAA": AccountKind.LIVE, "BBB": AccountKind.SIM} + + def test_missing_config_is_empty_not_an_error(self, monkeypatch, tmp_path): + monkeypatch.delenv("TRADEKIT_ACCOUNT_KINDS", raising=False) + monkeypatch.setattr("tradekit.reporting.ingest.accounts_config", lambda: tmp_path / "nope.toml") + assert default_account_kinds() == {} + + def test_unmapped_account_renders_as_unmapped(self, monkeypatch): + monkeypatch.delenv("TRADEKIT_ACCOUNT_KINDS", raising=False) + monkeypatch.setattr("tradekit.reporting.ingest.accounts_config", lambda: Path("/nonexistent.toml")) + card = build_daily_card("2026-06-11", [self._stat("ZZZ")], {}) + out = render_daily_card(card) + assert "UNMAPPED (ZZZ)" in out + assert "LIVE (ZZZ)" not in out From b4e868c8ab3aa8b15beb33afedd004bb4d698881 Mon Sep 17 00:00:00 2001 From: David Duncan <297012+davdunc@users.noreply.github.com> Date: Sat, 29 Aug 2026 07:29:09 -0500 Subject: [PATCH 2/2] Update src/tradekit/reporting/ingest.py Co-authored-by: amazon-q-developer[bot] <208079219+amazon-q-developer[bot]@users.noreply.github.com> --- src/tradekit/reporting/ingest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tradekit/reporting/ingest.py b/src/tradekit/reporting/ingest.py index 84c75c3..c9d5849 100644 --- a/src/tradekit/reporting/ingest.py +++ b/src/tradekit/reporting/ingest.py @@ -83,7 +83,7 @@ def default_account_kinds() -> dict[str, AccountKind]: try: with path.open("rb") as fh: table = tomllib.load(fh).get("accounts", {}) - except OSError, tomllib.TOMLDecodeError: + except (OSError, tomllib.TOMLDecodeError): return {} out: dict[str, AccountKind] = {} for account, kind in table.items():