Skip to content

Commit 8428e4a

Browse files
Trecekclaude
andauthored
[FIX] fix: coerce None→0 in token telemetry serialization boundary (#3994)
## Summary `CanonicalTokenUsage` uses `int | None` for cache fields to preserve provider semantics (Codex has no cache-write concept). However, the persistence boundary in `flush_session_log()` and both readers (`load_from_log_dir`, `_load_sessions`) use `dict.get("key", 0)` which silently passes `None` through when the key is present — `dict.get()` only returns the default for **absent** keys, not for `None`-valued ones. This violates the `TokenUsageFileEntry` TypedDict contract (which declares `cache_write_tokens: int`, not `int | None`) and causes `int += None` → `TypeError` at reload. The architectural weakness is a **missing coercion layer at the serialization boundary**: `CanonicalTokenUsage.to_dict()` emits domain-model values (`None` = "not applicable") directly into a dict that consumers treat as storage-model values (all ints). The fix is to enforce the `TokenUsageFileEntry` contract structurally — make it impossible to write non-int values for int-typed fields. ## Requirements `patch_pr_token_summary` crashes with `TypeError: unsupported operand type(s) for +=: 'int' and 'NoneType'` when aggregating telemetry from pipeline runs that include Codex-backend sessions. The crash is caused by a `dict.get()` semantic trap: `None` values written to `token_usage.json` by the Codex backend bypass the `0` fallback in `.get("cache_write_tokens", 0)` because `dict.get()` only uses the default when the key is **absent**, not when it is explicitly `None`. Closes #3993 ## Implementation Plan Plan file: `/home/talon/projects/autoskillit-runs/remediation-20260610-190114-286663/.autoskillit/temp/rectify/rectify_none_bypass_token_serialization_2026-06-10_192500.md` 🤖 Generated with [Claude Code](https://claude.com/claude-code) via AutoSkillit <!-- autoskillit:pipeline-signature steps=prepare_pr,run_arch_lenses,compose_pr,annotate_pr_diff,review_pr --> ## Token Usage Summary | Step | Model | count | uncached | output | cache_read | peak_ctx | turns | cache_write | time | |------|-------|-------|----------|--------|------------|----------|-------|-------------|------| | rectify* | opus[1m] | 1 | 4.9k | 23.2k | 1.3M | 97.0k | 43 | 82.9k | 21m 11s | | review_approach* | opus[1m] | 1 | 4.5k | 6.1k | 260.0k | 49.0k | 14 | 35.4k | 5m 23s | | dry_walkthrough* | opus | 1 | 41 | 9.6k | 867.8k | 89.3k | 35 | 67.7k | 7m 33s | | implement* | MiniMax-M3 | 1 | 3.7M | 9.0k | 0 | 0 | 79 | 0 | 7m 44s | | audit_impl* | opus[1m] | 1 | 1.2k | 11.3k | 257.3k | 48.2k | 18 | 39.9k | 4m 33s | | prepare_pr* | MiniMax-M3 | 1 | 284.5k | 2.9k | 0 | 0 | 12 | 0 | 1m 7s | | compose_pr* | MiniMax-M3 | 1 | 351.0k | 1.3k | 0 | 0 | 12 | 0 | 51s | | **Total** | | | 4.4M | 63.4k | 2.6M | 97.0k | | 226.0k | 48m 25s | \* *Step used a non-Anthropic provider; caching behavior may differ.* ## Token Efficiency | Step | LoC Changed | cache_read/LoC | cache_write/LoC | output/LoC | |------|-------------|----------------|-----------------|------------| | rectify | 0 | — | — | — | | review_approach | 0 | — | — | — | | dry_walkthrough | 0 | — | — | — | | implement | 243 | 0.0 | 0.0 | 37.0 | | audit_impl | 0 | — | — | — | | prepare_pr | 0 | — | — | — | | compose_pr | 0 | — | — | — | | **Total** | **243** | 10866.4 | 930.1 | 260.9 | ## Model Usage Breakdown | Model | steps | uncached | output | cache_read | cache_write | time | |-------|-------|----------|--------|------------|-------------|------| | opus[1m] | 3 | 10.6k | 40.6k | 1.8M | 158.3k | 31m 9s | | opus | 1 | 41 | 9.6k | 867.8k | 67.7k | 7m 33s | | MiniMax-M3 | 3 | 4.4M | 13.2k | 0 | 0 | 9m 42s | Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent e8fe49b commit 8428e4a

9 files changed

Lines changed: 206 additions & 37 deletions

File tree

src/autoskillit/core/types/_type_token.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,12 @@ def to_dict(self) -> dict[str, Any]:
4545
return {
4646
"input_tokens": self.input_tokens,
4747
"output_tokens": self.output_tokens,
48-
"cache_read_tokens": self.cache_read_tokens,
49-
"cache_write_tokens": self.cache_write_tokens,
48+
"cache_read_tokens": self.cache_read_tokens
49+
if self.cache_read_tokens is not None
50+
else 0,
51+
"cache_write_tokens": self.cache_write_tokens
52+
if self.cache_write_tokens is not None
53+
else 0,
5054
"provider": self.provider,
5155
}
5256

src/autoskillit/execution/session_log.py

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -449,16 +449,20 @@ def flush_session_log(
449449
# Write per-session telemetry files; gate on data presence, not session identity
450450
label = _resolve_session_label(step_name, dispatch_id)
451451
if token_usage is not None:
452-
_cache_write = token_usage.get(
453-
"cache_write_tokens", token_usage.get("cache_creation_input_tokens", 0)
452+
_cw_raw = token_usage.get("cache_write_tokens")
453+
_cache_write = (
454+
_cw_raw
455+
if _cw_raw is not None
456+
else (token_usage.get("cache_creation_input_tokens") or 0)
454457
)
455-
_cache_read = token_usage.get(
456-
"cache_read_tokens", token_usage.get("cache_read_input_tokens", 0)
458+
_cr_raw = token_usage.get("cache_read_tokens")
459+
_cache_read = (
460+
_cr_raw if _cr_raw is not None else (token_usage.get("cache_read_input_tokens") or 0)
457461
)
458462
tu_data = {
459463
"session_label": label,
460-
"input_tokens": token_usage.get("input_tokens", 0),
461-
"output_tokens": token_usage.get("output_tokens", 0),
464+
"input_tokens": token_usage.get("input_tokens") or 0,
465+
"output_tokens": token_usage.get("output_tokens") or 0,
462466
"cache_write_tokens": _cache_write,
463467
"cache_read_tokens": _cache_read,
464468
"timing_seconds": timing_seconds if timing_seconds is not None else 0.0,
@@ -513,10 +517,10 @@ def flush_session_log(
513517
"peak_rss_kb": peak_rss_kb,
514518
"peak_oom_score": peak_oom_score,
515519
"step_name": step_name,
516-
"input_tokens": token_usage.get("input_tokens", 0) if token_usage else 0,
517-
"output_tokens": token_usage.get("output_tokens", 0) if token_usage else 0,
518-
"cache_write_tokens": token_usage.get("cache_write_tokens", 0) if token_usage else 0,
519-
"cache_read_tokens": token_usage.get("cache_read_tokens", 0) if token_usage else 0,
520+
"input_tokens": (token_usage.get("input_tokens") or 0) if token_usage else 0,
521+
"output_tokens": (token_usage.get("output_tokens") or 0) if token_usage else 0,
522+
"cache_write_tokens": (token_usage.get("cache_write_tokens") or 0) if token_usage else 0,
523+
"cache_read_tokens": (token_usage.get("cache_read_tokens") or 0) if token_usage else 0,
520524
"write_call_count": write_call_count,
521525
"fs_writes_detected": fs_writes_detected,
522526
"git_writes_detected": git_writes_detected,

src/autoskillit/hooks/token_summary_hook.py

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -239,19 +239,23 @@ def _load_sessions(
239239
_model = data.get("model_identifier", "")
240240
if _model and not entry["model"]:
241241
entry["model"] = _model
242-
entry["input_tokens"] += data.get("input_tokens", 0)
243-
entry["output_tokens"] += data.get("output_tokens", 0)
244-
entry["cache_write_tokens"] += data.get(
245-
"cache_write_tokens", data.get(_V1_CACHE_WRITE_KEY, 0)
242+
entry["input_tokens"] += data.get("input_tokens") or 0
243+
entry["output_tokens"] += data.get("output_tokens") or 0
244+
_cw = data.get("cache_write_tokens")
245+
entry["cache_write_tokens"] += (
246+
_cw if _cw is not None else (data.get(_V1_CACHE_WRITE_KEY) or 0)
246247
)
247-
entry["cache_read_tokens"] += data.get(
248-
"cache_read_tokens", data.get(_V1_CACHE_READ_KEY, 0)
248+
_cr = data.get("cache_read_tokens")
249+
entry["cache_read_tokens"] += (
250+
_cr if _cr is not None else (data.get(_V1_CACHE_READ_KEY) or 0)
249251
)
250252
_raw_timing = data.get("timing_seconds")
251253
entry["elapsed_seconds"] += float(_raw_timing) if _raw_timing is not None else 0.0
252254
entry["invocation_count"] += 1
253-
entry["loc_insertions"] = entry.get("loc_insertions", 0) + data.get("loc_insertions", 0)
254-
entry["loc_deletions"] = entry.get("loc_deletions", 0) + data.get("loc_deletions", 0)
255+
entry["loc_insertions"] = entry.get("loc_insertions", 0) + (
256+
data.get("loc_insertions") or 0
257+
)
258+
entry["loc_deletions"] = entry.get("loc_deletions", 0) + (data.get("loc_deletions") or 0)
255259
_raw_peak = data.get("peak_context", 0)
256260
if isinstance(_raw_peak, int) and _raw_peak > entry["peak_context"]:
257261
entry["peak_context"] = _raw_peak
@@ -389,10 +393,10 @@ def _format_model_table(aggregated: dict[str, dict[str, Any]]) -> str:
389393
}
390394
md = model_data[model]
391395
md["_steps"].add(entry.get("step_name", ""))
392-
md["input_tokens"] += entry.get("input_tokens", 0)
393-
md["output_tokens"] += entry.get("output_tokens", 0)
394-
md["cache_write_tokens"] += entry.get("cache_write_tokens", 0)
395-
md["cache_read_tokens"] += entry.get("cache_read_tokens", 0)
396+
md["input_tokens"] += entry.get("input_tokens") or 0
397+
md["output_tokens"] += entry.get("output_tokens") or 0
398+
md["cache_write_tokens"] += entry.get("cache_write_tokens") or 0
399+
md["cache_read_tokens"] += entry.get("cache_read_tokens") or 0
396400
md["elapsed_seconds"] += entry.get("elapsed_seconds", 0.0)
397401

398402
if not model_data:

src/autoskillit/pipeline/tokens.py

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,10 @@ def record(
140140
_model = model or _primary_model(token_usage)
141141
if _model and not e.model:
142142
e.model = _model
143-
e.input_tokens += token_usage.get("input_tokens", 0)
144-
e.output_tokens += token_usage.get("output_tokens", 0)
145-
e.cache_write_tokens += token_usage.get("cache_write_tokens", 0)
146-
e.cache_read_tokens += token_usage.get("cache_read_tokens", 0)
143+
e.input_tokens += token_usage.get("input_tokens") or 0
144+
e.output_tokens += token_usage.get("output_tokens") or 0
145+
e.cache_write_tokens += token_usage.get("cache_write_tokens") or 0
146+
e.cache_read_tokens += token_usage.get("cache_read_tokens") or 0
147147
e.invocation_count += 1
148148
e.loc_insertions += loc_insertions
149149
e.loc_deletions += loc_deletions
@@ -335,20 +335,22 @@ def load_from_log_dir(
335335
_model = data.get("configured_model") or data.get("model_identifier", "")
336336
if _model and not e.model:
337337
e.model = _model
338-
e.input_tokens += data.get("input_tokens", 0)
339-
e.output_tokens += data.get("output_tokens", 0)
340-
e.cache_write_tokens += data.get(
341-
"cache_write_tokens", data.get("cache_creation_input_tokens", 0)
338+
e.input_tokens += data.get("input_tokens") or 0
339+
e.output_tokens += data.get("output_tokens") or 0
340+
_cw = data.get("cache_write_tokens")
341+
e.cache_write_tokens += (
342+
_cw if _cw is not None else (data.get("cache_creation_input_tokens") or 0)
342343
)
343-
e.cache_read_tokens += data.get(
344-
"cache_read_tokens", data.get("cache_read_input_tokens", 0)
344+
_cr = data.get("cache_read_tokens")
345+
e.cache_read_tokens += (
346+
_cr if _cr is not None else (data.get("cache_read_input_tokens") or 0)
345347
)
346348
# timing_seconds is the on-disk key name written by session_log.py;
347349
# elapsed_seconds is the in-memory field name on TokenEntry.
348350
_raw_timing = data.get("timing_seconds")
349351
e.elapsed_seconds += float(_raw_timing) if _raw_timing is not None else 0.0
350-
e.loc_insertions += data.get("loc_insertions", 0)
351-
e.loc_deletions += data.get("loc_deletions", 0)
352+
e.loc_insertions += data.get("loc_insertions") or 0
353+
e.loc_deletions += data.get("loc_deletions") or 0
352354
_raw_peak = data.get("peak_context", 0)
353355
if isinstance(_raw_peak, int) and _raw_peak > e.peak_context:
354356
e.peak_context = _raw_peak

tests/core/test_canonical_token_usage.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,28 @@ def test_merge_raises_on_mismatched_providers(self):
184184
CanonicalTokenUsage.merge(a, b)
185185

186186

187+
class TestToDictNoneCoercion:
188+
def test_to_dict_coerces_none_to_zero_for_int_fields(self):
189+
ctu = CanonicalTokenUsage(
190+
input_tokens=100,
191+
output_tokens=50,
192+
cache_read_tokens=None,
193+
cache_write_tokens=None,
194+
provider="codex",
195+
)
196+
d = ctu.to_dict()
197+
for key in ("input_tokens", "output_tokens", "cache_read_tokens", "cache_write_tokens"):
198+
assert isinstance(d[key], int), f"{key} should be int, got {type(d[key])}"
199+
200+
def test_codex_to_dict_coerces_none_cache_fields(self):
201+
raw = {"input_tokens": 200, "output_tokens": 80, "cached_input_tokens": 50}
202+
ctu = CanonicalTokenUsage.from_codex_dict(raw)
203+
assert ctu.cache_write_tokens is None
204+
d = ctu.to_dict()
205+
assert d["cache_write_tokens"] == 0
206+
assert d["cache_read_tokens"] == 50
207+
208+
187209
class TestFrozen:
188210
def test_cannot_mutate_fields(self):
189211
usage = CanonicalTokenUsage(

tests/execution/backends/test_codex_result_parser.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ def test_cache_read_mapped_and_cache_write_none(self) -> None:
404404
result = CodexResultParser().parse_stdout(ndjson)
405405
canonical = result.raw["canonical_token_usage"]
406406
assert canonical["cache_read_tokens"] == 30
407-
assert canonical["cache_write_tokens"] is None
407+
assert canonical["cache_write_tokens"] == 0
408408

409409
def test_no_turn_completed_yields_none_canonical(self) -> None:
410410
"""Scenario 4: no turn.completed event — canonical_token_usage is None."""

tests/execution/test_session_log_flush.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,26 @@ def test_token_usage_json_includes_peak_context_and_turn_count(tmp_path):
641641
assert tu["turn_count"] == 14
642642

643643

644+
def test_token_usage_json_coerces_none_cache_fields_to_zero(tmp_path):
645+
"""Codex-originated None for cache_write_tokens must persist as 0 on disk."""
646+
_flush(
647+
tmp_path,
648+
step_name="implement",
649+
token_usage={
650+
"input_tokens": 200,
651+
"output_tokens": 80,
652+
"cache_write_tokens": None,
653+
"cache_read_tokens": 50,
654+
},
655+
timing_seconds=0.0,
656+
proc_snapshots=None,
657+
success=True,
658+
)
659+
tu = json.loads((tmp_path / "sessions" / "test-session-001" / "token_usage.json").read_text())
660+
assert tu["cache_write_tokens"] == 0
661+
assert tu["cache_read_tokens"] == 50
662+
663+
644664
def test_step_timing_json_schema(tmp_path):
645665
"""step_timing.json contains step_name and total_seconds."""
646666
_flush(tmp_path, step_name="plan", timing_seconds=20.0, proc_snapshots=None, success=False)

tests/infra/test_token_summary_core.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,3 +1206,43 @@ def test_reader_agreement_contract(
12061206
if scenario == "non-anthropic":
12071207
assert not disk_model.startswith("claude-")
12081208
assert disk_model not in ("sonnet", "opus", "haiku")
1209+
1210+
1211+
def test_load_sessions_handles_null_cache_write(tmp_path: Path) -> None:
1212+
"""_load_sessions must handle null cache_write_tokens in token_usage.json without crash."""
1213+
from autoskillit.hooks.token_summary_hook import _load_sessions
1214+
1215+
log_root = tmp_path / "logs"
1216+
log_root.mkdir()
1217+
pipeline_id = "test-null-cache"
1218+
1219+
_write_sessions(
1220+
log_root,
1221+
[
1222+
{
1223+
"dir_name": "s1",
1224+
"cwd": "/some/worktree",
1225+
"kitchen_id": pipeline_id,
1226+
"step_name": "plan",
1227+
},
1228+
],
1229+
)
1230+
sess_dir = log_root / "sessions" / "s1"
1231+
sess_dir.mkdir(parents=True, exist_ok=True)
1232+
(sess_dir / "token_usage.json").write_text(
1233+
json.dumps(
1234+
{
1235+
"session_label": "plan",
1236+
"input_tokens": 100,
1237+
"output_tokens": 50,
1238+
"cache_write_tokens": None,
1239+
"cache_read_tokens": None,
1240+
"timing_seconds": 0.0,
1241+
}
1242+
)
1243+
)
1244+
1245+
result = _load_sessions(log_root, pipeline_id)
1246+
assert "plan" in result
1247+
assert result["plan"]["cache_write_tokens"] == 0
1248+
assert result["plan"]["cache_read_tokens"] == 0

tests/pipeline/test_tokens_core.py

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,79 @@ def test_load_null_timing_seconds(self, tmp_path):
438438
report = log.get_report()
439439
assert report[0]["elapsed_seconds"] == 0.0
440440

441+
def test_load_null_cache_fields(self, tmp_path):
442+
"""TokenLog must handle null cache fields without TypeError."""
443+
_write_session(
444+
tmp_path,
445+
"s001",
446+
{
447+
"session_label": "implement",
448+
"input_tokens": 100,
449+
"output_tokens": 50,
450+
"cache_write_tokens": None,
451+
"cache_read_tokens": None,
452+
"timing_seconds": 0.0,
453+
"schema_version": 2,
454+
},
455+
)
456+
log = DefaultTokenLog()
457+
n = log.load_from_log_dir(tmp_path)
458+
assert n == 1
459+
report = log.get_report()
460+
assert report[0]["cache_write_tokens"] == 0
461+
assert report[0]["cache_read_tokens"] == 0
462+
463+
@pytest.mark.parametrize(
464+
"null_field",
465+
[
466+
"input_tokens",
467+
"output_tokens",
468+
"cache_write_tokens",
469+
"cache_read_tokens",
470+
"loc_insertions",
471+
"loc_deletions",
472+
],
473+
)
474+
def test_load_null_int_field(self, tmp_path, null_field):
475+
"""Each int field can be null on disk; load_from_log_dir must handle it."""
476+
payload = {
477+
"session_label": "implement",
478+
"input_tokens": 100,
479+
"output_tokens": 50,
480+
"cache_write_tokens": 0,
481+
"cache_read_tokens": 0,
482+
"loc_insertions": 5,
483+
"loc_deletions": 3,
484+
"timing_seconds": 0.0,
485+
"schema_version": 2,
486+
}
487+
payload[null_field] = None
488+
_write_session(tmp_path, "s001", payload)
489+
log = DefaultTokenLog()
490+
n = log.load_from_log_dir(tmp_path)
491+
assert n == 1
492+
493+
def test_load_preserves_zero_cache_write_with_v1_fallback(self, tmp_path):
494+
"""cache_write_tokens: 0 must not consult v1 fallback key (cache_creation_input_tokens)."""
495+
_write_session(
496+
tmp_path,
497+
"s001",
498+
{
499+
"session_label": "implement",
500+
"input_tokens": 100,
501+
"output_tokens": 50,
502+
"cache_write_tokens": 0,
503+
"cache_creation_input_tokens": 500,
504+
"cache_read_tokens": 0,
505+
"timing_seconds": 0.0,
506+
"schema_version": 2,
507+
},
508+
)
509+
log = DefaultTokenLog()
510+
log.load_from_log_dir(tmp_path)
511+
report = log.get_report()
512+
assert report[0]["cache_write_tokens"] == 0
513+
441514

442515
class TestLoadFromLogDirSchemaVersionCompat:
443516
"""Backward compatibility: load_from_log_dir reads both v1 and v2 token_usage.json."""

0 commit comments

Comments
 (0)