diff --git a/keel/cli.py b/keel/cli.py index 4801a1e..2f2ddc4 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -158,6 +158,7 @@ from keel.commands.fetch import run_fetch from keel.commands.insights import _parse_ts as _parse_since_until from keel.commands.insights import insights_group +from keel.commands.journal import journal_group from keel.commands.mcp import mcp_cmd from keel.commands.monitor import run_monitor from keel.commands.orders import orders_cmd @@ -1500,6 +1501,18 @@ def simulate( cli.add_command(insights_group) +# -- journal (the discretionary journal: human-sourced, CLI-only, append-only) -------------------- + +# #705. The `journal` table was declared in the schema from the beginning and had no repository +# method and no caller -- dead schema. This is its only write path, and it is deliberately the +# ONLY one: attestations are human-sourced or refused, `keel serve` has no route to it, and +# `journal add` takes no value options so the entry cannot be scripted past the TTY gate. +# +# NOT `keel insights journal`, registered above, which is a filterable view of closed TRADES. +# Two similar names over two different kinds of evidence, and both say which they are. +cli.add_command(journal_group) + + # -- versions (the deploy check: every keel distribution, not just this one) --------------------- # `--version` above answers for `keel-trader` alone and therefore cannot see a partial upgrade; diff --git a/keel/commands/journal.py b/keel/commands/journal.py new file mode 100644 index 0000000..cd7aa40 --- /dev/null +++ b/keel/commands/journal.py @@ -0,0 +1,366 @@ +"""The discretionary journal: what the operator says about their own conduct (#705). + +The `journal` table has been in the schema from the beginning with no repository method and no +caller -- dead schema, which is worse than no schema because a reader assumes a declared table is +a used one. This is its wiring: a CLI that writes it, a report both front-ends read, and a place +in the audit chain beside everything else a human swore to. + +── WHY THIS RECORD IS DIFFERENT FROM EVERY OTHER RECORD HERE ──────────────────────────────────── + +Everything else keel keeps is either a machine's observation or a claim about the world. An order +is what a venue reported. A transaction is a line out of a venue's own export. An asset +attestation says PAXG is backed by allocated gold -- a claim a prospectus could contradict. + +A journal entry has no external referent at all. "I felt rushed", "I broke my own rule", "it cost +me forty dollars" cannot be checked against anything, ever. That is not a defect: self-assessment +is the only way this information exists, and no competitor keeps it because no venue can produce +it. But it means the record's value depends entirely on it staying visibly separate from the ones +that can be checked -- hence `SELF_REPORTED`, which is a sixth provenance in the timeline's closed +vocabulary rather than a reuse of `human-attested`. + +── THE CLI IS THE ONLY WAY IN ─────────────────────────────────────────────────────────────────── + +Attestations are human-sourced or refused. `keel journal add` prompts, requires a terminal, and +accepts NO value options -- not merely "it prompts by default". A `--emotion 3` would make the +whole entry scriptable, and the TTY gate would then be guarding a ceremony that no longer needed a +human to supply anything. There is no web write path, and a test asserts it over +`keel.commands.setup.ACTIONS` -- the only surface `server.do_POST` will route to, and one that +already carries an attestation writer (`attest_asset`), which is precisely why a journal box is +the plausible next addition. + +**WHAT THE TERMINAL CHECK IS AND IS NOT.** `sys.stdin.isatty()` refuses a pipe, a redirect and a +cron job as ordinarily written. It does NOT refuse a determined script: a `pty.fork()` driver +allocates a real terminal and feeds the prompts, and this command answers it. That is true of +every gate in this codebase built on the same predicate, and it is the honest boundary -- the +check makes automated entry a thing someone has to MEAN, not a thing they can do by accident. A +record whose whole value is that a person wrote it cannot be enforced by software beyond that. + +── AND THERE IS NO EDIT ───────────────────────────────────────────────────────────────────────── + +Append-only, with no update method anywhere. A journal you can go back and change is a journal +that records what you wish you had thought. +""" + +from __future__ import annotations + +import time +from dataclasses import dataclass +from decimal import Decimal, InvalidOperation + +import click + +from keel.data.repository import Repository + +#: The word that must appear beside every entry, on every surface. Not "journal" and not +#: "attested": the reader has to know that nothing outside this operator's head produced it. +SELF_REPORTED = "SELF-REPORTED" + +#: The emotion scale, stated in the prompt and enforced on the way in. +#: +#: A "score" with no scale cannot be compared with itself next week, which is the only thing an +#: emotion column is for. 1-5 rather than free text, and a value off the scale is REFUSED rather +#: than quietly kept -- a journal holding "9", "very bad" and "3" in one column has three +#: vocabularies and no series. +EMOTION_MIN = 1 +EMOTION_MAX = 5 + +#: What `keel journal list` says over an empty table. Distinct from a bare header, which reads as +#: a table that failed to load rather than a deployment nobody has written in yet. +EMPTY_NOTE = "No journal entries yet. `keel journal add` writes one, and only a person can." + +#: What the TTY gate announces. Named here so the command and its test cannot disagree about +#: what the operator is being asked to confirm. +JOURNAL_ADD_ACTION = "write a journal entry" + + +@dataclass(frozen=True) +class JournalEntry: + """One entry, as both front-ends read it. + + Every field but `id`/`ts` is optional, and `None` means DID NOT SAY -- never a zero, an empty + string, or a `False`. `rules_followed` in particular is three-valued: `False` is the operator + confessing they broke their own rules, and nobody should make that confession by leaving a + prompt blank. + """ + + id: int + ts: int + emotion_score: str | None + rules_followed: bool | None + errors_made: str | None + dollar_impact: Decimal | None + chart_note: str | None + screenshot_ref: str | None + + +#: How many entries the console shows without being asked for more. +#: +#: The journal has its OWN cap and is deliberately not paged by `/api/journal`'s `?limit=`. That +#: parameter is the closed-trade table's page control; applying it here was a coincidence of the +#: two records sharing a route, and it meant narrowing to one trade silently hid 300 of an +#: operator's 301 notes. +DEFAULT_NOTES_LIMIT = 50 + + +@dataclass(frozen=True) +class JournalReport: + now_ts: int + entries: tuple[JournalEntry, ...] + #: How many entries the window holds BEFORE `limit` truncated `entries`. + #: + #: Carried, not derived, because a caller that bounds a read is showing a WINDOW of the record + #: and must say so -- the rule `Repository.get_equity_points` states and `count_equity_points` + #: exists to serve. The first cut shipped `shown_count` alone, so a capped journal was + #: indistinguishable on the page from a complete one. + total_count: int = 0 + + @property + def entry_count(self) -> int: + """Derived rather than stored, and held here because `keel/web/payload.py` may not call + `len()` (Rule 6e).""" + return len(self.entries) + + @property + def any_recorded(self) -> bool: + """Whether this deployment has a journal at all. + + Reads `total_count`, not `entries`: a window that returned nothing because a cap or a date + bound excluded everything is not a deployment with no journal, and the renderers say + different things about the two. + """ + return self.total_count > 0 + + @property + def truncated(self) -> bool: + """Whether this report is a PAGE of a longer journal. What the page says "50 of 301" + from, and the flag a renderer needs to say anything at all rather than showing a short + list that looks complete.""" + return self.total_count > self.entry_count + + +def gather_journal(repo: Repository, *, now_ts: int, limit: int | None = None) -> JournalReport: + """Every entry, oldest first -- a journal reads forwards. + + `limit` keeps the NEWEST entries and still returns them forwards, so a cap changes how much of + the journal a reader sees and never which way it reads. `total_count` comes off a separate + COUNT over the same window, so the report always knows what the cap left out. + + A NEGATIVE OR ZERO LIMIT IS REFUSED rather than obeyed. SQLite reads a negative `LIMIT` as + unbounded, so `--limit -1` would silently print everything; a zero returns no rows, and an + empty result is indistinguishable from an empty journal on both front-ends -- which is exactly + the hazard `keel/web/api.py::_journal_limit` was written to name. Refusing is the only reading + that cannot lie. + """ + if limit is not None and limit < 1: + raise ValueError(f"limit must be 1 or more (or omitted for all); got {limit}") + entries = tuple( + JournalEntry( + id=int(row["id"]), + ts=int(row["ts"]), + emotion_score=_optional_text(row.get("emotion_score")), + rules_followed=row.get("rules_followed"), + errors_made=_optional_text(row.get("errors_made")), + dollar_impact=row.get("dollar_impact"), + chart_note=_optional_text(row.get("chart_note")), + screenshot_ref=_optional_text(row.get("screenshot_ref")), + ) + for row in repo.get_journal_entries(limit=limit) + ) + return JournalReport( + now_ts=now_ts, entries=entries, total_count=repo.count_journal_entries() + ) + + +def _optional_text(value: object) -> str | None: + """`None` for absent AND for empty, because a column holding `""` says nothing a `NULL` does + not, and two spellings of "did not say" would render as two different states.""" + if value is None: + return None + text = str(value) + return text or None + + +# -- the prompts ------------------------------------------------------------------------------- +# +# Each returns `None` for a blank answer and RAISES for an answer it cannot honour. Refusing is +# the right response to "9" on a 1-5 scale or to "lots" as a dollar figure: storing either would +# put a value in the record that the operator did not mean and cannot be compared with the rest. + + +def parse_emotion(raw: str) -> str | None: + """A 1-5 score, as the digit it will be stored as, or `None` for a blank answer.""" + text = raw.strip() + if not text: + return None + try: + score = int(text) + except ValueError: + raise click.ClickException( + f"emotion score must be a whole number from {EMOTION_MIN} to {EMOTION_MAX} " + f"(or blank to skip); got {text!r}" + ) from None + if not EMOTION_MIN <= score <= EMOTION_MAX: + raise click.ClickException( + f"emotion score must be from {EMOTION_MIN} to {EMOTION_MAX} (or blank to skip); " + f"got {score}" + ) + return str(score) + + +def parse_rules_followed(raw: str) -> bool | None: + """`y`/`n`, or `None` for a blank answer. + + Blank is NOT `False`. "I broke my rules" is the single most consequential sentence in this + table, and an operator who skipped the question has not said it. + """ + text = raw.strip().lower() + if not text: + return None + if text in ("y", "yes"): + return True + if text in ("n", "no"): + return False + raise click.ClickException(f"answer y or n (or blank to skip); got {raw.strip()!r}") + + +def parse_impact(raw: str) -> Decimal | None: + """A signed dollar figure as `Decimal`, or `None` for a blank answer. + + `Decimal`, like every other money value here, and REFUSED rather than coerced: a journal whose + dollar column holds "lots" cannot be summed, and one that silently read it as zero would say + the day cost nothing. + """ + text = raw.strip() + if not text: + return None + try: + return Decimal(text) + except InvalidOperation: + raise click.ClickException( + f"dollar impact must be a number like -42.50 (or blank to skip); got {text!r}" + ) from None + + +def render_human(report: JournalReport) -> str: + """The terminal rendering. Chronological, with the provenance marker on the header. + + The marker is not decoration. `keel insights journal` already exists and is a filterable view + of closed TRADES -- venue facts, a different thing wearing a similar name -- and an operator + reading one after the other must not have to remember which is which. + """ + lines = [f"journal ({SELF_REPORTED} — the operator's own account, nothing verified it)", ""] + if not report.any_recorded: + lines.append(EMPTY_NOTE) + return "\n".join(lines) + + for entry in report.entries: + stamp = time.strftime("%Y-%m-%d %H:%M", time.gmtime(entry.ts)) + lines.append(f"[{entry.id}] {stamp} UTC") + lines.append(f" emotion : {entry.emotion_score or 'not said'}") + lines.append(f" rules followed: {_rules_word(entry.rules_followed)}") + lines.append(f" errors made : {entry.errors_made or 'not said'}") + lines.append(f" dollar impact : {_impact_word(entry.dollar_impact)}") + lines.append(f" chart note : {entry.chart_note or 'not said'}") + lines.append(f" screenshot : {entry.screenshot_ref or 'not said'}") + lines.append("") + if report.truncated: + lines.append(f"{report.entry_count} of {report.total_count} entries (newest).") + else: + lines.append(f"{report.entry_count} entr{'y' if report.entry_count == 1 else 'ies'}.") + return "\n".join(lines) + + +def _rules_word(value: bool | None) -> str: + """Three words for three values. "no" is a confession and must never be what "not said" + prints as.""" + if value is None: + return "not said" + return "yes" if value else "NO" + + +def _impact_word(value: Decimal | None) -> str: + return "not said" if value is None else format(value, "f") + + +# -- the CLI ----------------------------------------------------------------------------------- + + +@click.group("journal") +def journal_group() -> None: + """Your own account of your own trading -- what you felt, whether you followed your rules. + + NOT `keel insights journal`, which is a filterable view of closed TRADES: venue facts, a + different thing wearing a similar name. Nothing here was verified by anything. + """ + + +@journal_group.command("add") +@click.pass_context +def journal_add(ctx: click.Context) -> None: + """Write one entry. Prompts for every field; blank skips it. + + Needs a terminal, and takes no value options -- see the module docstring. Every question may + be skipped, including all of them: an operator recording one sentence about one day should not + have to invent an emotion score to do it. + """ + from keel.commands._common import _is_interactive, _open_repo + + # `_is_interactive`, NOT `_require_interactive_confirmation`. The heavier gate demands a typed + # `yes` and exists for DANGEROUS actions -- releasing a kill-switch, spending money -- and its + # own docstring warns against ceremony without a matching threat model. Writing a sentence + # about your own trading is not dangerous; it is unverifiable, which is a different problem + # and one a confirmation prompt does nothing about. + # + # What IS load-bearing is the terminal. The constitution's rule is that an attestation is + # human-sourced or refused, and off a TTY there is no human -- so cron, a pipe and a script + # are all refused here, using the same predicate the heavier gate is built on. That predicate + # has no env-var or flag override, deliberately, so nothing can reach past it. + if not _is_interactive(): + raise click.ClickException( + f"refusing to {JOURNAL_ADD_ACTION}: this needs an interactive terminal. " + f"A journal entry is {SELF_REPORTED} by definition -- there is no other source it " + "could come from, so there is no way to supply one from a script." + ) + click.echo(f"{SELF_REPORTED} — nothing verifies this, and it cannot be edited afterwards.") + click.echo("Every question may be skipped; a blank answer records that you did not say.") + + emotion = parse_emotion( + click.prompt( + f"emotion ({EMOTION_MIN}-{EMOTION_MAX})", default="", show_default=False + ) + ) + rules = parse_rules_followed( + click.prompt("did you follow your rules? (y/n)", default="", show_default=False) + ) + errors = click.prompt("errors made", default="", show_default=False).strip() + impact = parse_impact(click.prompt("dollar impact", default="", show_default=False)) + note = click.prompt("chart note", default="", show_default=False).strip() + shot = click.prompt("screenshot reference", default="", show_default=False).strip() + + repo = _open_repo(ctx) + entry_id = repo.append_journal_entry( + ts=int(time.time()), + emotion_score=emotion, + rules_followed=rules, + errors_made=errors or None, + dollar_impact=impact, + chart_note=note or None, + screenshot_ref=shot or None, + ) + click.echo(f"recorded journal entry {entry_id} ({SELF_REPORTED}).") + + +@journal_group.command("list") +@click.option( + "--limit", + type=click.IntRange(min=1), + default=None, + help="Show only the most recent N entries. They still read forwards.", +) +@click.pass_context +def journal_list(ctx: click.Context, limit: int | None) -> None: + """Read the journal back, oldest first.""" + from keel.commands._common import _open_repo + + report = gather_journal(_open_repo(ctx), now_ts=int(time.time()), limit=limit) + click.echo(render_human(report)) diff --git a/keel/commands/timeline.py b/keel/commands/timeline.py index bccdd3d..4234adb 100644 --- a/keel/commands/timeline.py +++ b/keel/commands/timeline.py @@ -1,8 +1,9 @@ """One chronology over everything keel has done -- issue #703. -Four stores record activity and none of them knew about the others: the engine's JSONL log -(cycles), the `orders` table (fills), the `transactions` ledger (cash flows), and the attestation -tables (what a human swore to). This module merges them into one timeline WITHOUT letting them +Five stores record activity and none of them knew about the others: the engine's JSONL log +(cycles), the `orders` table (fills), the `transactions` ledger (cash flows), the attestation +tables (what a human swore to about the world), and the discretionary `journal` (#705 -- what the +operator says about THEMSELVES). This module merges them into one timeline WITHOUT letting them blur, which is the whole difficulty: a venue-reported fill, a line imported from a venue's CSV, and a sentence a human typed are three different kinds of evidence, and a feed that presented them identically would be worse than four separate tables. @@ -58,11 +59,18 @@ #: - `imported-ledger` -- a `transactions` row, read out of a venue's own CSV export. #: - `human-attested` -- someone typed it and signed their name to it. #: - `engine-log` -- the agent's own structured log of what it did. +#: - `self-reported` -- the operator's account of their OWN conduct (#705). A sixth word rather +#: than a reuse of `human-attested`, because the two are different kinds of claim: an asset +#: attestation says PAXG is backed by allocated gold, which a prospectus could contradict, while +#: "I felt rushed and broke my rule" has no external referent and cannot be checked by anyone, +#: ever. Filing a self-assessment under the word this feed uses for checkable human claims would +#: put the one unverifiable record in the database under a heading that implies otherwise. PROVENANCES: tuple[str, ...] = ( "venue-reported", "simulated", "imported-ledger", "human-attested", + "self-reported", "engine-log", ) @@ -405,6 +413,69 @@ def _attestation_rows( return rows +def _journal_rows( + repo: Repository, since_ts: int | None, chain: _Chain +) -> list[TimelineRow]: + """`journal` -> attestation rows (#705). + + Under the ATTESTATION chip, because that is the kind of thing this is -- something a person + put their name to -- and with `self-reported` provenance, because it is the one kind of + attestation nothing outside the operator's head produced. The chip groups it with the asset + and instrument attestations; the provenance column is what keeps it from being read as one. + + The summary carries EVERY sentence the operator wrote, in a fixed order, because this row is + the journal's whole representation in the CSV -- there is no other column any of it could + reappear in. An entry whose only content is an emotion score still says something, and a row + reading only "journal entry" would make the feed's densest human content its least legible. + """ + rows: list[TimelineRow] = [] + for raw in repo.get_journal_entries(since_ts=since_ts): + entry_id = str(raw.get("id") or "") + rows.append( + TimelineRow( + ts=int(raw["ts"]), + kind="attestation", + provenance="self-reported", + source="journal", + reference=entry_id, + summary=_journal_summary(raw), + product_id="", + # The figure is what the operator SAYS the day cost them, and `amount_kind` names + # it as such: a self-reported impact and a venue-reported fee in one column, with + # nothing saying which is which, is a column that will be summed. + amount=raw.get("dollar_impact"), + amount_kind="self-reported impact" if raw.get("dollar_impact") is not None else "", + **chain.of("journal", entry_id), + ) + ) + return rows + + +def _journal_summary(raw: dict[str, Any]) -> str: + """One line from an entry, leading with whatever the operator wrote. + + `rules_followed` is THREE-valued and only one of the three is worth a chip: `False` is the + operator saying they broke their own rules, which is the single most consequential thing this + table can hold, and `None` is a question they skipped. `bool(None)` would print the confession + over the silence. + """ + parts: list[str] = [] + if raw.get("rules_followed") is False: + parts.append("BROKE RULES") + # EVERY sentence the operator wrote, not the first one found. The first cut used `elif`, so an + # entry carrying both an error and a chart note exported only the error -- and this row is the + # journal's whole representation in a file an operator hands to an auditor. There is no other + # column it could reappear in. + for field in ("errors_made", "chart_note", "screenshot_ref"): + written = str(raw.get(field) or "").strip() + if written: + parts.append(written) + emotion = str(raw.get("emotion_score") or "").strip() + if emotion: + parts.append(f"emotion {emotion}") + return " — ".join(parts) if parts else "journal entry (nothing written)" + + def _cycle_rows(cycles: Iterable[Any], since_ts: int | None) -> list[TimelineRow]: """`ActivityCycle`s -> system rows. @@ -490,6 +561,7 @@ def gather_timeline( scoped.extend(_order_rows(repo, since, chained)) scoped.extend(_transaction_rows(repo, since, chained)) scoped.extend(_attestation_rows(repo, since, chained)) + scoped.extend(_journal_rows(repo, since, chained)) # No chain argument, and never one: `_cycle_rows` reads the engine's own log FILE, which is # not a chained store. A cycle row carrying a hash would be this module attesting to something # it merely read. diff --git a/keel/data/audit.py b/keel/data/audit.py index 5b461eb..cc1a274 100644 --- a/keel/data/audit.py +++ b/keel/data/audit.py @@ -70,6 +70,12 @@ "transaction_recorded": "transactions", "asset_attested": "asset_attestations", "instrument_attested": "instrument_attestations", + # #705. The journal is chained for the same reason the two attestation tables are -- it is + # something a human swore to, and it rides the same audit export. It is also the ONLY store + # here whose subject is the operator rather than the world, which is a difference the + # timeline's `provenance` column carries, not this one: the chain's job is that a row cannot + # be altered quietly, and that is the same job whatever the row claims. + "journal_recorded": "journal", } diff --git a/keel/data/repository.py b/keel/data/repository.py index 7100750..c01e745 100644 --- a/keel/data/repository.py +++ b/keel/data/repository.py @@ -226,6 +226,159 @@ class Repository: def __init__(self, conn: sqlite3.Connection) -> None: self._conn = conn + # -- the discretionary journal (#705) --------------------------------- + # + # The operator's own account of their own conduct: what they felt, whether they followed + # their rules, what it cost. Declared in the schema since the beginning with no method and no + # caller -- dead schema, which is worse than none, because a reader assumes a declared table + # is a used one. + # + # UNLIKE EVERY OTHER STORE HERE, none of this can be checked. An order is what a venue + # reported, a transaction is a line from a venue's own export, an asset attestation is a claim + # a prospectus could contradict. A self-assessment has no external referent at all, and the + # whole value of keeping it depends on it staying visibly separate from the things that do -- + # which is why `commands/timeline.py` gives it its own provenance word rather than filing it + # under `human-attested` beside the attestations. + # + # APPEND-ONLY, and there is no update method by design. A journal you can go back and edit is + # a journal that records what you wish you had thought. + + def append_journal_entry( + self, + *, + ts: int, + emotion_score: str | None = None, + rules_followed: bool | None = None, + errors_made: str | None = None, + dollar_impact: Decimal | None = None, + chart_note: str | None = None, + screenshot_ref: str | None = None, + ) -> int: + """Append one entry and return its `id`. + + Every field but `ts` defaults to `None`, and `None` means DID NOT SAY -- never a zero, an + empty string or a `False`. An operator who wants to record one sentence about one day must + not have to invent an emotion score to do it, and `rules_followed=False` is a positive + confession that nobody should be able to make by omission. + + The entry and its audit-chain row land in one transaction (#721), the same discipline + every other writer here follows. + """ + with write_transaction(self._conn): + cursor = self._conn.execute( + """ + INSERT INTO journal + (ts, emotion_score, rules_followed, errors_made, dollar_impact, chart_note, + screenshot_ref) + VALUES (?, ?, ?, ?, ?, ?, ?) + """, + ( + ts, + emotion_score, + None if rules_followed is None else int(rules_followed), + errors_made, + _dec_to_text(dollar_impact), + chart_note, + screenshot_ref, + ), + ) + assert cursor.lastrowid is not None + entry_id = cursor.lastrowid + # The row id, because `journal` has no natural key -- no `coinbase_id`, no asset, no + # venue pair -- and it is what `commands/timeline.py` prints as the row's reference. + append_event( + self._conn, + ts=ts, + event_type="journal_recorded", + entity_id=str(entry_id), + payload={ + "id": entry_id, + "ts": ts, + "emotion_score": emotion_score, + "rules_followed": rules_followed, + "errors_made": errors_made, + "dollar_impact": dollar_impact, + "chart_note": chart_note, + "screenshot_ref": screenshot_ref, + }, + ) + return entry_id + + def get_journal_entries( + self, + *, + since_ts: int | None = None, + until_ts: int | None = None, + limit: int | None = None, + ) -> list[dict[str, Any]]: + """Entries in the window, OLDEST FIRST -- a journal reads forwards. + + The window is half-open (`since_ts <= ts < until_ts`), matching `get_candles` and + `commands/orders.py::scope_start_ts`, so two adjacent windows cover a range without + double-counting the seam. + + `limit` keeps the NEWEST entries and still returns them oldest-first: a capped read of a + journal wants the recent end, and the cap must change how many entries a caller sees + rather than which way they read. `id` breaks a timestamp tie, so two notes written in one + second keep a stable order across reads. + """ + where, params = self._journal_where(since_ts, until_ts) + if limit is None: + query = f"SELECT * FROM journal{where} ORDER BY ts, id" + else: + # The SUBQUERY, not a DESC read reversed in Python -- the identical shape + # `get_equity_points` and `get_cycle_balances` use, and `get_equity_points`' docstring + # is where the reasoning lives: the ordering is the caller's contract rather than an + # artefact of how the rows were selected, so a bounded read and an unbounded one + # differ only in how much they return. `id` breaks the tie in BOTH directions, so the + # newest-N and the oldest-first re-order agree about which of two same-second entries + # is the newer. + query = ( + f"SELECT * FROM (SELECT * FROM journal{where} " + "ORDER BY ts DESC, id DESC LIMIT ?) ORDER BY ts, id" + ) + params = [*params, limit] + rows = self._conn.execute(query, params).fetchall() + return [self._journal_row_to_dict(row) for row in rows] + + def count_journal_entries( + self, *, since_ts: int | None = None, until_ts: int | None = None + ) -> int: + """How many entries the window holds, BEFORE any `limit` truncated it. + + The sibling `count_equity_points` exists for the same reason and its docstring states the + rule: a caller that bounds a read is showing a WINDOW of the record and must say so. The + console showed a capped journal with nothing on the page distinguishing it from a complete + one, which is the failure that rule exists to prevent. + """ + where, params = self._journal_where(since_ts, until_ts) + row = self._conn.execute(f"SELECT COUNT(*) AS n FROM journal{where}", params).fetchone() + return int(row["n"]) + + @staticmethod + def _journal_where(since_ts: int | None, until_ts: int | None) -> tuple[str, list[Any]]: + """The half-open window, shared by the read and the count so the two cannot disagree + about which entries are in it.""" + clauses: list[str] = [] + params: list[Any] = [] + if since_ts is not None: + clauses.append("ts >= ?") + params.append(since_ts) + if until_ts is not None: + clauses.append("ts < ?") + params.append(until_ts) + return ((" WHERE " + " AND ".join(clauses)) if clauses else "", params) + + def _journal_row_to_dict(self, row: sqlite3.Row) -> dict[str, Any]: + entry = dict(row) + entry["dollar_impact"] = _text_to_dec(entry.get("dollar_impact")) + raw = entry.get("rules_followed") + # THREE-VALUED. `bool(None)` is `False`, and `False` on this column is the operator + # saying they broke their rules -- a confession nobody should make by leaving a prompt + # blank. + entry["rules_followed"] = None if raw is None else bool(raw) + return entry + # -- the audit chain ------------------------------------------------ def rollback(self) -> None: diff --git a/keel/web/api.py b/keel/web/api.py index ea37d17..cd14abe 100644 --- a/keel/web/api.py +++ b/keel/web/api.py @@ -710,14 +710,34 @@ def read_journal(cfg: ServeConfig, query: Query, _state: Any, now_ts: int) -> di redrawn in `pnl` order would be a cumulative total of a sequence that never happened. """ from keel.commands.insights import build_equity_curve, build_journal_report + from keel.commands.journal import DEFAULT_NOTES_LIMIT, gather_journal limit = _journal_limit(query) repo = open_repo(cfg.db_path) try: report = build_journal_report(repo, _status_report(cfg, now_ts), now_ts, limit=limit) + # #705's DISCRETIONARY journal, on this route rather than one of its own. Two things + # called a journal, and this is the page where the distinction has to be visible: the + # table above is closed trades as a venue reported them, and these are sentences the + # operator wrote about themselves. Putting them on separate pages would let a reader meet + # one without ever learning the other exists. + # + # It is NOT this route's `collection`, so `?sort=` reorders the trades and leaves these + # alone -- and that is a refusal, not an omission. A journal reads forwards; sorted by + # dollar impact it becomes a ranking of your own worst days, which is the shape this + # codebase refuses everywhere else it appears. + # + # ITS OWN CAP, not the trades' `?limit=`. The first cut passed `limit` through, so + # narrowing to one closed trade silently hid 300 of an operator's 301 notes -- one + # record's page control truncating a different record, by the coincidence of their + # sharing a route. `total_count` rides the payload either way, so the page can say what + # it is not showing. + notes = gather_journal(repo, now_ts=now_ts, limit=DEFAULT_NOTES_LIMIT) finally: close_repo(repo) - return payload.journal_payload(report, curve=build_equity_curve(report.entries)) + return payload.journal_payload( + report, curve=build_equity_curve(report.entries), notes=notes + ) def read_rules(cfg: ServeConfig, _query: Query, _state: Any, _now_ts: int) -> dict[str, Any]: diff --git a/keel/web/payload.py b/keel/web/payload.py index 4016018..c82eec7 100644 --- a/keel/web/payload.py +++ b/keel/web/payload.py @@ -134,6 +134,11 @@ JournalReport, RuleTrackRecord, ) + + # #705's journal is a DIFFERENT record from `insights.JournalReport` above -- closed trades + # there, the operator's own account of themselves here -- and the alias says so at the import + # rather than leaving two `JournalReport`s in one namespace to be told apart by context. + from keel.commands.journal import JournalReport as DiscretionaryJournal from keel.commands.orders import OrderRow, OrdersReport from keel.commands.positions import PositionRow, PositionsReport from keel.commands.research_record import RuleExploration, TrialRow, TrialsReport @@ -1189,7 +1194,9 @@ def equity_series_payload(series: EquitySeries) -> dict[str, Any]: } -def journal_payload(report: JournalReport, *, curve: EquityCurve) -> dict[str, Any]: +def journal_payload( + report: JournalReport, *, curve: EquityCurve, notes: DiscretionaryJournal +) -> dict[str, Any]: """`build_journal_report`'s `JournalReport`, as JSON. `curve` is passed in rather than built here, and the direction is the point: `keel/web/api.py` @@ -1243,9 +1250,123 @@ def journal_payload(report: JournalReport, *, curve: EquityCurve) -> dict[str, A "filters": {str(key): stringify(value) for key, value in sorted(report.filters.items())}, "entries": entries, "curve": equity_curve_payload(curve), + # #705's DISCRETIONARY journal, beside the closed trades rather than on a page of its own: + # two things called a journal, and this is where a reader has to be able to tell them + # apart. A REQUIRED keyword like `curve`, for the same reason -- a default would let every + # existing caller keep working while quietly serving the page without it. + "notes": _discretionary_journal_payload(notes), } +def _discretionary_journal_payload(notes: DiscretionaryJournal) -> dict[str, Any]: + """The operator's own account of their own conduct (#705). + + **Every figure here is a claim by the person reading it.** `dollar_impact` is what the + operator SAYS the day cost them -- not a fill, not a fee, nothing a venue reported -- so it + carries `WARN` and a marker rather than sitting in a money column looking like the ones above + it. That is the same judgement `_PROVENANCE_STATES` makes for `self-reported` in the timeline, + and for the same reason: a row that reads like evidence and is not is the one thing an honest + surface must not produce. + + `rules_followed` is a THREE-valued flag. `False` is the operator confessing they broke their + own rules -- the most consequential thing this table can hold -- and `None` is a question they + skipped, so `flag()`'s two-state rendering would print the confession over the silence. + + No sortable column, here or on the route. A journal reads forwards, and sorted by dollar + impact it becomes a ranking of the operator's own worst days: the Strathern rail, in the one + place where the thing being ranked is a person. + """ + # The marker word is READ from the command module rather than restated here, so the CLI and + # the console cannot come to use two different words for the one record neither can verify. + from keel.commands.journal import SELF_REPORTED + + return { + "marker": SELF_REPORTED, + "recorded": flag( + notes.any_recorded, + on="recorded", + off="no journal entries yet — `keel journal add` writes one, and only a person can", + on_state=NEUTRAL, + off_state=UNKNOWN, + ), + # BOTH counts, and a sentence that already says which is which. A journal is capped on + # this page, and a short list with nothing beside it reads as a complete one -- the + # failure `Repository.get_equity_points`' docstring names ("a caller that bounds a read is + # showing a WINDOW of the record and must say so"). The sentence is composed here rather + # than by the client, because choosing between "3 entries" and "50 of 301 (newest)" is a + # judgement (Rule 2) and the client may not count (Rule 6e). + "shown_count": count(notes.entry_count), + "total_count": count(notes.total_count), + "window": label( + "page" if notes.truncated else "all", + display=_notes_window_display(notes), + state=WARN if notes.truncated else NEUTRAL, + ), + "entries": [ + { + "at": moment(entry.ts), + "reference": str(entry.id), + "emotion": label( + entry.emotion_score, + display=entry.emotion_score or None, + state=NEUTRAL, + ), + # `""` and not `None` for the skipped question. `label(None)` is `absent()`, + # whose display is the em-dash -- fine in a numeric column and ambiguous here, + # where the neighbouring values read "followed their rules" and "BROKE their own + # rules": a dash between those two invites the reader to supply the missing one. + # The empty `value` still says absent to anything reading the field + # programmatically, and the display says which absence it is. + "rules_followed": label( + "" + if entry.rules_followed is None + else ("yes" if entry.rules_followed else "no"), + display=_rules_followed_display(entry.rules_followed), + state=_rules_followed_state(entry.rules_followed), + ), + "errors_made": entry.errors_made or "", + # WARN, and named as self-reported in `amount_kind`'s spirit: this figure is the + # operator's estimate of their own damage, and a money column that did not say so + # would invite it to be added to the venue-reported ones above. + "dollar_impact": money(entry.dollar_impact, state=WARN), + "chart_note": entry.chart_note or "", + "screenshot_ref": entry.screenshot_ref or "", + } + for entry in notes.entries + ], + } + + +def _notes_window_display(notes: DiscretionaryJournal) -> str: + """What the page says about how much of the journal it is showing. + + `WARN` on a truncated window rather than `NEUTRAL`, because the reader is looking at an + incomplete record of their own conduct and the whole point of the sentence is that they + notice. + """ + if not notes.any_recorded: + return "nothing recorded" + if notes.truncated: + return f"showing the {notes.entry_count} most recent of {notes.total_count}" + return f"showing all {notes.entry_count}" + + +def _rules_followed_display(value: bool | None) -> str: + """Three readings for three values, spelled out. "not said" is not "no".""" + if value is None: + return "not said" + return "followed their rules" if value else "BROKE their own rules" + + +def _rules_followed_state(value: bool | None) -> str: + """`UNKNOWN` for the skipped question, `WARN` for the confession, `NEUTRAL` for the ordinary + day. Never `GOOD`: "I followed my rules" is a self-assessment, and grading it green would have + the console endorsing a claim nothing verified.""" + if value is None: + return UNKNOWN + return NEUTRAL if value else WARN + + # -- activity ------------------------------------------------------------------------------------ @@ -1780,6 +1901,11 @@ def balances_payload(report: BalancesReport) -> dict[str, Any]: "simulated": WARN, "imported-ledger": NEUTRAL, "human-attested": NEUTRAL, + # WARN, like `simulated`, and for the parallel reason: both mark a row that reads like + # evidence and is not. `simulated` warns that no venue was involved; this warns that nothing + # outside the operator's own head was. Not `BAD` -- a self-assessment is worth keeping, and + # grading the operator's honesty is not this column's job. + "self-reported": WARN, "engine-log": NEUTRAL, } @@ -1791,6 +1917,7 @@ def balances_payload(report: BalancesReport) -> dict[str, Any]: "simulated": "the paper trader wrote this -- no venue was involved", "imported-ledger": "imported from a venue CSV; nothing verified it on the way in", "human-attested": "a person typed this and signed their name to it", + "self-reported": "the operator's own account of their own conduct — nothing verified it", "engine-log": "the agent's own log of what it did", } diff --git a/keel/web/static/js/render.js b/keel/web/static/js/render.js index 8c7e0af..2efc78a 100644 --- a/keel/web/static/js/render.js +++ b/keel/web/static/js/render.js @@ -2538,6 +2538,72 @@ export function insightsView(insights, journal, sort, onSort, journalSort, onJou row.setAttribute("tabindex", "0"); } fragment.append(journalTable); + fragment.append(notesSection(journal.notes)); + return fragment; +} + +/** + * The DISCRETIONARY journal (#705): what the operator wrote about themselves. + * + * **Below the closed trades, deliberately, and never blended into them.** Two records on one page + * both called a journal, and this is where a reader has to be able to tell them apart: the table + * above is what a venue reported, and nothing outside this operator's own head produced anything + * here. So the heading carries the marker (`SELF-REPORTED`, written by the payload so the CLI and + * this page cannot come to use two different words for it) and the impact column says so again on + * every row. + * + * **No sort control, on any column.** Not an omission -- a journal reads forwards, and sorted by + * dollar impact it becomes a ranking of the operator's own worst days. That is the Strathern rail + * in the one place where the thing being ranked is a person, and `api.py` refuses it on the route + * as well: these entries are not this endpoint's sortable collection. + * + * Every cell that carries a judgement arrives as a `Field` -- `rules_followed` is three-valued + * (`not said` is not `no`) and `dollar_impact` warns because it is a claim rather than a fill. + * This function places them and decides nothing. + * + * @param {any} notes `/api/journal`'s `notes`, or `null`/undefined where that read failed. + * @returns {DocumentFragment} + */ +function notesSection(notes) { + const fragment = document.createDocumentFragment(); + if (!notes) return fragment; + + fragment.append(heading("h-notes", "Your own account")); + const sub = el("p", "sub"); + sub.append(pill(plain(notes.marker), "warn"), " "); + sub.append(field(notes.recorded), " · "); + // How much of the journal this is. A capped list with nothing beside it reads as a complete + // one, and the sentence comes off the payload -- choosing between "showing all 3" and "showing + // the 50 most recent of 301" is a judgement, and this file may not count either. + sub.append(field(notes.window)); + fragment.append(sub); + + fragment.append( + table( + "h-notes", + [ + { label: "when (UTC)", numeric: false }, + { label: "emotion", numeric: false }, + { label: "rules", numeric: false }, + { label: "errors made", numeric: false }, + { label: "self-reported impact", numeric: true }, + { label: "chart note", numeric: false }, + { label: "screenshot", numeric: false }, + ], + (notes.entries || []).map(/** @param {any} entry */ (entry) => [ + entry.at, + entry.emotion, + entry.rules_followed, + plain(entry.errors_made) || "—", + entry.dollar_impact, + plain(entry.chart_note) || "—", + plain(entry.screenshot_ref) || "—", + ]), + // From the payload, never written here: "no entries yet" and "this could not be read" are + // different sentences, and choosing between them is a judgement (Rule 2). + plain(notes.recorded.display), + ), + ); return fragment; } diff --git a/tests/commands/test_journal.py b/tests/commands/test_journal.py new file mode 100644 index 0000000..6181bf4 --- /dev/null +++ b/tests/commands/test_journal.py @@ -0,0 +1,230 @@ +"""`keel journal` -- the CLI that is the ONLY way in (#705). + +The constitution's line is that attestations are human-sourced or refused. This is the purest +case of it in the codebase: the entry is a person's account of their own conduct, and there is no +other source it could come from. So there is no web form, no `--json` input, no pipe, and no +default that would let an unattended process record a feeling on someone's behalf. +""" + +from __future__ import annotations + +from decimal import Decimal + +import pytest +from click.testing import CliRunner + +from keel.cli import cli +from keel.commands import journal as journal_mod +from keel.data.db import connect, migrate +from keel.data.repository import Repository + +NOW = 1_756_000_000 + + +@pytest.fixture() +def deployment(tmp_path, monkeypatch: pytest.MonkeyPatch): + """A migrated database and a CLI that believes it is at a terminal.""" + db_path = tmp_path / "keel.db" + conn = connect(str(db_path)) + migrate(conn) + conn.close() + monkeypatch.setattr("keel.commands._common._is_interactive", lambda: True) + return db_path + + +def _run(deployment, args: list[str], stdin: str = "") -> object: + return CliRunner().invoke(cli, ["--db", str(deployment), *args], input=stdin) + + +def _entries(deployment) -> list[dict]: + conn = connect(str(deployment)) + migrate(conn) + return Repository(conn).get_journal_entries() + + +# -- the refusals ------------------------------------------------------------------------------ + + +def test_adding_an_entry_off_a_terminal_is_refused(deployment, monkeypatch) -> None: + """Fails closed off a TTY, so cron, a pipe and a script can never write a feeling into + someone's journal. The same gate `resume` and `assets attest` sit behind.""" + monkeypatch.setattr("keel.commands._common._is_interactive", lambda: False) + result = _run(deployment, ["journal", "add"]) + + assert result.exit_code != 0 + assert "interactive terminal" in result.output + assert _entries(deployment) == [] + + +def test_the_add_command_takes_no_value_options_at_all() -> None: + """Not merely "it prompts": a `--emotion 3` would make the whole entry scriptable, and the + TTY gate would then guard a ceremony that no longer needed a human to supply anything. + + Asserted as the PROPERTY -- click's own parameter list is empty -- rather than as a blocklist + of six names I happened to think of. The first version scanned `--help` for `--emotion`, + `--impact` and four others, which a `--feeling 3` or a `--score 3` walks straight past while + defeating exactly what the docstring defends. + """ + from keel.commands.journal import journal_add + + assert [param.name for param in journal_add.params] == [] + + +def test_nothing_outside_the_cli_can_reach_the_writer() -> None: + """No web write path, by construction. + + The FIRST version of this guarded `keel/web/api.py`, where a write could not have lived: that + module's own docstring says "Reads only. Not one route below answers a POST". The real surface + is `keel.commands.setup.ACTIONS` -- the only thing `server.do_POST` will route to -- and it + ALREADY carries a human-attestation writer (`attest_asset`), which is exactly why a "quick + note" box is the plausible next addition. Demonstrated: adding a journal writer to `setup.py` + passed the old test untouched. + """ + import inspect + + from keel.commands import setup + from keel.web import api + + for module in (setup, api): + text = inspect.getsource(module) + assert "append_journal_entry" not in text, ( + f"{module.__name__} can reach the journal writer; the console's write surface is " + "`setup.ACTIONS` and the journal must not be on it" + ) + + +# -- what it writes ---------------------------------------------------------------------------- + + +def test_a_full_entry_is_recorded_exactly_as_typed(deployment, monkeypatch) -> None: + monkeypatch.setattr(journal_mod.time, "time", lambda: NOW) + result = _run( + deployment, + ["journal", "add"], + stdin="3\ny\nentered before the close confirmed\n-42.50\nrange top\n~/shot.png\n", + ) + assert result.exit_code == 0, result.output + + (entry,) = _entries(deployment) + assert entry["ts"] == NOW + assert entry["emotion_score"] == "3" + assert entry["rules_followed"] is True + assert entry["errors_made"] == "entered before the close confirmed" + assert entry["dollar_impact"] == Decimal("-42.50") + assert entry["chart_note"] == "range top" + assert entry["screenshot_ref"] == "~/shot.png" + + +def test_a_skipped_field_is_recorded_as_unsaid_not_as_a_zero(deployment) -> None: + """The refusal in the issue: no pre-filled emotion scores. An empty answer must reach the + database as NULL, because a `0` impact and a `3` emotion are claims the operator did not + make.""" + result = _run(deployment, ["journal", "add"], stdin="\n\n\n\n\n\n") + assert result.exit_code == 0, result.output + + (entry,) = _entries(deployment) + for field in ("emotion_score", "rules_followed", "errors_made", "dollar_impact"): + assert entry[field] is None, f"{field} was invented from a blank answer" + + +def test_an_emotion_score_outside_the_scale_is_refused_not_stored(deployment) -> None: + """A score needs a scale or it cannot be compared with itself next week. 1-5, stated in the + prompt, and a value off it is rejected rather than quietly kept as free text.""" + result = _run(deployment, ["journal", "add"], stdin="9\n\n\n\n\n\n") + + assert result.exit_code != 0 + assert "1" in result.output and "5" in result.output + assert _entries(deployment) == [] + + +def test_an_unparseable_dollar_impact_is_refused_not_rounded(deployment) -> None: + result = _run(deployment, ["journal", "add"], stdin="\n\n\nlots\n\n\n") + + assert result.exit_code != 0 + assert _entries(deployment) == [] + + +# -- reading it back --------------------------------------------------------------------------- + + +def test_list_reads_the_entries_back_oldest_first(deployment) -> None: + conn = connect(str(deployment)) + migrate(conn) + repo = Repository(conn) + repo.append_journal_entry(ts=200, chart_note="second") + repo.append_journal_entry(ts=100, chart_note="first") + conn.close() + + result = _run(deployment, ["journal", "list"]) + assert result.exit_code == 0, result.output + assert result.output.index("first") < result.output.index("second") + + +def test_list_says_so_when_there_is_nothing_rather_than_printing_a_bare_header( + deployment, +) -> None: + result = _run(deployment, ["journal", "list"]) + assert result.exit_code == 0 + assert "No journal entries" in result.output + + +def test_the_report_marks_every_entry_self_reported(deployment) -> None: + """The marker the issue asks for, on the CLI too. An operator reading their own journal + beside `keel insights journal` -- which is closed TRADES, a different thing wearing a similar + name -- must not have to remember which is which.""" + conn = connect(str(deployment)) + migrate(conn) + Repository(conn).append_journal_entry(ts=100, chart_note="a note") + conn.close() + + result = _run(deployment, ["journal", "list"]) + assert "SELF-REPORTED" in result.output + + +# -- the acceptance round trip ------------------------------------------------------------------ + + +def test_cli_add_then_web_view_then_csv_export_all_carry_the_marker(deployment, monkeypatch): + """The issue's acceptance criterion, end to end and in one test. + + Three surfaces read this record and each could lose the provenance separately: the CLI that + wrote it, the console that renders it, and the CSV an operator hands to someone else. The + export is the one that leaves the machine, so it is the one where a self-assessment sitting + unmarked beside venue-reported fills would do real harm. + """ + from keel.commands.journal import SELF_REPORTED + from keel.commands.timeline import export_rows, to_csv + from keel.web import payload + + monkeypatch.setattr(journal_mod.time, "time", lambda: NOW) + written = _run(deployment, ["journal", "add"], stdin="2\nn\nchased it\n-42.50\nthin\n\n") + assert written.exit_code == 0, written.output + + conn = connect(str(deployment)) + migrate(conn) + repo = Repository(conn) + + # 1. The CLI reads it back with the marker. + listed = _run(deployment, ["journal", "list"]) + assert SELF_REPORTED in listed.output + assert "NO" in listed.output, "a broken rule must be legible, not merely stored" + + # 2. The web payload carries the marker and grades the CLAIM rather than the operator. + # + # `_discretionary_journal_payload` directly rather than the whole `journal_payload`: the + # closed-trade half needs a full `StatusReport` this test has no business building, and that + # the two halves are composed onto one route is pinned by `test_client_assets.py`'s parity + # scan over `/api/journal`'s `notes.entries`. + notes = payload._discretionary_journal_payload(journal_mod.gather_journal(repo, now_ts=NOW)) + assert notes["marker"] == SELF_REPORTED + (row,) = notes["entries"] + assert row["dollar_impact"]["state"] == "warn", "a self-reported figure must not read as a fill" + assert row["rules_followed"]["state"] == "warn" + assert "BROKE" in row["rules_followed"]["display"] + + # 3. The export names it as self-reported beside the venue-reported rows. + text = to_csv(export_rows(repo, now_ts=NOW + 10)) + lines = [line for line in text.splitlines() if "journal" in line] + assert len(lines) == 1 + assert "self-reported" in lines[0] + assert "chased it" in lines[0] diff --git a/tests/commands/test_timeline.py b/tests/commands/test_timeline.py index 5ed3d29..a5335fd 100644 --- a/tests/commands/test_timeline.py +++ b/tests/commands/test_timeline.py @@ -626,3 +626,119 @@ def test_a_broken_chain_is_stated_above_the_header_not_only_per_row(db_conn) -> text = timeline.to_csv(timeline.export_rows(repo, now_ts=2_000)) assert text.splitlines()[0].startswith("# NOTE") assert "chain" in text.splitlines()[0] + + +# -- the journal in the feed (#705) -------------------------------------------------------------- + + +def _journal(repo: Repository, **overrides: Any) -> int: + row: dict[str, Any] = {"ts": NOW_TS - 100, "chart_note": "range top, thin book"} + row.update(overrides) + return repo.append_journal_entry(**row) + + +def test_a_journal_entry_reaches_the_feed_under_the_attestation_chip(repo: Repository) -> None: + """It IS an attestation -- something a person put their name to -- and the chip groups it with + the others. What keeps it from being READ as one is the provenance column, not the chip.""" + _journal(repo) + report = gather_timeline(repo, now_ts=NOW_TS, scope="all") + + (row,) = [r for r in report.rows if r.source == "journal"] + assert row.kind == "attestation" + assert "attestation" in report.kinds_present + + +def test_a_journal_entry_is_self_reported_and_never_human_attested(repo: Repository) -> None: + """Two different kinds of claim. An asset attestation says PAXG is backed by allocated gold, + which a prospectus could contradict; "I broke my rule" has no external referent at all. Filing + the second under the word this feed uses for checkable human claims would put the one + unverifiable record in the database under a heading implying otherwise.""" + _journal(repo) + _attestation(repo) + report = gather_timeline(repo, now_ts=NOW_TS, scope="all") + + by_source = {r.source: r.provenance for r in report.rows} + assert by_source["journal"] == "self-reported" + assert by_source["asset_attestations"] == "human-attested" + + +def test_the_self_reported_impact_is_labelled_as_self_reported(repo: Repository) -> None: + """A self-reported dollar figure and a venue-reported fee in one column, with nothing saying + which is which, is a column that will be summed.""" + _journal(repo, dollar_impact=Decimal("-42.50")) + report = gather_timeline(repo, now_ts=NOW_TS, scope="all") + + (row,) = [r for r in report.rows if r.source == "journal"] + assert row.amount == Decimal("-42.50") + assert "self-reported" in row.amount_kind + + +def test_a_broken_rule_is_said_and_a_skipped_question_is_not(repo: Repository) -> None: + """`rules_followed` is three-valued and `bool(None)` is `False`. Printing the confession over + the silence would put words in the operator's mouth on the one row where that matters most.""" + _journal(repo, ts=NOW_TS - 300, rules_followed=False, chart_note="rushed it") + _journal(repo, ts=NOW_TS - 200, rules_followed=None, chart_note="quiet day") + _journal(repo, ts=NOW_TS - 100, rules_followed=True, chart_note="by the book") + + summaries = { + r.reference: r.summary for r in gather_timeline(repo, now_ts=NOW_TS).rows + if r.source == "journal" + } + broke = [s for s in summaries.values() if "BROKE RULES" in s] + assert len(broke) == 1 + assert "rushed it" in broke[0] + + +def test_an_empty_entry_still_says_something(repo: Repository) -> None: + """An operator can record a day with nothing written on it. A blank summary would make the + feed's densest human content its least legible.""" + _journal(repo, chart_note=None) + (row,) = [r for r in gather_timeline(repo, now_ts=NOW_TS).rows if r.source == "journal"] + assert row.summary.strip() + + +def test_a_journal_entry_carries_its_chain_hash_like_every_other_record(repo: Repository) -> None: + """It rides the audit export beside orders and fills, so it is chained beside them too.""" + _journal(repo) + (row,) = [r for r in gather_timeline(repo, now_ts=NOW_TS).rows if r.source == "journal"] + assert row.chain_status == "chained" + assert len(row.row_hash) == 64 + + +def test_the_journal_respects_the_scope_window(repo: Repository) -> None: + _journal(repo, ts=NOW_TS - 100) + _journal(repo, ts=NOW_TS - (40 * 86_400)) + assert len([r for r in gather_timeline(repo, now_ts=NOW_TS, scope="7d").rows + if r.source == "journal"]) == 1 + assert len([r for r in gather_timeline(repo, now_ts=NOW_TS, scope="all").rows + if r.source == "journal"]) == 2 + + +def test_the_export_row_carries_every_sentence_the_operator_wrote(repo: Repository) -> None: + """This row is the journal's WHOLE representation in the CSV -- there is no other column any + of it could reappear in. The first cut used `elif`, so an entry carrying both an error and a + chart note exported only the error, and `screenshot_ref` reached the file nowhere at all.""" + _journal( + repo, + errors_made="entered early", + chart_note="range top", + screenshot_ref="~/shot.png", + emotion_score="2", + ) + text = timeline.to_csv(timeline.export_rows(repo, now_ts=NOW_TS)) + (line,) = [row for row in text.splitlines() if "journal" in row] + + for written in ("entered early", "range top", "~/shot.png", "emotion 2"): + assert written in line, f"{written!r} is in no column and in no summary" + + +def test_a_hostile_journal_note_cannot_execute_in_a_spreadsheet(repo: Repository) -> None: + """Journal text is operator-typed free text and it lands in a cell of a file an auditor + opens. `csv_safe` applies to every cell, and the summary's `.strip()` means a note beginning + with a tab or a newline still reaches it starting with the trigger.""" + _journal(repo, errors_made="=cmd|' /C calc'!A0", chart_note=None) + text = timeline.to_csv(timeline.export_rows(repo, now_ts=NOW_TS)) + (line,) = [row for row in text.splitlines() if "cmd" in row] + + assert "'=cmd" in line + assert ",=cmd" not in line and not line.startswith("=cmd") diff --git a/tests/data/test_journal.py b/tests/data/test_journal.py new file mode 100644 index 0000000..972a8a6 --- /dev/null +++ b/tests/data/test_journal.py @@ -0,0 +1,271 @@ +"""The discretionary journal's storage layer (#705). + +The `journal` table has been declared in the schema since the beginning and had NO repository +method and no read or write path anywhere in the code -- dead schema, which is worse than no +schema because a reader assumes a declared table is a used one. + +What it records is unlike anything else in this database: the operator's own account of their own +conduct. Everything else here is a machine's observation or a claim about the world that a +document could contradict. A self-assessment cannot be checked at all, and the whole point of +keeping it is that it stays visibly separate from the things that can. +""" + +from __future__ import annotations + +import sqlite3 +from decimal import Decimal + +import pytest + +from keel.data import audit, db +from keel.data.repository import Repository + + +@pytest.fixture() +def repo() -> Repository: + conn = db.connect(":memory:") + db.migrate(conn) + return Repository(conn) + + +def test_an_entry_round_trips_every_field(repo: Repository) -> None: + repo.append_journal_entry( + ts=1_000, + emotion_score="3", + rules_followed=True, + errors_made="entered before the close confirmed", + dollar_impact=Decimal("-42.50"), + chart_note="range top, thin book", + screenshot_ref="~/shots/2026-09-05.png", + ) + (entry,) = repo.get_journal_entries() + + assert entry["ts"] == 1_000 + assert entry["emotion_score"] == "3" + assert entry["rules_followed"] is True + assert entry["errors_made"] == "entered before the close confirmed" + assert entry["dollar_impact"] == Decimal("-42.50") + assert entry["chart_note"] == "range top, thin book" + assert entry["screenshot_ref"] == "~/shots/2026-09-05.png" + + +def test_dollar_impact_keeps_its_scale_and_its_sign(repo: Repository) -> None: + """Money is `Decimal` stored as TEXT, like every other money column here. A float would + make a self-reported dollar impact disagree with itself between writes.""" + repo.append_journal_entry(ts=1, dollar_impact=Decimal("-0.10")) + (entry,) = repo.get_journal_entries() + assert entry["dollar_impact"] == Decimal("-0.10") + assert str(entry["dollar_impact"]) == "-0.10" + + +def test_every_field_but_the_timestamp_may_be_absent(repo: Repository) -> None: + """The schema makes only `ts` NOT NULL, and that is the right shape for this table: an + operator who wants to record one sentence about one day must not have to invent an emotion + score to do it. `None` is "did not say", never a zero or an empty string.""" + repo.append_journal_entry(ts=1) + (entry,) = repo.get_journal_entries() + + for field in ("emotion_score", "rules_followed", "errors_made", "dollar_impact"): + assert entry[field] is None, f"{field} invented a value" + + +def test_rules_followed_is_three_valued(repo: Repository) -> None: + """`bool(None)` is `False`, and `False` here is a POSITIVE claim -- "I broke my rules". An + operator who skipped the question has not confessed to anything.""" + repo.append_journal_entry(ts=1, rules_followed=None) + repo.append_journal_entry(ts=2, rules_followed=False) + repo.append_journal_entry(ts=3, rules_followed=True) + + assert [e["rules_followed"] for e in repo.get_journal_entries()] == [None, False, True] + + +def test_entries_come_back_oldest_first(repo: Repository) -> None: + for ts in (300, 100, 200): + repo.append_journal_entry(ts=ts) + assert [e["ts"] for e in repo.get_journal_entries()] == [100, 200, 300] + + +def test_two_entries_at_one_instant_keep_a_stable_order(repo: Repository) -> None: + """`id` breaks the tie. A journal is a sequence of what someone wrote, and two notes on one + day must not swap places between reads.""" + first = repo.append_journal_entry(ts=1, chart_note="first") + second = repo.append_journal_entry(ts=1, chart_note="second") + assert first < second + assert [e["chart_note"] for e in repo.get_journal_entries()] == ["first", "second"] + + +def test_the_date_bounds_are_inclusive_of_since_and_exclusive_of_until(repo: Repository) -> None: + """Half-open, matching every other window in this codebase (`get_candles`, + `scope_start_ts`), so two adjacent windows cover a range without double-counting the seam.""" + for ts in (100, 200, 300): + repo.append_journal_entry(ts=ts) + + assert [e["ts"] for e in repo.get_journal_entries(since_ts=200)] == [200, 300] + assert [e["ts"] for e in repo.get_journal_entries(until_ts=300)] == [100, 200] + assert [e["ts"] for e in repo.get_journal_entries(since_ts=200, until_ts=300)] == [200] + + +def test_the_limit_keeps_the_NEWEST_entries(repo: Repository) -> None: + """A capped read of a journal wants the recent end -- and still returns them oldest-first, so + the cap changes how many entries a caller sees and never which way they read.""" + for ts in (100, 200, 300): + repo.append_journal_entry(ts=ts, chart_note=str(ts)) + + assert [e["ts"] for e in repo.get_journal_entries(limit=2)] == [200, 300] + + +def test_an_entry_is_chained_like_every_other_attestation(repo: Repository) -> None: + """#721's chain covers what a human swore to. A journal entry is the most purely + human-sourced record in this database, and the export shows it beside orders and fills.""" + repo.append_journal_entry(ts=1_000, chart_note="a note") + + conn = repo._conn # noqa: SLF001 - the chain is read by store, not by repository method + events = audit.read_events(conn) + assert [e.event_type for e in events] == ["journal_recorded"] + assert audit.chain_state(conn).errors == () + + +def test_the_event_is_filed_under_the_rows_own_id(repo: Repository) -> None: + """`journal` has no natural key -- no `coinbase_id`, no asset, no venue pair. The row id is + the only identifier, and it is what `commands/timeline.py` prints as the reference.""" + entry_id = repo.append_journal_entry(ts=1_000) + conn = repo._conn # noqa: SLF001 + assert [e.entity_id for e in audit.read_events(conn)] == [str(entry_id)] + + +def test_an_entry_and_its_chain_row_land_together_or_not_at_all( + repo: Repository, monkeypatch: pytest.MonkeyPatch +) -> None: + def _boom(*_args: object, **_kwargs: object) -> None: + raise sqlite3.OperationalError("disk I/O error") + + monkeypatch.setattr("keel.data.repository.append_event", _boom) + with pytest.raises(sqlite3.OperationalError): + repo.append_journal_entry(ts=1_000) + + assert repo.get_journal_entries() == [] + + +def test_the_journal_table_is_no_longer_dead_schema(repo: Repository) -> None: + """The condition that made #705 necessary, pinned so it cannot return. + + A declared table with no repository method is worse than no table: a reader assumes a declared + one is used, and `journal` sat in the schema from the beginning with no read or write path + anywhere in the code. This asserts the methods exist AND that they reach the real table -- + a method that wrote somewhere else would satisfy a name check and nothing else. + """ + assert hasattr(repo, "append_journal_entry") + assert hasattr(repo, "get_journal_entries") + + repo.append_journal_entry(ts=1, chart_note="proof") + stored = repo._conn.execute("SELECT chart_note FROM journal").fetchall() # noqa: SLF001 + assert [row["chart_note"] for row in stored] == ["proof"] + + +def test_there_is_no_way_to_edit_or_delete_an_entry(repo: Repository) -> None: + """Append-only, and the absence is the design: a journal you can go back and change is a + journal that records what you wish you had thought. Pinned by name, because the natural thing + for a later contributor to add beside two existing methods is a third that updates.""" + for forbidden in ("update_journal_entry", "delete_journal_entry", "set_journal_entry"): + assert not hasattr(repo, forbidden), f"Repository grew {forbidden}" + + # And the SQL, over the whole package -- a name sweep misses `amend_`, `edit_`, a generic + # executor, and anything that reaches the table without a method at all. This is the property; + # the names above are the readable half of it. + import pathlib + + root = pathlib.Path(__file__).resolve().parents[2] / "keel" + offenders = [ + path + for path in root.rglob("*.py") + for text in [path.read_text(encoding="utf-8")] + if "UPDATE journal" in text or "DELETE FROM journal" in text + ] + assert offenders == [], f"the journal is mutated in {offenders}" + + +def test_the_limit_breaks_a_timestamp_tie_by_id(repo: Repository) -> None: + """With three entries in one second and a cap of two, "newest" means the two written LAST. + + **This test's own blind spot, named.** It pins the OUTCOME, not the `id DESC` clause that + guarantees it: measured on this SQLite build, dropping `id DESC` from the subquery returns the + same two rows, because a bare `ORDER BY ts DESC` happens to fall back to rowid order. That is + an implementation detail of one engine and not a promise, so the clause stays -- the sibling + `get_equity_points` states the reasoning ("applied in BOTH directions so the newest-N and the + oldest-first re-order agree about which of two same-second readings is the newer") -- but no + test in this suite can currently make its absence fail, and saying so beats implying otherwise. + """ + for note in ("first", "second", "third"): + repo.append_journal_entry(ts=1_000, chart_note=note) + + assert [e["chart_note"] for e in repo.get_journal_entries(limit=2)] == ["second", "third"] + + +def test_the_count_is_the_window_before_the_limit(repo: Repository) -> None: + """What lets a bounded read SAY what it left out -- the rule `get_equity_points`' docstring + states and `count_equity_points` exists to serve.""" + for ts in (100, 200, 300): + repo.append_journal_entry(ts=ts) + + assert repo.count_journal_entries() == 3 + assert len(repo.get_journal_entries(limit=1)) == 1 + assert repo.count_journal_entries(since_ts=200) == 2 + assert repo.count_journal_entries(since_ts=200, until_ts=300) == 1 + + +def test_an_empty_string_is_read_back_as_unsaid(repo: Repository) -> None: + """`label(None)` is `absent()` and `label("")` is an empty cell, so a column holding `""` + would render as a second, different-looking spelling of "did not say".""" + from keel.commands.journal import gather_journal + + repo.append_journal_entry(ts=1, chart_note="", errors_made="", emotion_score="") + (entry,) = gather_journal(repo, now_ts=2).entries + + assert entry.chart_note is None + assert entry.errors_made is None + assert entry.emotion_score is None + + +def test_a_zero_or_negative_limit_is_refused_rather_than_obeyed(repo: Repository) -> None: + """SQLite reads a NEGATIVE `LIMIT` as unbounded, so `--limit -1` would silently print + everything; a zero returns no rows, and an empty result is indistinguishable from an empty + journal on both front-ends. That is the hazard `keel/web/api.py::_journal_limit` was written + to name, and refusing is the only reading that cannot lie. + + Refused in `gather_journal` rather than only at the click option, so the guard travels with + the function instead of with one of its callers. + """ + from keel.commands.journal import gather_journal + + repo.append_journal_entry(ts=1) + for bad in (0, -1): + with pytest.raises(ValueError, match="limit"): + gather_journal(repo, now_ts=2, limit=bad) + + +def test_a_report_knows_when_it_is_a_page_of_a_longer_journal(repo: Repository) -> None: + """The flag that lets both front-ends say "50 of 301" instead of showing a short list that + reads as a complete one.""" + from keel.commands.journal import gather_journal + + for ts in (100, 200, 300): + repo.append_journal_entry(ts=ts) + + whole = gather_journal(repo, now_ts=400) + assert whole.truncated is False + assert (whole.entry_count, whole.total_count) == (3, 3) + + page = gather_journal(repo, now_ts=400, limit=2) + assert page.truncated is True + assert (page.entry_count, page.total_count) == (2, 3) + + +def test_an_empty_window_is_not_the_same_as_an_empty_journal(repo: Repository) -> None: + """`any_recorded` reads `total_count`, not `entries`. A cap or a date bound that excluded + everything is not a deployment with no journal, and the renderers say different things.""" + from keel.commands.journal import JournalReport, gather_journal + + assert gather_journal(repo, now_ts=1).any_recorded is False + repo.append_journal_entry(ts=100) + assert gather_journal(repo, now_ts=200).any_recorded is True + assert JournalReport(now_ts=1, entries=(), total_count=7).any_recorded is True diff --git a/tests/web/test_api.py b/tests/web/test_api.py index b0fa4c0..1cbffc2 100644 --- a/tests/web/test_api.py +++ b/tests/web/test_api.py @@ -1264,3 +1264,30 @@ def test_close_repo_actually_closes_the_connection(tmp_path: Path) -> None: close_repo(repo) with pytest.raises(sqlite3.ProgrammingError): conn.execute("SELECT 1") + + +def test_the_trade_limit_does_not_truncate_the_operators_journal(tmp_path: Path) -> None: + """#705. `?limit=` is the closed-trade table's page control, and the two records share a route + only by arrangement. Passing it through meant narrowing to one trade silently hid 300 of an + operator's 301 notes -- one record's page control truncating a different record.""" + from keel.data.db import connect, migrate + from keel.data.repository import Repository + from keel.web.api import read_journal + + db_path = tmp_path / "keel.db" + conn = connect(str(db_path)) + migrate(conn) + repo = Repository(conn) + for index in range(5): + repo.append_journal_entry(ts=1_000 + index, chart_note=f"note {index}") + conn.close() + + config_path = tmp_path / "config.yaml" + config_path.write_text(VALID_CONFIG_YAML) + cfg = _serve_config(str(db_path), str(config_path)) + + body = read_journal(cfg, {"limit": ["1"]}, None, 2_000) + notes = body["notes"] + assert notes["shown_count"]["value"] == "5", "the trades' limit reached the journal" + assert notes["total_count"]["value"] == "5" + assert notes["window"]["value"] == "all" diff --git a/tests/web/test_client_assets.py b/tests/web/test_client_assets.py index f45e5dc..e71838e 100644 --- a/tests/web/test_client_assets.py +++ b/tests/web/test_client_assets.py @@ -1760,6 +1760,10 @@ def _row_reads(view: str) -> dict[tuple[str, str], set[str]]: ("rulesView", "data", "rules", "/api/rules", "rules"), ("researchView", "slippage", "rows", "/api/research/slippage", "none"), ("researchView", "gauntlet", "rows", "/api/research/gauntlet", "gauntlet"), + # #705's discretionary journal, registered WITH the code that reads it. It hangs off + # `/api/journal`'s `notes`, and the seeder writes one entry through the repository because the + # CLI writer refuses to run without a terminal. + ("insightsView", "notes", "notes.entries", "/api/journal", "journal"), ) #: Mapped collections this test does NOT cover, each with the reason. Named rather than omitted: @@ -1833,6 +1837,27 @@ def _seed_for(kind: str, db_path: str) -> None: _seed_orders(db_path, (("BTC-USD", "buy", "50000"),)) elif kind == "rules": _seed_rules(db_path, ("breakout",)) + elif kind == "journal": + # Through the REPOSITORY, not the CLI: `keel journal add` refuses to run without a + # terminal (#705), which is the property that makes the record an attestation and is + # exactly what a test harness cannot supply. + from decimal import Decimal + + from keel.data.db import connect, migrate + from keel.data.repository import Repository + + conn = connect(db_path) + migrate(conn) + Repository(conn).append_journal_entry( + ts=1_756_000_000, + emotion_score="3", + rules_followed=False, + errors_made="entered before the close confirmed", + dollar_impact=Decimal("-42.50"), + chart_note="range top", + screenshot_ref="~/shot.png", + ) + conn.close() @pytest.mark.parametrize(("view", "root", "collection", "endpoint", "seed"), _ROW_ENDPOINTS) @@ -1855,11 +1880,23 @@ def test_every_row_key_a_view_reads_is_a_key_its_endpoint_sends( assert status == 200, endpoint document = json.loads(body) assert document["engine"]["value"] == "running", document - rows = document["data"][collection] + # A DOTTED PATH, because a collection is not always a top-level key: #705's discretionary + # journal rides `/api/journal` at `data.notes.entries`, beside the closed trades it must not + # be blended into. Walking the path keeps that arrangement checkable rather than forcing a + # route of its own for the sake of this test. + rows = document["data"] + for part in collection.split("."): + assert isinstance(rows, dict), ( + f"{endpoint}: {part} is not an object on the way to {collection}" + ) + assert part in rows, f"{endpoint} does not send {collection}" + rows = rows[part] # A collection with no rows proves nothing, so an empty one is the failure rather than a pass. assert rows, f"{endpoint} sent no {collection} to check {view}'s row reads against" - reads = _row_reads(view).get((root, collection), set()) + # The SCAN is keyed by the last segment -- `render.js` maps `notes.entries`, so `_row_reads` + # sees the receiver `notes` and the collection `entries`. + reads = _row_reads(view).get((root, collection.rsplit(".", 1)[-1]), set()) assert reads, f"the scan found no row keys in {view} -- it would pass against any payload" for row in rows: @@ -1874,7 +1911,13 @@ def test_every_mapped_collection_is_either_checked_or_named() -> None: added later inherits the exact hole #725 fell into. With it, a new `.map()` over a payload collection fails the build until it is either checked or written down with a reason. """ - checked = {(view, root, collection) for view, root, collection, _e, _s in _ROW_ENDPOINTS} + # The last segment of the path, because that is what `_row_reads` sees: `render.js` maps + # `notes.entries`, so the scan reports the receiver `notes` and the collection `entries`, + # while the table above carries the payload path `notes.entries` for the walk. + checked = { + (view, root, collection.rsplit(".", 1)[-1]) + for view, root, collection, _e, _s in _ROW_ENDPOINTS + } views = {view for view, _root, _endpoint in _VIEW_ENDPOINTS} | {"statusView", "gatesView"} mapped = { @@ -1946,12 +1989,24 @@ def _comments_stripped(source: str) -> str: def _function_body(source: str, name: str) -> str: - """One exported function's body, comments stripped and string literals kept.""" + """One top-level function's body, comments stripped and string literals kept. + + Exported OR module-private: `notesSection` (#705) is private, and a helper that only knew how + to find exports raised `ValueError` on it -- which at least fails loudly, unlike the shape + where a scan silently finds nothing. Bounded at the next top-level function of either kind, so + the next function's body cannot be read as this one's. + """ code = _comments_stripped(source) - start = code.index("export function " + name + "(") + for prefix in ("export function ", "function "): + marker = prefix + name + "(" + if marker in code: + start = code.index(marker) + break + else: + raise AssertionError(f"render.js declares no top-level function {name}") rest = code[start + 1 :] - end = rest.find("\nexport function ") - return rest if end == -1 else rest[:end] + ends = [at for at in (rest.find("\nexport function "), rest.find("\nfunction ")) if at != -1] + return rest if not ends else rest[: min(ends)] def test_the_clickable_scan_can_actually_see_a_string_literal() -> None: @@ -2082,3 +2137,41 @@ def test_the_chip_separators_the_comments_describe_actually_ship() -> None: css = (_STATIC / "css" / "keel.css").read_text(encoding="utf-8") for selector in ("#session-profile:not(:empty)::after", "#session-equity:not(:empty)::before"): assert selector in css, f"no separator rule for {selector}" + + +# -- the discretionary journal's own section (#705) ----------------------------------------------- +# +# Every other pin on `notesSection` asks what it WOULD draw. None asked whether anything draws it, +# or whether the marker survives -- so deleting the call, or the `SELF-REPORTED` pill, left the +# whole 6,000-test suite green while the console lost the section and the acceptance criterion the +# issue names. The parity scan cannot catch either: an uncalled function still reads the keys it +# reads. + + +def test_the_insights_view_actually_renders_the_journal_section() -> None: + body = _function_body(_source("render.js"), "insightsView") + assert "notesSection(" in body, "insightsView never draws the discretionary journal" + + +def test_the_journal_section_shows_the_self_reported_marker() -> None: + """The issue's acceptance criterion, on the surface it names. The marker is what keeps a + self-assessment from being read as a venue fact, and it is one deleted line away from gone.""" + body = _function_body(_source("render.js"), "notesSection") + assert "notes.marker" in body + + +def test_the_journal_section_says_how_much_of_the_journal_it_is_showing() -> None: + """A capped list with nothing beside it reads as a complete one. The payload composes the + sentence; this asserts the client places it.""" + body = _function_body(_source("render.js"), "notesSection") + assert "notes.window" in body + + +def test_the_journal_section_offers_no_sort_control() -> None: + """Not an omission. A journal reads forwards, and sorted by dollar impact it becomes a ranking + of the operator's own worst days -- the Strathern rail where the thing ranked is a person. + `table()` draws a sort control only when handed a `sort`/`onSort` pair, so the refusal is the + absence of that argument.""" + body = _function_body(_source("render.js"), "notesSection") + assert "onSort" not in body + assert "sort:" not in body diff --git a/tests/web/test_payload.py b/tests/web/test_payload.py index 7592fa8..b54b86a 100644 --- a/tests/web/test_payload.py +++ b/tests/web/test_payload.py @@ -243,6 +243,16 @@ def _journal_report(**overrides: Any) -> JournalReport: return JournalReport(**base) + +def _empty_notes(): + """An empty discretionary journal (#705), for the callers that are testing the closed-trade + half. A REQUIRED keyword on `journal_payload`, like `curve`: a default would let every one of + these keep passing while the page quietly lost the section.""" + from keel.commands.journal import JournalReport + + return JournalReport(now_ts=0, entries=()) + + def _journal_json(**overrides: Any) -> dict[str, Any]: """`journal_payload` over `_journal_report(**overrides)`, with the curve built from THOSE entries. @@ -256,7 +266,9 @@ def _journal_json(**overrides: Any) -> dict[str, Any]: Nothing about `_journal_report` changed, and every test that called it directly still does. """ report = _journal_report(**overrides) - return payload.journal_payload(report, curve=build_equity_curve(report.entries)) + return payload.journal_payload( + report, curve=build_equity_curve(report.entries), notes=_empty_notes() + ) def _activity_feed(**overrides: Any) -> ActivityFeed: @@ -665,7 +677,11 @@ def _journal_case() -> tuple[str, tuple[Any, ...], dict[str, Any]]: """ report = _journal_report() curve = build_equity_curve(report.entries) - return ("journal", (report, curve), payload.journal_payload(report, curve=curve)) + return ( + "journal", + (report, curve), + payload.journal_payload(report, curve=curve, notes=_empty_notes()), + ) def test_the_serialiser_computes_nothing_every_wire_figure_came_from_the_report() -> None: @@ -1363,7 +1379,11 @@ def test_every_payload_is_json_serialisable_without_a_custom_encoder(builder: st # the table below, because folding it in would mean a default somewhere -- and the whole # reason `curve` is required is that a default serves a journal with no chart. report = _journal_report() - json.dumps(payload.journal_payload(report, curve=build_equity_curve(report.entries))) + json.dumps( + payload.journal_payload( + report, curve=build_equity_curve(report.entries), notes=_empty_notes() + ) + ) return other = { @@ -1822,3 +1842,69 @@ def test_the_complete_set_of_banner_sentences_is_pinned() -> None: "LIVE — every order is previewed and waits for your approval; equity state live", "LIVE — every order is previewed and waits for your approval; equity state not recorded", } + + +# -- the discretionary journal (#705) ------------------------------------------------------------ + + +def _notes(**overrides: object): + from keel.commands.journal import JournalEntry, JournalReport + + fields: dict[str, object] = { + "id": 1, + "ts": 1_000, + "emotion_score": "3", + "rules_followed": True, + "errors_made": "", + "dollar_impact": Decimal("-42.50"), + "chart_note": "", + "screenshot_ref": "", + } + fields.update(overrides) + return JournalReport(now_ts=2_000, entries=(JournalEntry(**fields),)) # type: ignore[arg-type] + + +def test_a_followed_rule_is_never_graded_good() -> None: + """`GOOD` would be the console ENDORSING a claim nothing verified. + + Every other green on this page is a fact -- a fill happened, a chain verified, an attestation + is in date. "I followed my rules" is the operator's opinion of the operator, and a surface + that reflected it back as a pass would be flattering them with their own words. Neutral is the + strongest thing that can honestly be said, and `WARN` on the confession is not the mirror + image of a `GOOD` that does not exist. + """ + entry = payload._discretionary_journal_payload(_notes(rules_followed=True))["entries"][0] + assert entry["rules_followed"]["state"] == "neutral" + + broke = payload._discretionary_journal_payload(_notes(rules_followed=False))["entries"][0] + assert broke["rules_followed"]["state"] == "warn" + + silent = payload._discretionary_journal_payload(_notes(rules_followed=None))["entries"][0] + assert silent["rules_followed"]["state"] == "unknown" + assert silent["rules_followed"]["display"] == "not said" + + +def test_no_state_on_this_section_is_ever_good() -> None: + """The rule above, swept rather than spot-checked: nothing an operator says about themselves + may come back from this codebase as a pass.""" + for rules in (True, False, None): + for impact in (Decimal("10"), Decimal("-10"), None): + body = payload._discretionary_journal_payload( + _notes(rules_followed=rules, dollar_impact=impact) + ) + states = [ + value["state"] + for entry in body["entries"] + for value in entry.values() + if isinstance(value, dict) and "state" in value + ] + assert "good" not in states, f"rules={rules} impact={impact} graded a claim good" + + +def test_a_profitable_self_reported_day_still_warns() -> None: + """`money()` grades a positive figure GOOD by default, which is right for a realised p&l and + wrong here: a self-reported gain is still a claim, and the sign of a number the operator chose + is not evidence about it.""" + body = payload._discretionary_journal_payload(_notes(dollar_impact=Decimal("250"))) + entry = body["entries"][0] + assert entry["dollar_impact"]["state"] == "warn"