Implementation Plan: Split Token Summary Table Into 4 API Token Fields#615
Conversation
Replace the misleading 3-column layout (input/output/cached) with 4 accurate columns (uncached/output/cache_read/cache_write) across all three formatter implementations: telemetry_fmt.py, pretty_output.py, and token_summary_appender.py. Update all related tests including golden snapshot, compact KV prefixes, and parity checks. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit PR Review — Verdict: changes_requested
| pipeline=False, | ||
| ) | ||
| assert rendered is not None | ||
| assert "tokens_uncached:" in rendered |
There was a problem hiding this comment.
[info] tests: Assertions use substring matching (in rendered) rather than exact field values. Would catch value-formatting regressions if checking exact output like tokens_cache_read: 200.0k.
| wc = step.get("wall_clock_seconds", step.get("elapsed_seconds", 0.0)) | ||
| lines.append(f"{name} x{count} [in:{inp} out:{out} cached:{cached} t:{wc:.1f}s]") | ||
| lines.append( | ||
| f"{name} x{count} [uc:{inp} out:{out} cr:{cache_rd} cw:{cache_wr} t:{wc:.1f}s]" |
There was a problem hiding this comment.
[warning] cohesion: Asymmetric abbreviation: per-step compact KV uses short labels uc:, cr:, cw: but total summary lines use full total_uncached, total_cache_read, total_cache_write. The same asymmetry appears in pretty_output.py _fmt_get_token_summary. Either abbreviate the totals consistently or expand the per-step labels to match.
There was a problem hiding this comment.
Investigated — this is intentional. The per-step compact format [uc:X out:X cr:X cw:X t:Xs] is space-constrained inline; totals are standalone KV pairs where full names (total_uncached, total_cache_read, total_cache_write) improve readability. This same pattern appears consistently in both telemetry_fmt.py::format_compact_kv and pretty_output.py::_fmt_get_token_summary, and is explicitly encoded as expected behavior in tests/pipeline/test_telemetry_formatter.py:239-246 (asserts both 'uc:' and 'total_cache_read:').
| wc = step.get("wall_clock_seconds", step.get("elapsed_seconds", 0.0)) | ||
| lines.append(f"{name} x{count} [in:{inp} out:{out} cached:{cached} t:{wc:.1f}s]") | ||
| lines.append( | ||
| f"{name} x{count} [uc:{inp} out:{out} cr:{cache_rd} cw:{cache_wr} t:{wc:.1f}s]" |
There was a problem hiding this comment.
[info] cohesion: _fmt_run_skill uses full labels tokens_uncached, tokens_cache_read, tokens_cache_write while _fmt_get_token_summary uses abbreviated uc:, cr:, cw: for the same token categories in the same file. Creates inconsistent user experience.
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit review found 3 blocking issues (1 critical + 2 warnings). See inline comments for details.
… _fmt_run_skill Addresses reviewer comment: if cr: and if cw: conditionals suppress cache token fields when zero, but no test verified the suppression behavior. Added test_fmt_run_skill_suppresses_zero_cache_fields to assert absence of the fields.
…try_fmt.py Addresses reviewer comment: local variable names total_cached_rd/total_cached_wr diverge from total_cache_rd/total_cache_wr used in token_summary_appender.py. Renamed for cohesion — same token category names, consistent across files.
Summary
The token summary table (displayed in PRs, terminal, and compact KV output) collapses 4 distinct Claude API token fields into 3 misleading columns. The column labeled "input" actually shows only the tiny uncached delta (
input_tokens), and "cached" silently sums two cost-distinct categories (cache_read_input_tokensat 0.1x billing +cache_creation_input_tokensat 1.25x billing). This change splits the display into 4 token columns —uncached,output,cache_read,cache_write— across all 3 independent formatter implementations and their tests.No data model, extraction, or storage changes are needed —
TokenEntryalready preserves all 4 fields. This is purely a formatting-layer fix.Architecture Impact
Data Lineage Diagram
%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 60, 'curve': 'basis'}}}%% flowchart LR classDef cli fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff; classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff; classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff; classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff; classDef integration fill:#c62828,stroke:#ef9a9a,stroke-width:2px,color:#fff; subgraph API ["Claude API Response"] direction TB F1["input_tokens<br/>━━━━━━━━━━<br/>Uncached delta"] F2["output_tokens<br/>━━━━━━━━━━<br/>Generated tokens"] F3["cache_read_input_tokens<br/>━━━━━━━━━━<br/>0.1x billing"] F4["cache_creation_input_tokens<br/>━━━━━━━━━━<br/>1.25x billing"] end subgraph Storage ["TokenEntry Storage"] TE[("TokenEntry<br/>━━━━━━━━━━<br/>4 fields intact<br/>Accumulated per step")] TJ[("token_usage.json<br/>━━━━━━━━━━<br/>Persisted session data<br/>All 4 fields")] end subgraph Canonical ["● telemetry_fmt.py (Canonical Formatter)"] direction TB FMD["● format_token_table()<br/>━━━━━━━━━━<br/>Markdown table<br/>Step|uncached|output|cache_read|cache_write|count|time"] FTM["● format_token_table_terminal()<br/>━━━━━━━━━━<br/>Terminal table<br/>UNCACHED|OUTPUT|CACHE_RD|CACHE_WR"] FKV["● format_compact_kv()<br/>━━━━━━━━━━<br/>Compact KV<br/>uc:|out:|cr:|cw:"] end subgraph Hooks ["Stdlib Hooks (no autoskillit imports)"] direction TB TSA["● token_summary_appender._format_table()<br/>━━━━━━━━━━<br/>Reads token_usage.json<br/>Markdown table → GitHub PR body"] POS["● pretty_output._fmt_get_token_summary()<br/>━━━━━━━━━━<br/>Reads get_token_summary JSON<br/>Compact KV → PostToolUse"] POR["● pretty_output._fmt_run_skill()<br/>━━━━━━━━━━<br/>Reads run_skill result dict<br/>Inline KV → PostToolUse"] end subgraph Outputs ["Display Targets"] direction TB MD["PR Body<br/>━━━━━━━━━━<br/>GitHub markdown table"] TERM["Terminal<br/>━━━━━━━━━━<br/>Padded column output"] KV["Compact KV<br/>━━━━━━━━━━<br/>One-liner summaries"] HOOK["PostToolUse Output<br/>━━━━━━━━━━<br/>Hook-formatted display"] end F1 --> TE F2 --> TE F3 --> TE F4 --> TE TE --> TJ TE --> FMD TE --> FTM TE --> FKV TJ --> TSA TJ -.-> POS FMD -->|"markdown rows"| MD FTM -->|"padded columns"| TERM FKV -->|"kv lines"| KV TSA -->|"gh api PATCH"| MD POS -->|"formatted text"| HOOK POR -->|"formatted text"| HOOK class F1,F2,F3,F4 cli; class TE,TJ stateNode; class FMD,FTM,FKV handler; class TSA,POS,POR integration; class MD,TERM,KV,HOOK output;Color Legend:
Operational Diagram
%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 60, 'curve': 'basis'}}}%% flowchart TB classDef cli fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff; classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff; classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff; classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff; classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff; classDef integration fill:#c62828,stroke:#ef9a9a,stroke-width:2px,color:#fff; subgraph Triggers ["OPERATOR TRIGGERS"] direction TB GTS["get_token_summary<br/>━━━━━━━━━━<br/>MCP tool call<br/>format=json|markdown"] RS["run_skill<br/>━━━━━━━━━━<br/>MCP tool call<br/>Headless session"] PRPATCH["PR body update<br/>━━━━━━━━━━<br/>After open-pr skill<br/>PostToolUse event"] end subgraph State ["TOKEN STATE (read/write)"] direction TB TL[("DefaultTokenLog<br/>━━━━━━━━━━<br/>In-memory accumulator<br/>4 fields per step")] TJ[("token_usage.json<br/>━━━━━━━━━━<br/>Per-session disk files<br/>Read by stdlib hooks")] end subgraph Formatters ["● FORMATTERS (modified)"] direction TB TF["● telemetry_fmt.py<br/>━━━━━━━━━━<br/>format_token_table()<br/>format_token_table_terminal()<br/>format_compact_kv()"] TSA["● token_summary_appender.py<br/>━━━━━━━━━━<br/>_format_table()<br/>Stdlib-only hook"] PO["● pretty_output.py<br/>━━━━━━━━━━<br/>_fmt_get_token_summary()<br/>_fmt_run_skill()"] end subgraph Outputs ["OBSERVABILITY OUTPUTS (write-only)"] direction TB MDTBL["PR Body Table<br/>━━━━━━━━━━<br/>## Token Usage Summary<br/>Step|uncached|output|cache_read|cache_write|count|time"] TERM["Terminal Table<br/>━━━━━━━━━━<br/>STEP UNCACHED OUTPUT CACHE_RD CACHE_WR COUNT TIME<br/>Padded for readability"] KV["Compact KV<br/>━━━━━━━━━━<br/>name xN [uc:X out:X cr:X cw:X t:Xs]<br/>total_uncached / total_cache_read / total_cache_write"] HOOK["PostToolUse Display<br/>━━━━━━━━━━<br/>tokens_uncached:<br/>tokens_cache_read:<br/>tokens_cache_write:"] end GTS -->|"reads"| TL TL -.->|"flush"| TJ TJ -->|"load_sessions"| TSA TJ -.->|"via MCP JSON payload"| PO GTS --> TF TF -->|"markdown"| MDTBL TF -->|"terminal"| TERM TF -->|"compact"| KV RS -->|"PostToolUse event"| PO PO -->|"_fmt_run_skill"| HOOK PO -->|"_fmt_get_token_summary"| KV PRPATCH -->|"PostToolUse event"| TSA TSA -->|"gh api PATCH"| MDTBL class GTS,RS,PRPATCH cli; class TL,TJ stateNode; class TF,TSA,PO handler; class MDTBL,TERM,KV,HOOK output;Color Legend:
Closes #604
Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/impl-20260404-190817-266225/.autoskillit/temp/make-plan/token_summary_4_columns_plan_2026-04-04_191000.md🤖 Generated with Claude Code via AutoSkillit