fix(pr-cost): make the posted cost readable, not just correct

skills/pr-cost/references/annotate.md

@@ -50,9 +50,23 @@ PR_COST_HOOK_LIVE=1 python3 scripts/pr_cost_collect.py annotate \
   --window-start "$(key window_start)" \
   --window-end "$(key window_end)" \
   --pr-url "$PR_URL" \
-  --notes "Session usage from $READER. USD basis: $(key usd_basis unknown). Token counts retain the reader contract; cached tokens are not added again."
+  --tokens-in-uncached "$(key uncached_input_tokens)" \
+  --tokens-in-cache-read "$(key cache_read_input_tokens)" \
+  --tokens-in-cache-write "$(key cache_creation_input_tokens)" \
+  --usd-basis "$(key usd_basis)" \
+  --notes "Session usage from $READER. Token counts retain the reader contract; cached tokens are not added again."
 ```
 
+The three `--tokens-in-*` flags are what make the posted comment readable:
+without them the comment shows only the merged `tokens_in`, which reads as
+uncached input. The reader already emits the split under its own key names
+(`uncached_input_tokens`, `cache_read_input_tokens`,
+`cache_creation_input_tokens`), so these are a rename, not a recomputation —
+and the collector refuses the payload if the three do not sum to `tokens_in`.
+
+`usd_basis` moves out of the free-text note and into its own field, where the
+comment can label what the dollar figure was priced from.
+
 All three readers emit the same eight shared keys, so `READER`, `READER_FLAG`
 and `HARNESS` are what change between the claude and codex lanes.
 

skills/pr-cost/references/payload.md

@@ -28,6 +28,42 @@ The collector emits one JSON object with this required shape:
 may be `null` when the harness cannot supply them. The keys still remain
 present so downstream adapters receive a stable typed contract.
 
+### Reading `tokens_in`
+
+`tokens_in` is the sum of three token classes that cost three different
+amounts, so the total on its own cannot be multiplied by the input rate.
+These five keys are additive and nullable — a payload written before they
+existed still validates, and `schema_version` stays `pr-cost/v1`:
+
+```json
+{
+  "tokens_in_uncached": 2956,
+  "tokens_in_cache_read": 697885763,
+  "tokens_in_cache_write": 22807403,
+  "usd_basis": "model-rates | default-rates | provider-reported",
+  "scope": "session-total | this-pr"
+}
+```
+
+Why it matters: a posted comment once read `tokens_in 721,979,117` beside
+`usd ~510`, and its reader took that as 722M tokens bought at input rates.
+96.8% of it was cache reads, billed at a tenth of the input rate. The payload
+held no field for the split, so the comment printed the merged number alone.
+
+Two rules the collector enforces:
+
+- When all three parts are present they must sum to `tokens_in`. A split that
+  does not add up is worse than no split — both numbers reach the comment and
+  a reader cannot tell which to believe.
+- `scope` defaults to `session-total` whenever `tokens_in` is present. A
+  session reader sums the whole session, which may cover other PRs and
+  unrelated work; unlabelled, those numbers read as this PR's cost. Pass
+  `--scope this-pr` only when the figures really were scoped to one PR.
+
+`usd_basis` says how the dollar figure was reached. `default-rates` means flat
+lane rates were used, *not* the rates of the model named in the payload — so
+the number can look right while being priced from the wrong table.
+
 ## Harness guidance
 
 - `cursor`: hook payload can detect `gh pr create`, but it does not expose

skills/pr-cost/scripts/pr_cost_collect.py

@@ -18,6 +18,25 @@
 SCHEMA_VERSION = "pr-cost/v1"
 HARNESSES = {"claude", "cursor", "codex", "opencode"}
 CONFIDENCE_LEVELS = {"metered", "estimated", "unavailable"}
+USD_BASES = {"model-rates", "default-rates", "provider-reported"}
+SCOPES = {"session-total", "this-pr"}
+
+# tokens_in is the sum of three token classes that cost three different
+# amounts. A reader who sees only the total reads it as uncached input and
+# multiplies by the input rate. On one real posted comment that produced a
+# large over-reading: tokens_in 721,979,117 next to usd ~510, where 96.8% of
+# the total was cache reads billed at a tenth of the input rate.
+#
+# These keys carry the parts, the basis, and what the window covers, so the
+# total can be read. They are additive and nullable, so a pr-cost/v1 payload
+# written before they existed still validates -- the version does not move.
+OPTIONAL_FIELDS = (
+    "tokens_in_uncached",
+    "tokens_in_cache_read",
+    "tokens_in_cache_write",
+    "usd_basis",
+    "scope",
+)
 DEFAULT_LEDGER = "~/.local/share/pr-cost/ledger.jsonl"
 PR_URL_PATTERN = re.compile(r"https://github\.com/[^/\s]+/[^/\s]+/pull/\d+")
 
@@ -96,6 +115,30 @@ def validate_payload(payload: dict[str, Any]) -> dict[str, Any]:
     if notes is not None and (not isinstance(notes, str) or not notes.strip()):
         raise PrCostError("notes must be a non-empty string or null")
 
+    for field in ("tokens_in_uncached", "tokens_in_cache_read", "tokens_in_cache_write"):
+        if field in payload:
+            validate_nullable_integer(payload, field)
+    usd_basis = payload.get("usd_basis")
+    if usd_basis is not None and usd_basis not in USD_BASES:
+        raise PrCostError("usd_basis must be one of " + ", ".join(sorted(USD_BASES)) + ", or null")
+    scope = payload.get("scope")
+    if scope is not None and scope not in SCOPES:
+        raise PrCostError("scope must be one of " + ", ".join(sorted(SCOPES)) + ", or null")
+
+    # A split that does not add up to the total is worse than no split: both
+    # numbers are then in the comment, and a reader has no way to tell which
+    # one to believe.
+    parts = [
+        payload.get(field)
+        for field in ("tokens_in_uncached", "tokens_in_cache_read", "tokens_in_cache_write")
+    ]
+    if payload.get("tokens_in") is not None and all(part is not None for part in parts):
+        if sum(parts) != payload["tokens_in"]:
+            raise PrCostError(
+                f"tokens_in ({payload['tokens_in']}) must equal uncached + cache read + "
+                f"cache write ({sum(parts)})"
+            )
+
     for field in ("window_start", "window_end", "generated_at"):
         value = payload.get(field)
         if not isinstance(value, str) or not value.strip() or not is_iso_timestamp(value):
@@ -193,6 +236,17 @@ def payload_from_args(
         }
     )
 
+    for field in OPTIONAL_FIELDS:
+        supplied = getattr(args, field, None)
+        payload[field] = supplied if supplied is not None else payload.get(field)
+
+    # A session reader sums a whole session: several PRs plus unrelated work,
+    # if that is what the window held. Saying so by default is the honest
+    # label; a caller who really did scope the numbers to one PR passes
+    # --scope this-pr.
+    if payload.get("scope") is None and payload.get("tokens_in") is not None:
+        payload["scope"] = "session-total"
+
     if args.notes is not None:
         payload["notes"] = args.notes
     elif "notes" not in payload:
@@ -251,11 +305,73 @@ def append_ledger(
     return "corrected" if duplicate else "annotated"
 
 
+USD_BASIS_TEXT = {
+    "model-rates": "the published rates of the model named above",
+    "default-rates": "fixed lane rates, NOT the rates of the model named above",
+    "provider-reported": "the cost the provider recorded, copied not recomputed",
+}
+
+SCOPE_TEXT = {
+    "session-total": "whole session, which may cover other PRs and unrelated work",
+    "this-pr": "scoped to this PR",
+}
+
+
+def human_summary(payload: dict[str, Any]) -> list[str]:
+    """The lines a reader sees before the JSON block.
+
+    The JSON alone put `tokens_in` next to `usd` with nothing between them,
+    and a reader who knows the input rate multiplies one by the other. These
+    lines say which parts of tokens_in cost what, on what basis the USD was
+    priced, and what work the window covers.
+    """
+    lines: list[str] = []
+
+    usd = payload.get("usd")
+    headline = f"~${usd:,.2f}" if isinstance(usd, (int, float)) else "cost unavailable"
+    model = payload.get("model") or "model unknown"
+    confidence = payload.get("confidence") or "unknown"
+    lines.append(f"**{headline}** — {model} — confidence: {confidence}")
+
+    basis = payload.get("usd_basis")
+    if basis:
+        lines.append(f"- Priced from: {basis} ({USD_BASIS_TEXT.get(basis, 'see the skill docs')})")
+
+    tokens_in = payload.get("tokens_in")
+    uncached = payload.get("tokens_in_uncached")
+    cache_read = payload.get("tokens_in_cache_read")
+    cache_write = payload.get("tokens_in_cache_write")
+    if tokens_in is not None and None not in (uncached, cache_read, cache_write):
+        share = f" ({cache_read / tokens_in:.1%} of input)" if tokens_in else ""
+        lines.append(
+            f"- Input {tokens_in:,} = {uncached:,} uncached + {cache_read:,} cache read"
+            f"{share} + {cache_write:,} cache write. Cache reads bill at a fraction "
+            "of the input rate, so this total is not input-priced."
+        )
+    elif tokens_in is not None:
+        lines.append(
+            f"- Input {tokens_in:,} tokens, cached and uncached combined; "
+            "this lane did not report the split."
+        )
+
+    tokens_out = payload.get("tokens_out")
+    if tokens_out is not None:
+        lines.append(f"- Output {tokens_out:,} tokens")
+
+    scope = payload.get("scope")
+    if scope:
+        lines.append(f"- Covers: {SCOPE_TEXT.get(scope, scope)}")
+
+    return lines
+
+
 def comment_body(payload: dict[str, Any]) -> str:
     session_marker = payload.get("session_id") or "unknown"
+    summary = "\n".join(human_summary(payload))
     return (
         f"<!-- pr-cost:{session_marker} -->\n"
         "AI cost payload for the session that created this PR:\n\n"
+        f"{summary}\n\n"
         "```json\n"
         f"{json.dumps(payload, indent=2, sort_keys=True)}\n"
         "```"
@@ -412,6 +528,11 @@ def add_payload_arguments(parser: argparse.ArgumentParser) -> None:
     parser.add_argument("--pr-url")
     parser.add_argument("--generated-at")
     parser.add_argument("--notes")
+    parser.add_argument("--tokens-in-uncached", type=int, dest="tokens_in_uncached")
+    parser.add_argument("--tokens-in-cache-read", type=int, dest="tokens_in_cache_read")
+    parser.add_argument("--tokens-in-cache-write", type=int, dest="tokens_in_cache_write")
+    parser.add_argument("--usd-basis", dest="usd_basis", choices=sorted(USD_BASES))
+    parser.add_argument("--scope", dest="scope", choices=sorted(SCOPES))
 
 
 def build_parser() -> argparse.ArgumentParser:

skills/pr-cost/tests/test_annotation_recipe.py

@@ -9,23 +9,49 @@
 SKILL = Path(__file__).resolve().parents[1]
 
 
-def render_notes(document, basis):
+def render_flag(document, flag, usage):
+    """Render one flag value from the documented recipe through its own key()."""
     helper = re.search(r'key\(\) \{.*?\n\}', document, re.S).group(0)
-    note = re.search(r'^  --notes (.*)$', document, re.M).group(1)
-    env = dict(os.environ, USAGE=json.dumps({'usd_basis': basis}), READER='synthetic-reader')
+    value = re.search(r'^  --' + re.escape(flag) + r' (.*?) *\\?$', document, re.M).group(1)
+    env = dict(os.environ, USAGE=json.dumps(usage), READER='synthetic-reader')
     return subprocess.check_output(
-        ['bash', '-c', helper + '\nprintf "%s" ' + note], env=env, text=True
+        ['bash', '-c', helper + '\nprintf "%s" ' + value], env=env, text=True
     )
 
 
 class AnnotationRecipeTest(unittest.TestCase):
-    def test_notes_follow_reader_basis(self):
+    def test_recipe_passes_the_reader_basis_as_a_typed_field(self):
+        # The basis used to ride along inside --notes, which is free text a
+        # comment cannot label. It now has its own field, so the property this
+        # file guards -- the reader's basis survives into the annotation --
+        # is checked on that field instead.
         document = (SKILL / 'references/annotate.md').read_text()
         for basis in ('model-rates', 'default-rates', 'provider-reported'):
             with self.subTest(basis=basis):
-                notes = render_notes(document, basis)
-                self.assertIn('USD basis: ' + basis, notes)
-                self.assertIn('cached tokens are not added again', notes)
+                rendered = render_flag(document, 'usd-basis', {'usd_basis': basis})
+                self.assertEqual(rendered, basis)
+
+    def test_recipe_passes_the_token_split_from_the_reader(self):
+        # Without these three the comment shows only the merged tokens_in,
+        # which is what was misread as uncached input.
+        document = (SKILL / 'references/annotate.md').read_text()
+        usage = {
+            'uncached_input_tokens': 2956,
+            'cache_read_input_tokens': 697885763,
+            'cache_creation_input_tokens': 22807403,
+        }
+        for flag, key in (
+            ('tokens-in-uncached', 'uncached_input_tokens'),
+            ('tokens-in-cache-read', 'cache_read_input_tokens'),
+            ('tokens-in-cache-write', 'cache_creation_input_tokens'),
+        ):
+            with self.subTest(flag=flag):
+                self.assertEqual(render_flag(document, flag, usage), str(usage[key]))
+
+    def test_notes_keep_the_token_contract_caveat(self):
+        document = (SKILL / 'references/annotate.md').read_text()
+        notes = render_flag(document, 'notes', {'usd_basis': 'model-rates'})
+        self.assertIn('cached tokens are not added again', notes)
 
 
 if __name__ == '__main__':

skills/pr-cost/tests/test_pr_cost_collect.py

@@ -3,6 +3,7 @@
 
 from __future__ import annotations
 
+import importlib.util
 import json
 import os
 import pathlib
@@ -17,6 +18,11 @@
 SCRIPT = SKILL_DIR / "scripts" / "pr_cost_collect.py"
 FIXTURES = SKILL_DIR / "tests" / "fixtures"
 
+SPEC = importlib.util.spec_from_file_location("pr_cost_collect", SCRIPT)
+collector = importlib.util.module_from_spec(SPEC)
+assert SPEC.loader is not None
+SPEC.loader.exec_module(collector)
+
 
 class PrCostCollectTest(unittest.TestCase):
     def setUp(self) -> None:
@@ -160,6 +166,86 @@ def test_from_hook_has_no_allow_duplicate_escape(self) -> None:
         )
         self.assertIn("--allow-duplicate", result.stderr)
 
+    # The posted comment has to be readable, not just correct.
+
+    def test_comment_body_shows_the_split_the_basis_and_the_scope(self) -> None:
+        # The reported defect: the JSON put tokens_in next to usd with nothing
+        # between them, so a reader multiplied one by the input rate. The
+        # figures here are the session that produced that comment.
+        body = collector.comment_body(
+            {
+                "schema_version": "pr-cost/v1",
+                "harness": "claude",
+                "confidence": "estimated",
+                "usd": 602.99,
+                "tokens_in": 720_696_122,
+                "tokens_in_uncached": 2_956,
+                "tokens_in_cache_read": 697_885_763,
+                "tokens_in_cache_write": 22_807_403,
+                "tokens_out": 1_038_408,
+                "usd_basis": "model-rates",
+                "scope": "session-total",
+                "model": "claude-opus-5",
+                "session_id": "s",
+                "window_start": "2026-01-01T00:00:00+00:00",
+                "window_end": "2026-01-01T00:01:00+00:00",
+                "pr_url": None,
+                "generated_at": "2026-01-01T00:01:00+00:00",
+            }
+        )
+        # Assert on the prose above the JSON block: the JSON always held these
+        # numbers, so matching the whole body would pass on the old comment.
+        heading = body.split("```json")[0]
+        self.assertIn("697,885,763 cache read", heading)
+        self.assertIn("96.8% of input", heading)
+        self.assertIn("model-rates", heading)
+        self.assertIn("may cover other PRs", heading)
+
+    def test_a_split_that_does_not_sum_to_tokens_in_is_refused(self) -> None:
+        # A split that does not add up is worse than no split: both numbers
+        # are then in the comment and a reader cannot tell which to believe.
+        payload = {
+            "schema_version": "pr-cost/v1",
+            "harness": "claude",
+            "confidence": "estimated",
+            "usd": 1.0,
+            "tokens_in": 1_000,
+            "tokens_in_uncached": 100,
+            "tokens_in_cache_read": 100,
+            "tokens_in_cache_write": 100,
+            "tokens_out": 10,
+            "model": "claude-opus-5",
+            "session_id": "s",
+            "window_start": "2026-01-01T00:00:00+00:00",
+            "window_end": "2026-01-01T00:01:00+00:00",
+            "pr_url": None,
+            "generated_at": "2026-01-01T00:01:00+00:00",
+        }
+        with self.assertRaises(collector.PrCostError):
+            collector.validate_payload(payload)
+
+    def test_scope_defaults_to_session_total_when_tokens_are_present(self) -> None:
+        # A session reader sums the whole session, which may cover several PRs
+        # and unrelated work. Unlabelled, those numbers read as this PR's cost.
+        result = self.run_cli(
+            "emit",
+            "--harness",
+            "claude",
+            "--confidence",
+            "estimated",
+            "--usd",
+            "1.0",
+            "--tokens-in",
+            "100",
+            "--tokens-out",
+            "10",
+            "--model",
+            "claude-opus-5",
+            "--session-id",
+            "s",
+        )
+        self.assertEqual(json.loads(result.stdout)["scope"], "session-total")
+
 
 if __name__ == "__main__":
     unittest.main()