From 6c3523aa2f60d9003fe2d2b4d111ebbee46bba56 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sat, 5 Sep 2026 17:05:32 -0400 Subject: [PATCH 1/2] =?UTF-8?q?feat(engine+web):=20wire=20the=20discretion?= =?UTF-8?q?ary=20journal=20=E2=80=94=20CLI-only=20in,=20self-reported=20ev?= =?UTF-8?q?erywhere=20out=20(#705)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `journal` table has been in the schema from the beginning with no repository method and no caller anywhere in the code. Dead schema is worse than no schema: a reader assumes a declared table is a used one. THE ONE RECORD HERE THAT NOTHING CAN CHECK Everything else keel keeps is 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. "I felt rushed and broke my own rule" has no external referent at all. That is not a defect: self-assessment is the only way this information exists, and 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. So `self-reported` is a SIXTH provenance in the timeline's closed vocabulary, not a reuse of `human-attested`. 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 implying otherwise. It carries WARN for the same reason `simulated` does: both mark a row that reads like evidence and is not. THE CLI IS THE ONLY WAY IN, AND `add` TAKES NO OPTIONS Attestations are human-sourced or refused. `keel journal add` prompts and requires a terminal, and it accepts no value options at all -- not merely "prompts by default". A `--emotion 3` would make the entry scriptable and leave the TTY gate guarding a ceremony that no longer needed a human to supply anything. A test asserts `keel/web/api.py` never reaches the writer. `_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 that a confirmation prompt does nothing about. What is load-bearing is the terminal: off a TTY there is no human, so cron, a pipe and a script are all refused. And there is no edit. No update method, pinned by name, because the natural thing for a later contributor to add beside two methods is a third that updates -- and a journal you can go back and change records what you wish you had thought. BLANK IS "DID NOT SAY", AND `rules_followed` IS THREE-VALUED Every field but the timestamp may be skipped, including all of them: an operator recording one sentence about one day must not have to invent an emotion score to do it. A blank answer reaches the database as NULL, never a zero. `rules_followed=False` is the operator confessing they broke their own rules -- the most consequential thing this table can hold -- and `bool(None)` would print that confession over a question they skipped. Three values, three words, on every surface. An emotion score off the 1-5 scale and a dollar impact that is not a number are REFUSED rather than kept. A journal whose emotion column holds "9", "very bad" and "3" has three vocabularies and no series; one that read "lots" as zero would say the day cost nothing. NOTHING AN OPERATOR SAYS ABOUT THEMSELVES COMES BACK GREEN Every other green on the Insights 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 reflecting it back as a pass would be flattering them with their own words. NEUTRAL is the strongest thing that can honestly be said, and a swept test asserts no state in this section is ever `good` -- including a self-reported PROFIT, where `money()`'s default would have graded the sign of a number the operator chose. The entries are not the route's sortable collection, and that is a refusal rather than 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, in the one place where the thing being ranked is a person. ON THE SAME PAGE AS THE OTHER JOURNAL, DELIBERATELY `keel insights journal` already exists and is a filterable view of closed TRADES. Two things called a journal, and separate pages would let a reader meet one without learning the other exists -- so the discretionary entries ride `/api/journal` at `data.notes`, below the closed trades, with the marker on the heading and on every impact figure. Both CLI surfaces say which they are. Chained into `audit_events` (#721) like everything else a human swore to, so it rides the audit export with a real `row_hash` beside the orders and fills. Twelve mutants killed, including: a blank emotion stored as zero, a skipped rules question becoming a confession, an out-of-scale score kept as free text, an unparseable impact becoming zero, `journal add` no longer needing a terminal, the row filed as human-attested, `bool(None)` confessing, the entry going unchained, the limit keeping the oldest entries, the window closing at both ends, a self-reported impact that stops warning, and a followed rule graded good. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KZZxmspQXe5qJ9FAsG13s6 --- keel/cli.py | 13 ++ keel/commands/journal.py | 316 ++++++++++++++++++++++++++++++++ keel/commands/timeline.py | 75 +++++++- keel/data/audit.py | 6 + keel/data/repository.py | 125 +++++++++++++ keel/web/api.py | 16 +- keel/web/payload.py | 104 ++++++++++- keel/web/static/js/render.js | 62 +++++++ tests/commands/test_journal.py | 212 +++++++++++++++++++++ tests/commands/test_timeline.py | 86 +++++++++ tests/data/test_journal.py | 170 +++++++++++++++++ tests/web/test_client_assets.py | 49 ++++- tests/web/test_payload.py | 92 +++++++++- 13 files changed, 1315 insertions(+), 11 deletions(-) create mode 100644 keel/commands/journal.py create mode 100644 tests/commands/test_journal.py create mode 100644 tests/data/test_journal.py 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..824bf17 --- /dev/null +++ b/keel/commands/journal.py @@ -0,0 +1,316 @@ +"""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 `keel/web/api.py` never +reaches the writer. + +── 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 + + +@dataclass(frozen=True) +class JournalReport: + now_ts: int + entries: tuple[JournalEntry, ...] + + @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. The renderers say two different things + over an empty table and a missing one, and only this can tell them apart.""" + return bool(self.entries) + + +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 capped page changes how + much of the journal a reader sees and never which way it reads. + """ + 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) + + +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("") + 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=int, + 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..513b8f3 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,66 @@ 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 leads with whichever sentence the operator actually wrote. 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") + note = str(raw.get("chart_note") or "").strip() + errors = str(raw.get("errors_made") or "").strip() + if errors: + parts.append(errors) + elif note: + parts.append(note) + 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 +558,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..dfb5284 100644 --- a/keel/data/repository.py +++ b/keel/data/repository.py @@ -226,6 +226,131 @@ 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. + """ + query = "SELECT * FROM journal WHERE 1=1" + params: list[Any] = [] + if since_ts is not None: + query += " AND ts >= ?" + params.append(since_ts) + if until_ts is not None: + query += " AND ts < ?" + params.append(until_ts) + if limit is None: + rows = self._conn.execute(query + " ORDER BY ts, id", params).fetchall() + else: + # Newest `limit` in the database, then re-read forwards in Python. Ordering DESC in + # SQL and reversing here keeps the cap on the right end without a subquery. + newest = self._conn.execute( + query + " ORDER BY ts DESC, id DESC LIMIT ?", [*params, limit] + ).fetchall() + rows = list(reversed(newest)) + return [self._journal_row_to_dict(row) for row in rows] + + 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..e05f01f 100644 --- a/keel/web/api.py +++ b/keel/web/api.py @@ -710,14 +710,28 @@ 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 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. + notes = gather_journal(repo, now_ts=now_ts, limit=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..eaa9dfc 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,98 @@ 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), + } + + +#: The word that must appear beside every self-reported figure on this page. Read from the command +#: module rather than restated, so the CLI and the console cannot come to use two different words +#: for the one record neither of them can verify. +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. + """ + 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, + ), + "shown_count": count(notes.entry_count), + "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 _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 +1876,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 +1892,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..d9ef3bd 100644 --- a/keel/web/static/js/render.js +++ b/keel/web/static/js/render.js @@ -2538,6 +2538,68 @@ 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)); + 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..3253fa2 --- /dev/null +++ b/tests/commands/test_journal.py @@ -0,0 +1,212 @@ +"""`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(deployment) -> 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.""" + result = _run(deployment, ["journal", "add", "--help"]) + for smuggled in ("--emotion", "--rules-followed", "--errors", "--impact", "--note", "--json"): + assert smuggled not in result.output, f"`journal add` accepts {smuggled}" + + +def test_nothing_outside_the_cli_can_reach_the_writer() -> None: + """No web write path, by construction. `keel serve` has one write surface (`/api/setup`), and + the journal is not on it -- pinned here rather than trusted, because the day someone adds a + "quick note" box to the console is the day this record stops being an attestation.""" + from keel.web import api + + source = (api.__file__ or "").replace(".pyc", ".py") + text = open(source, encoding="utf-8").read() + assert "append_journal_entry" not in text + + +# -- 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..fc143c0 100644 --- a/tests/commands/test_timeline.py +++ b/tests/commands/test_timeline.py @@ -626,3 +626,89 @@ 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 diff --git a/tests/data/test_journal.py b/tests/data/test_journal.py new file mode 100644 index 0000000..ccd84cb --- /dev/null +++ b/tests/data/test_journal.py @@ -0,0 +1,170 @@ +"""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}" diff --git a/tests/web/test_client_assets.py b/tests/web/test_client_assets.py index f45e5dc..9e819b1 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 = { 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" From 6fa98de66e1fe6aad7756d52b55a550d32883cae Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sat, 5 Sep 2026 18:12:33 -0400 Subject: [PATCH 2/2] =?UTF-8?q?fix(engine+web):=20review=20findings=20on?= =?UTF-8?q?=20#705=20=E2=80=94=20the=20trades'=20page=20control=20was=20tr?= =?UTF-8?q?uncating=20the=20journal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An independent review found eleven issues. Every one of them sat on a seam the mutation battery had not reached: the client, and the limit. ONE RECORD'S PAGE CONTROL WAS TRUNCATING A DIFFERENT RECORD `read_journal` passed `?limit=` to both journals, so narrowing to one closed trade silently hid 300 of an operator's 301 notes -- and the page said nothing, because the payload shipped `shown_count` with no total and `render.js` never read even that. Measured: 60 entries, `?limit=1`, one note served. The rule is stated twice in the neighbouring docstrings this change should have read. `journal_payload` already says a client showing "50 of 812" needs both counts; `get_equity_points` says a caller that bounds a read is showing a WINDOW and must say so, and names `count_equity_points` as how it learns what it left out. There was no `count_journal_entries`, so the payload could not have said it. The journal now has its OWN cap -- the coupling was a coincidence of two records sharing a route -- and carries `total_count` beside `shown_count` plus the sentence composed from them. Mutating the wiring back survived the whole 6,000- test suite before this. THE WHOLE SECTION COULD BE DELETED FROM THE CONSOLE WITH EVERY GATE GREEN Two independent deletions survived the full suite: the call to `notesSection`, and the SELF-REPORTED pill. The acceptance test claimed to cover "the console that renders it" and in fact called the payload function directly, and the parity scan cannot help -- an uncalled function still reads the keys it reads. That is the third time this series a client-side pin has needed a mutant to show it was pinning nothing. `--limit 0` PRINTED "No journal entries yet" OVER A JOURNAL And `--limit -1` printed everything, because SQLite reads a negative LIMIT as unbounded. `keel/web/api.py::_journal_limit` names this exact hazard for the other journal on the same route. Refused in `gather_journal` rather than only at the click option, so the guard travels with the function instead of one caller. THE "NO WEB WRITE PATH" TEST GUARDED A FILE WHERE A WRITE COULD NOT LIVE It grepped `keel/web/api.py`, whose own docstring says "Reads only. Not one route below answers a POST". The write surface is `keel.commands.setup.ACTIONS`, the only thing `server.do_POST` routes to -- and it ALREADY carries a human-attestation writer (`attest_asset`), which is precisely why a "quick note" box is the plausible next addition. Adding a journal writer to `setup.py` passed the old test untouched. THE EXPORT DROPPED EVERY SENTENCE AFTER THE FIRST `errors_made` and `chart_note` were an if/elif, so an entry carrying both exported only the error, and `screenshot_ref` reached the file in no column and no summary. This row is the journal's WHOLE representation in a CSV an operator hands to an auditor -- there is nowhere else any of it could reappear. AND THE READ INVENTED A THIRD SPELLING OF AN IDIOM TWO SIBLINGS SHARE `get_equity_points` and `get_cycle_balances` both cap with the same re-ordered subquery, and one names the other as canon. This used a DESC read reversed in Python "without a subquery" -- behaviourally equivalent for a positive limit, and exactly where the negative-limit bug came from. Also: the blocklist test for `journal add`'s options is now `journal_add.params == []` (a `--feeling 3` walked past six literal names); the append-only pin sweeps the package for `UPDATE journal`/`DELETE FROM journal` rather than three method names; `_optional_text`'s empty-string rule is tested; a stray `#:` attribute comment above a `def` is gone; and `_function_body` now finds module-private functions, which is why `notesSection` could be scanned at all. TWO THINGS NAMED RATHER THAN CLAIMED The module docstring said the TTY gate means "nothing can reach past it". A `pty.fork()` driver runnable from cron allocates a real terminal and this command answers it. True of every gate built on the same predicate; the honest statement is that the check makes automated entry something someone has to MEAN. And `test_the_limit_breaks_a_timestamp_tie_by_id` pins the outcome, not the `id DESC` clause: measured on this SQLite build, dropping it returns the same rows. The clause stays because the sibling states why, but no test here can make its absence fail, and the test says so. Thirteen mutants killed of fourteen tried; the fourteenth is the tie-break above, recorded in the test as a named blind spot rather than left implied. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KZZxmspQXe5qJ9FAsG13s6 --- keel/commands/journal.py | 70 ++++++++++++++++++---- keel/commands/timeline.py | 21 ++++--- keel/data/repository.py | 54 +++++++++++++---- keel/web/api.py | 10 +++- keel/web/payload.py | 31 +++++++++- keel/web/static/js/render.js | 6 +- tests/commands/test_journal.py | 40 +++++++++---- tests/commands/test_timeline.py | 30 ++++++++++ tests/data/test_journal.py | 101 ++++++++++++++++++++++++++++++++ tests/web/test_api.py | 27 +++++++++ tests/web/test_client_assets.py | 58 ++++++++++++++++-- 11 files changed, 395 insertions(+), 53 deletions(-) diff --git a/keel/commands/journal.py b/keel/commands/journal.py index 824bf17..cd7aa40 100644 --- a/keel/commands/journal.py +++ b/keel/commands/journal.py @@ -23,8 +23,17 @@ 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 `keel/web/api.py` never -reaches the writer. +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 ───────────────────────────────────────────────────────────────────────── @@ -84,10 +93,26 @@ class JournalEntry: 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: @@ -97,17 +122,37 @@ def entry_count(self) -> int: @property def any_recorded(self) -> bool: - """Whether this deployment has a journal at all. The renderers say two different things - over an empty table and a missing one, and only this can tell them apart.""" - return bool(self.entries) + """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 capped page changes how - much of the journal a reader sees and never which way it reads. + `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"]), @@ -121,7 +166,9 @@ def gather_journal(repo: Repository, *, now_ts: int, limit: int | None = None) - ) for row in repo.get_journal_entries(limit=limit) ) - return JournalReport(now_ts=now_ts, entries=entries) + return JournalReport( + now_ts=now_ts, entries=entries, total_count=repo.count_journal_entries() + ) def _optional_text(value: object) -> str | None: @@ -216,7 +263,10 @@ def render_human(report: JournalReport) -> str: lines.append(f" chart note : {entry.chart_note or 'not said'}") lines.append(f" screenshot : {entry.screenshot_ref or 'not said'}") lines.append("") - lines.append(f"{report.entry_count} entr{'y' if report.entry_count == 1 else 'ies'}.") + 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) @@ -303,7 +353,7 @@ def journal_add(ctx: click.Context) -> None: @journal_group.command("list") @click.option( "--limit", - type=int, + type=click.IntRange(min=1), default=None, help="Show only the most recent N entries. They still read forwards.", ) diff --git a/keel/commands/timeline.py b/keel/commands/timeline.py index 513b8f3..4234adb 100644 --- a/keel/commands/timeline.py +++ b/keel/commands/timeline.py @@ -423,9 +423,10 @@ def _journal_rows( 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 leads with whichever sentence the operator actually wrote. 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. + 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): @@ -461,12 +462,14 @@ def _journal_summary(raw: dict[str, Any]) -> str: parts: list[str] = [] if raw.get("rules_followed") is False: parts.append("BROKE RULES") - note = str(raw.get("chart_note") or "").strip() - errors = str(raw.get("errors_made") or "").strip() - if errors: - parts.append(errors) - elif note: - parts.append(note) + # 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}") diff --git a/keel/data/repository.py b/keel/data/repository.py index dfb5284..c01e745 100644 --- a/keel/data/repository.py +++ b/keel/data/repository.py @@ -322,24 +322,52 @@ def get_journal_entries( rather than which way they read. `id` breaks a timestamp tie, so two notes written in one second keep a stable order across reads. """ - query = "SELECT * FROM journal WHERE 1=1" + 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: - query += " AND ts >= ?" + clauses.append("ts >= ?") params.append(since_ts) if until_ts is not None: - query += " AND ts < ?" + clauses.append("ts < ?") params.append(until_ts) - if limit is None: - rows = self._conn.execute(query + " ORDER BY ts, id", params).fetchall() - else: - # Newest `limit` in the database, then re-read forwards in Python. Ordering DESC in - # SQL and reversing here keeps the cap on the right end without a subquery. - newest = self._conn.execute( - query + " ORDER BY ts DESC, id DESC LIMIT ?", [*params, limit] - ).fetchall() - rows = list(reversed(newest)) - return [self._journal_row_to_dict(row) for row in rows] + return ((" WHERE " + " AND ".join(clauses)) if clauses else "", params) def _journal_row_to_dict(self, row: sqlite3.Row) -> dict[str, Any]: entry = dict(row) diff --git a/keel/web/api.py b/keel/web/api.py index e05f01f..cd14abe 100644 --- a/keel/web/api.py +++ b/keel/web/api.py @@ -710,7 +710,7 @@ 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 gather_journal + from keel.commands.journal import DEFAULT_NOTES_LIMIT, gather_journal limit = _journal_limit(query) repo = open_repo(cfg.db_path) @@ -726,7 +726,13 @@ def read_journal(cfg: ServeConfig, query: Query, _state: Any, now_ts: int) -> di # 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. - notes = gather_journal(repo, now_ts=now_ts, limit=limit) + # + # 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( diff --git a/keel/web/payload.py b/keel/web/payload.py index eaa9dfc..c82eec7 100644 --- a/keel/web/payload.py +++ b/keel/web/payload.py @@ -1258,9 +1258,6 @@ def journal_payload( } -#: The word that must appear beside every self-reported figure on this page. Read from the command -#: module rather than restated, so the CLI and the console cannot come to use two different words -#: for the one record neither of them can verify. def _discretionary_journal_payload(notes: DiscretionaryJournal) -> dict[str, Any]: """The operator's own account of their own conduct (#705). @@ -1279,6 +1276,8 @@ def _discretionary_journal_payload(notes: DiscretionaryJournal) -> dict[str, Any 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 { @@ -1290,7 +1289,19 @@ def _discretionary_journal_payload(notes: DiscretionaryJournal) -> dict[str, Any 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), @@ -1326,6 +1337,20 @@ def _discretionary_journal_payload(notes: DiscretionaryJournal) -> dict[str, Any } +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: diff --git a/keel/web/static/js/render.js b/keel/web/static/js/render.js index d9ef3bd..2efc78a 100644 --- a/keel/web/static/js/render.js +++ b/keel/web/static/js/render.js @@ -2571,7 +2571,11 @@ function notesSection(notes) { 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)); + 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( diff --git a/tests/commands/test_journal.py b/tests/commands/test_journal.py index 3253fa2..6181bf4 100644 --- a/tests/commands/test_journal.py +++ b/tests/commands/test_journal.py @@ -56,23 +56,41 @@ def test_adding_an_entry_off_a_terminal_is_refused(deployment, monkeypatch) -> N assert _entries(deployment) == [] -def test_the_add_command_takes_no_value_options_at_all(deployment) -> None: +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.""" - result = _run(deployment, ["journal", "add", "--help"]) - for smuggled in ("--emotion", "--rules-followed", "--errors", "--impact", "--note", "--json"): - assert smuggled not in result.output, f"`journal add` accepts {smuggled}" + 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. `keel serve` has one write surface (`/api/setup`), and - the journal is not on it -- pinned here rather than trusted, because the day someone adds a - "quick note" box to the console is the day this record stops being an attestation.""" + """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 - source = (api.__file__ or "").replace(".pyc", ".py") - text = open(source, encoding="utf-8").read() - assert "append_journal_entry" not in text + 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 ---------------------------------------------------------------------------- diff --git a/tests/commands/test_timeline.py b/tests/commands/test_timeline.py index fc143c0..a5335fd 100644 --- a/tests/commands/test_timeline.py +++ b/tests/commands/test_timeline.py @@ -712,3 +712,33 @@ def test_the_journal_respects_the_scope_window(repo: Repository) -> None: 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 index ccd84cb..972a8a6 100644 --- a/tests/data/test_journal.py +++ b/tests/data/test_journal.py @@ -168,3 +168,104 @@ def test_there_is_no_way_to_edit_or_delete_an_entry(repo: Repository) -> None: 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 9e819b1..e71838e 100644 --- a/tests/web/test_client_assets.py +++ b/tests/web/test_client_assets.py @@ -1989,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: @@ -2125,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