fix(cost): serialize ledger records with json.dumps so ids can't corrupt JSON (#165)

docs/JOURNAL.md

@@ -18,6 +18,30 @@ protocol. **Newest entries at the top.** Tag each entry with one or more of:
 
 ---
 
+## 2026-07-23 — hand-built cost-ledger JSON silently under-reported API spend  #mistake #fix #gotcha
+**Context:** issue #165 — `_ledger_append` in `src/trinity/llm/openai_compatible_pool.py` writes one
+token-usage record per call to the JSONL cost ledger (`TRINITY_COST_LEDGER`).
+**Expected:** every recorded call is counted by `costing.ledger_cost_report`.
+**Actual:** the record was hand-formatted with an f-string interpolating `provider` and `model`
+straight into a JSON literal:
+```python
+f'{{"provider":"{provider}","m":"{model}","p":{int(prompt_tokens)},"c":{int(completion_tokens)}}}\n'
+```
+so any id containing a `"`, a backslash, or a control character emitted a **structurally invalid**
+line. The reader skips unparseable lines rather than raising, so the failure was silent: affected
+calls simply vanished from the spend total.
+**Root cause:** string interpolation into JSON with no escaping — the classic injection shape, here
+producing corruption rather than a security issue. Provider/model ids come from config and remote
+API responses, so they are not guaranteed to be JSON-safe.
+**Fix / decision:** build a `dict` and serialize with `json.dumps(..., ensure_ascii=False)`, which
+escapes quotes, backslashes, and control characters correctly by construction. `ensure_ascii=False`
+keeps non-ASCII model names readable rather than `\uXXXX`-escaped; the output is still valid JSON
+since the file is read back as UTF-8. The `except Exception: pass` guard is retained — ledger
+writes must never take down a run. Added `tests/test_cost_ledger_json.py` (14 cases, offline):
+adversarial ids (embedded quote, backslash, newline, unicode) round-trip through `json.loads`, and
+the totals `ledger_cost_report` computes are unchanged for ordinary ids.
+**Follow-up:** none. Worth a grep for other hand-built JSON writers if any remain.
+
 ## 2026-07-12 — code grader: add resource limits on top of the HOME/secrets fix  #security #decision
 **Context:** issue #71 — the code grader (`run_pass_at_1`) runs untrusted miner/LLM candidate code. The core secret-leak fix (isolated throwaway HOME/cwd, scrubbed env, `python -I`) already landed on main.
 **Finding:** main's sandbox closes the HOME/secrets exfiltration vector but has **no resource limits** — an untrusted candidate can still exhaust host memory or fork-bomb the eval box within its wall-clock timeout (verified: a 4 GiB `bytearray` allocation runs to completion and "passes" on main).

src/trinity/llm/openai_compatible_pool.py

@@ -3,6 +3,7 @@
 
 import argparse
 import asyncio
+import json
 import os
 import sys
 import time
@@ -69,16 +70,25 @@ def _ledger_append(
     prompt_tokens: int,
     completion_tokens: int,
 ) -> None:
-    """Append one token-usage record to the cost ledger, if TRINITY_COST_LEDGER is set."""
+    """Append one token-usage record to the cost ledger, if TRINITY_COST_LEDGER is set.
+
+    The record is serialized with :func:`json.dumps` rather than hand-built, so a
+    provider/model id containing a quote, a backslash, or a control character
+    still produces valid JSON. Hand-formatting silently emitted unparseable lines
+    that ``costing.ledger_cost_report`` then skipped, under-reporting API spend.
+    """
     path = os.environ.get("TRINITY_COST_LEDGER")
     if not path:
         return
     try:
+        record = {
+            "provider": provider,
+            "m": model,
+            "p": int(prompt_tokens),
+            "c": int(completion_tokens),
+        }
         with open(path, "a") as f:
-            f.write(
-                f'{{"provider":"{provider}","m":"{model}","p":{int(prompt_tokens)},'
-                f'"c":{int(completion_tokens)}}}\n'
-            )
+            f.write(json.dumps(record, ensure_ascii=False) + "\n")
     except Exception:
         pass
 

tests/test_cost_ledger_json.py

@@ -0,0 +1,97 @@
+"""Offline tests for cost-ledger JSON escaping (#165).
+
+`_ledger_append` hand-built each ledger line with an f-string, so a provider or
+model id containing a quote, a backslash, or a control character emitted a line
+that is not valid JSON. `costing.ledger_cost_report` skips unparseable lines
+silently, so those calls vanished from `cost_usd` / `cost_calls` / the per-model
+breakdown — silent under-reporting of API spend, with no error and no warning.
+
+These tests pin that every such id round-trips as valid JSON and is counted.
+Pure stdlib: no network / GPU / torch.
+"""
+from __future__ import annotations
+
+import json
+
+import pytest
+
+from trinity.costing import ledger_cost_report
+from trinity.llm.openai_compatible_pool import _ledger_append
+
+# Model ids that broke the hand-built writer, plus benign controls.
+_TRICKY_IDS = [
+    'weird"model',       # double quote  -> unescaped, truncated the JSON string
+    "back\\slash",       # backslash     -> invalid JSON escape
+    "ctrl\tchar",        # control char  -> raw tab is invalid inside a JSON string
+    "new\nline",         # newline       -> would also split the JSONL record
+    "uniécode/model",
+    "normal-model",
+]
+
+
+@pytest.fixture()
+def ledger(tmp_path, monkeypatch):
+    path = tmp_path / "ledger.jsonl"
+    monkeypatch.setenv("TRINITY_COST_LEDGER", str(path))
+    return path
+
+
+@pytest.mark.parametrize("model", _TRICKY_IDS)
+def test_each_record_is_valid_json(ledger, model):
+    _ledger_append("openrouter", model, 11, 22)
+    lines = [ln for ln in ledger.read_text().splitlines() if ln.strip()]
+    assert len(lines) == 1
+    row = json.loads(lines[0])  # would raise before the fix
+    assert row["m"] == model
+    assert row["provider"] == "openrouter"
+    assert row["p"] == 11
+    assert row["c"] == 22
+
+
+@pytest.mark.parametrize("provider", ['prov"ider', "prov\\ider", "openrouter"])
+def test_provider_is_escaped_too(ledger, provider):
+    _ledger_append(provider, "some-model", 1, 2)
+    row = json.loads(ledger.read_text().strip())
+    assert row["provider"] == provider
+
+
+def test_no_call_is_dropped_from_the_cost_report(ledger):
+    for i, model in enumerate(_TRICKY_IDS):
+        _ledger_append("openrouter", model, 100 + i, 200 + i)
+
+    report = ledger_cost_report(ledger)
+
+    assert report["cost_calls"] == len(_TRICKY_IDS)
+    assert report["cost_prompt_tokens"] == sum(100 + i for i in range(len(_TRICKY_IDS)))
+    assert report["cost_completion_tokens"] == sum(200 + i for i in range(len(_TRICKY_IDS)))
+
+
+def test_tricky_model_ids_survive_in_the_per_model_breakdown(ledger):
+    for model in _TRICKY_IDS:
+        _ledger_append("openrouter", model, 1, 1)
+
+    per_model = ledger_cost_report(ledger)["cost_per_model"]
+    for model in _TRICKY_IDS:
+        assert any(key.endswith(model) for key in per_model), model
+
+
+def test_newline_in_id_does_not_split_the_record(ledger):
+    # A raw newline would otherwise end the JSONL record mid-string and turn one
+    # call into two unparseable lines.
+    _ledger_append("openrouter", "new\nline", 5, 6)
+    lines = [ln for ln in ledger.read_text().splitlines() if ln.strip()]
+    assert len(lines) == 1
+    assert json.loads(lines[0])["m"] == "new\nline"
+
+
+def test_appends_accumulate(ledger):
+    _ledger_append("openrouter", "a", 1, 2)
+    _ledger_append("openrouter", "b", 3, 4)
+    rows = [json.loads(ln) for ln in ledger.read_text().splitlines() if ln.strip()]
+    assert [r["m"] for r in rows] == ["a", "b"]
+
+
+def test_no_ledger_env_is_a_no_op(tmp_path, monkeypatch):
+    monkeypatch.delenv("TRINITY_COST_LEDGER", raising=False)
+    _ledger_append("openrouter", "model", 1, 2)  # must not raise
+    assert not list(tmp_path.iterdir())