diff --git a/.gitignore b/.gitignore index cc274662..76a2d1d3 100644 --- a/.gitignore +++ b/.gitignore @@ -190,9 +190,14 @@ test/test_data/*.html local.ps1 -# Integration test data - exclude generated cache/HTML, keep JSONL files +# Integration test data - exclude generated cache/HTML/images, keep JSONL files test/test_data/real_projects/*/cache/ test/test_data/real_projects/*/*.html test/test_data/real_projects/index.html +# Referenced-image export writes images/ next to the HTML it references, so a +# render pointed at this corpus leaves binaries here. Same generated class as +# the two rules above, which predate that export mode. Use `git add -f` if an +# image ever needs to be a real fixture. +test/test_data/real_projects/*/images/ # SQLite cache the CLI drops into a projects dir when run against test data claude-code-log-cache.db diff --git a/claude_code_log/converter.py b/claude_code_log/converter.py index c7df57b3..7656a906 100644 --- a/claude_code_log/converter.py +++ b/claude_code_log/converter.py @@ -22,7 +22,10 @@ import dateparser if TYPE_CHECKING: + from collections.abc import Iterable + from .cache import CacheManager + from .providers.base import ProviderTokenTotals from .utils import ( coalesce_trunk_session_id, @@ -2931,6 +2934,65 @@ def _wholesale_should_render( return stale +def _sum_provider_token_totals( + totals: "Iterable[Optional[ProviderTokenTotals]]", +) -> dict[str, int]: + """Sum per-session cumulative totals into the four index-summary token + keys. ``None`` sessions (no ``token_count``) contribute nothing. + + Each :class:`ProviderTokenTotals` is ALREADY a whole-session cumulative + figure, so summing across the DISTINCT sessions of a project is correct — + what must never be summed is the per-step deltas WITHIN one session, which + is why the provider returns one cumulative value per session, not a stream. + + ``total_cache_creation_tokens`` is kept at 0 purely so the emitted dict is + shape-identical to the Claude path (the drift pin's contract). Codex has no + cache-creation concept and the renderer omits a zero column, so nothing + labelled "Cache Creation" ever renders — absent, not "0". + """ + total_input = 0 + total_output = 0 + total_cache_read = 0 + for totals_item in totals: + if totals_item is None: + continue + total_input += totals_item.input_tokens + total_output += totals_item.output_tokens + total_cache_read += totals_item.cache_read_tokens + return { + "total_input_tokens": total_input, + "total_output_tokens": total_output, + "total_cache_creation_tokens": 0, + "total_cache_read_tokens": total_cache_read, + } + + +# The four token keys a project card carries, in the shape +# ``_sum_provider_token_totals`` emits. Named once so the provider path and +# the per-message fallback cannot drift apart. +_PROJECT_TOKEN_KEYS = ( + "total_input_tokens", + "total_output_tokens", + "total_cache_creation_tokens", + "total_cache_read_tokens", +) + + +def _project_token_totals_from_messages( + messages: List[TranscriptEntry], +) -> dict[str, int]: + """Per-message project token totals, in the project-card key shape. + + The fallback for a provider with no cumulative ``session_token_totals`` + seam: its usage lives on the messages, exactly as Claude's does, so the + ordinary accumulator is the right source. Kept to the four token keys so + it can be spread into the card dict interchangeably with + ``_sum_provider_token_totals``. + """ + aggregates = compute_project_aggregates(messages) + return {key: int(aggregates.get(key, 0) or 0) for key in _PROJECT_TOKEN_KEYS} + + def render_provider_wholesale( provider_name: str, sessions_root: Optional[Path], @@ -3061,13 +3123,29 @@ def render_provider_wholesale( # Phase 1 — load every session in the project fresh. v1 always re-parses # rollouts (cache-backed load is a documented deferral); only rendering # is skipped when unchanged. + # Entries and cumulative token totals come back from ONE provider call: + # a provider reading both from the same file (Codex) would otherwise + # re-parse it for the totals, which measured +118 rollout decodes and + # +478 MB re-parsed over a 34-rollout archive. The base implementation + # of the seam is the old call pair, so providers that don't override it + # behave exactly as before. + # + # The totals ride along with the entries rather than being collected + # separately, because they must stay subject to the SAME survival test: + # a session emptied by --from-date/--to-date contributes no messages and + # must likewise contribute no tokens. Hoisting the totals out of this + # filter would let a filtered-out session inflate the project totals — + # a behaviour change that no decode count would reveal. loaded: list[tuple[SessionInfo, list[TranscriptEntry]]] = [] + loaded_totals: dict[str, Optional[ProviderTokenTotals]] = {} for info in group_infos: - messages = list(provider.load_session_under(sessions_root, info.session_id)) + session = provider.load_session_with_totals(sessions_root, info.session_id) + messages = session.entries if from_date or to_date: messages = filter_messages_by_date(messages, from_date, to_date) if messages: loaded.append((info, messages)) + loaded_totals[info.session_id] = session.token_totals if not loaded: continue # everything in this project was empty / filtered out @@ -3076,6 +3154,31 @@ def render_provider_wholesale( m for _info, msgs in loaded for m in msgs ] + # Token accounting (#296 deferral). Codex-style providers record + # cumulative session totals in the rollout rather than per-assistant- + # message ``usage``, so the message-usage accumulators + # (compute_session_data / compute_project_aggregates) see zero here. + # Pull each session's cumulative total from the provider seam and apply + # it directly — a cumulative figure must bypass that per-message + # summation, never flow through it (that path would double-count). The + # default seam returns None, so a provider without session-level totals + # leaves every surface exactly as before. + session_totals: dict[str, Optional[ProviderTokenTotals]] = { + info.session_id: loaded_totals[info.session_id] for info, _ in loaded + } + project_token_totals = _sum_provider_token_totals(session_totals.values()) + # Did the provider actually supply cumulative totals? When it did not — + # every seam returned ``None``, which is the DEFAULT — the sum above is + # an all-zero dict, and applying it would REPLACE a provider's real + # per-message aggregates with zeros. The session-level override below + # is already gated on ``session_total is not None``; the two + # project-level uses must be gated symmetrically, or a provider that + # reports usage per message but has no cumulative seam silently loses + # its project totals. + has_provider_token_totals = any( + total is not None for total in session_totals.values() + ) + # Phase 2 — populate the cache and capture the pre-render modified set. cache: Optional[CacheManager] = None modified_sources: set[Path] = set() @@ -3105,10 +3208,28 @@ def render_provider_wholesale( info.source_path, messages, subagents_fp="" ) merged_session_data.update(compute_session_data(messages)) + # Replace the zero message-usage token totals with the + # provider's cumulative session totals (see the seam above). + # Keyed by session_id — Codex messages carry sessionId, so + # compute_session_data already keys each session that way. + for info, _ in loaded: + session_total = session_totals.get(info.session_id) + session_datum = merged_session_data.get(info.session_id) + if session_total is not None and session_datum is not None: + session_datum.total_input_tokens = session_total.input_tokens + session_datum.total_output_tokens = session_total.output_tokens + session_datum.total_cache_creation_tokens = 0 + session_datum.total_cache_read_tokens = ( + session_total.cache_read_tokens + ) cache.update_session_cache(merged_session_data) - cache.update_project_aggregates( - **compute_project_aggregates(combined_messages) - ) + project_aggregates = compute_project_aggregates(combined_messages) + # Cumulative project totals override the (zero) per-message sum + # — but only when the provider supplied any; see + # ``has_provider_token_totals``. + if has_provider_token_totals: + project_aggregates.update(project_token_totals) + cache.update_project_aggregates(**project_aggregates) session_counts = { sid: sd.message_count for sid, sd in merged_session_data.items() } @@ -3172,6 +3293,12 @@ def render_provider_wholesale( "first_user_message": first_user or "[No user message found in session.]", "file": f"{rel_dest}/session-{session_key}{suffix}.{ext}", + # NOTE: no per-session token_summary key here — the Claude + # index project-card session list carries none either, and + # the drift pin (test_index_summary_dict_shape_matches_claude_path) + # locks the two session-dict shapes together. Per-session + # cumulative totals are stored on the session cache instead + # (durability); the project card shows the rolled-up total. } ) @@ -3212,14 +3339,20 @@ def render_provider_wholesale( "jsonl_count": len(session_dicts), "message_count": len(combined_messages), "last_modified": last_modified, - # Codex has no token accounting yet; emit zero totals so the - # index-summary dict is shape-identical to the Claude path - # (the contract the drift pin locks) rather than relying on the - # renderer's ``.get(..., 0)`` fallbacks. - "total_input_tokens": 0, - "total_output_tokens": 0, - "total_cache_creation_tokens": 0, - "total_cache_read_tokens": 0, + # Project-card token totals — the cumulative session totals + # summed across the project's sessions (#296 deferral closed). + # ``_sum_provider_token_totals`` keeps the dict shape-identical + # to the Claude path (all four keys, cache_creation pinned 0 and + # never displayed) so the drift pin's contract still holds. + # Sibling of the cache-side override above and gated the same + # way: with no cumulative seam this dict is all zeros, so fall + # back to the per-message aggregate rather than zeroing the + # card. Computed lazily — the fallback never runs for Codex. + **( + project_token_totals + if has_provider_token_totals + else _project_token_totals_from_messages(combined_messages) + ), "latest_timestamp": last_ts_all or "", "earliest_timestamp": first_ts_all or "", "working_directories": working_directories, diff --git a/claude_code_log/providers/base.py b/claude_code_log/providers/base.py index 245ab7c0..cb6fc55c 100644 --- a/claude_code_log/providers/base.py +++ b/claude_code_log/providers/base.py @@ -18,6 +18,49 @@ ) +@dataclass(frozen=True) +class ProviderTokenTotals: + """Cumulative session token totals surfaced by a provider that records + them at the session level (e.g. Codex ``token_count`` events), as opposed + to the Claude path which sums per-assistant-message ``usage``. + + Mapped onto the same four columns the index renders, minus one: + ``cache_creation`` is deliberately ABSENT, not zero. Codex has no + cache-creation concept, and an omitted column ("we don't record this") is + a different, honest claim than a zero one ("we recorded zero of it"). + + ``total_tokens`` is the record's own authoritative total — never + recomputed from the components. For the well-formed cumulative records + that back session totals the identity ``input + cache_read + output == + total`` holds, but degenerate records (all components zero, non-zero + total) do occur in the per-step stream, and there the stored total is the + only trustworthy figure. It is **currently unconsumed by the render/cache + paths** (the four displayed columns come from input/cache_read/output); it + is kept as the reconstruction anchor the tests validate and the reserve a + future per-turn layer would need. + """ + + input_tokens: int # billable non-cached input = input_tokens - cached + cache_read_tokens: int # cached_input_tokens + output_tokens: int # output_tokens, which already includes reasoning + total_tokens: int # record's authoritative total; never recomputed + + +@dataclass(frozen=True) +class LoadedSession: + """One session's rendered entries together with its cumulative token + totals, as returned by :meth:`BaseProvider.load_session_with_totals`. + + The pair travels together because the caller needs both and a provider may + be able to produce both from a single parse. ``token_totals`` is ``None`` + for the providers (and the sessions) that record none — omitted, never + zeroed, since a zero total is a different claim from an absent one. + """ + + entries: list[TranscriptEntry] + token_totals: Optional[ProviderTokenTotals] + + @dataclass class SessionInfo: provider: str @@ -281,3 +324,45 @@ def load_session_under( def get_session_stats(self, session_id: str) -> dict[str, Any]: return {} + + def session_token_totals( + self, root: Path, session_id: str + ) -> Optional[ProviderTokenTotals]: + """Cumulative session token totals for the session ``session_id`` under + ``root``, or ``None`` when the provider records none. + + The default is ``None``: providers whose token accounting is + per-assistant-message ``usage`` (Claude) leave this alone — those + totals flow through the message-usage accumulators in ``converter``, + not this seam. A provider that records session-level cumulative totals + (Codex) overrides this so the wholesale/index path can surface them + directly, bypassing the per-message summation that would otherwise + double-count a cumulative figure. + + Still the seam for a totals-only lookup. The wholesale walker uses + :meth:`load_session_with_totals` instead, so that a provider whose + totals live in the same source it just parsed need not re-read it. + """ + return None + + def load_session_with_totals( + self, root: Path, session_id: str, max_messages: Optional[int] = None + ) -> LoadedSession: + """Entries *and* cumulative token totals for one session, in one call. + + The wholesale walker needs both, and for a provider that reads them + from the same file this is the difference between parsing that file + once and parsing it twice — the second parse being work the first + already did and discarded, not a recomputation worth caching (the + decoded records of one real archive reach 124 MB for a single session, + so any cache here would need a byte budget rather than an entry count). + + **The default is exactly the pair of calls the walker used to make**, + so a provider that does not override this cannot change behaviour by + the seam existing. Override it only when the two can genuinely share + work; leave it alone otherwise. + """ + return LoadedSession( + entries=list(self.load_session_under(root, session_id, max_messages)), + token_totals=self.session_token_totals(root, session_id), + ) diff --git a/claude_code_log/providers/codex.py b/claude_code_log/providers/codex.py index ce835133..6a1bb02c 100644 --- a/claude_code_log/providers/codex.py +++ b/claude_code_log/providers/codex.py @@ -34,6 +34,8 @@ from .base import ( BaseProvider, + LoadedSession, + ProviderTokenTotals, SessionInfo, file_mtime_iso, make_assistant_entry, @@ -131,6 +133,33 @@ class CodexSessionInfo(SessionInfo): inherited_prefix_records: int = 0 +@dataclass +class _SessionIndex: + """Everything one tree walk already learned, kept for the whole run. + + ``paths`` and ``headers`` are both filled by the index build, which reads + every rollout's header anyway; keeping the identity it produced is what + stops discovery and each load from reading those headers again. + + ``resolved`` is separate and deliberately so. A ``CodexSessionIdentity`` + whose ``inherited_prefix_records`` is 0 is indistinguishable from one whose + prefix was never computed -- and 0 is the *common* case, so conflating them + would silently send every non-fork session back down the slow path. + Membership in ``resolved`` is therefore the "prefix has been computed" + signal: entries are admitted only after resolution, which makes the + invariant structural instead of a sentinel every caller must remember. + + Sized for the whole run on purpose: entries are a fixed handful of scalars + and two ``Path``s, so bounding the entry *count* bounds the memory. That is + what separates this from caching decoded *records*, where one rollout is + 124 MB and no entry-count bound is a memory bound. + """ + + paths: dict[str, list[Path]] + headers: dict[str, CodexSessionIdentity] + resolved: dict[str, CodexSessionIdentity] + + @dataclass(frozen=True) class _DecodedRecord: line_no: int @@ -224,15 +253,170 @@ def _contained_rollouts(root: Path) -> Iterator[Path]: continue +def _token_totals_from_records( + records: list[_DecodedRecord], +) -> Optional[ProviderTokenTotals]: + """Session token totals from the LAST cumulative ``token_count`` record. + + The session total is the final ``payload.info.total_token_usage``, never a + sum of the per-step deltas: each ``total_token_usage`` is cumulative and + monotonic over the session, so the last one already subsumes every prior + turn. Compaction lowers the live context window but does NOT reset the + cumulative counter, so "last record" stays correct across a compacted + session. That monotonicity is ENFORCED, not merely assumed — if + ``total_tokens`` ever decreases the guard below omits the session's totals. + Returns ``None`` when the session emitted no ``token_count`` (a + pre-accounting rollout) or when monotonicity is violated — the totals are + then OMITTED, not zeroed, and never a guessed-through wrong number. + + WHY SESSION-LEVEL ONLY (evidenced design limit — do not "fix" into + per-message numbers): + The argument is STRUCTURAL, not statistical. A ``token_count`` delta + (``last_token_usage``) measures everything consumed since the *previous* + ``token_count`` — and one agent-loop step bundles reasoning + assistant + text + a tool call + its (often large, cached) tool result under a single + delta. The window contains more than one rendered thing, so a delta cannot + be attributed to any one message the transcript renders, regardless of + which record the step happens to end on. The corpus distribution merely + confirms that steps overwhelmingly end on tool work: measured post- + inherited-prefix-strip (the records that actually render), n=4138 events + across 34 sessions, ~75.6% of ``token_count`` events follow a tool- + execution step and ~22.5% follow an assistant/agent message — but even the + 22.5% are not attributable, because that message shares its delta with the + reasoning and the next turn's cached context re-read. Per-message (and even + per-turn) attribution is therefore not recoverable from this stream; the + session cumulative is the finest honest unit, which is why this returns a + whole-session total. + """ + last_usage: Optional[dict[str, Any]] = None + prev_total: Optional[int] = None + malformed_total_shapes: list[str] = [] + first_malformed_at: Optional[str] = None + for record in records: + if record.kind != "event_msg": + continue + if record.payload.get("type") != "token_count": + continue + info = record.payload.get("info") + if not isinstance(info, dict): + continue + usage_raw = cast(dict[str, Any], info).get("total_token_usage") + if not isinstance(usage_raw, dict): + continue + usage = cast(dict[str, Any], usage_raw) + # Monotonicity guard. total_token_usage is cumulative, so total_tokens + # must never decrease. If it does, the cumulative-counter assumption has + # broken (e.g. a future Codex build that resets the counter mid-session), + # and NO single record is the honest total — "last" would understate, + # and max() would report a pre-reset peak; both are confidently wrong. + # Fail closed: omit the session's totals and warn, exactly as a + # pre-accounting rollout (no token_count) is omitted. A wrong number is + # worse than an absent one. Enforced, not merely assumed — monk and I + # measured 0 violations across the corpus, so this fires only on a + # future spec change, loudly. + # A record whose ``total_tokens`` is absent or not an int tells us + # nothing about the ordering. Coercing it to 0 (the pre-review + # behaviour) made it look like a counter reset, so a single malformed + # record tripped the guard below and omitted the WHOLE session's totals + # for what is a data-quality problem, not a broken counter. Skip it + # instead — out of the comparison AND out of last-record selection, + # since ``_map_cumulative_usage`` treats the stored total as + # authoritative and would carry a fabricated 0 through — and say so + # once at the end, naming the shape actually seen. ``bool`` is excluded + # deliberately: it is an ``int`` subclass, so a JSON ``true`` would + # otherwise pass as the total 1. + total_raw = usage.get("total_tokens") + if not isinstance(total_raw, int) or isinstance(total_raw, bool): + malformed_total_shapes.append( + "absent" if total_raw is None else type(total_raw).__name__ + ) + if first_malformed_at is None: + # Enough to open the rollout at the offending record instead of + # re-scanning it. One warning per session stays O(1), but a bare + # count would leave the reader nothing to search on. + first_malformed_at = record.timestamp or "(no timestamp)" + continue + total = total_raw + if prev_total is not None and total < prev_total: + logger.warning( + "Codex token_count total_tokens decreased (%d < %d); cumulative " + "monotonicity broken — omitting session totals", + total, + prev_total, + ) + return None + prev_total = total + last_usage = usage + if malformed_total_shapes: + # Degrade visibly: the totals we return are real, but they are drawn + # from fewer records than the session actually holds. + logger.warning( + "Codex token_count: skipped %d record(s) whose total_tokens was " + "not an integer (saw: %s; first at %s); session totals come from " + "the remaining records", + len(malformed_total_shapes), + ", ".join(sorted(set(malformed_total_shapes))), + first_malformed_at, + ) + if last_usage is None: + return None + return _map_cumulative_usage(last_usage) + + +def _map_cumulative_usage(usage: dict[str, Any]) -> ProviderTokenTotals: + """Map a Codex ``total_token_usage`` dict onto the index's token columns. + + THE SUBTRACTION PIN: billable input EXCLUDES the cached portion, and + ``cache_read`` is the sole home of the cached tokens. A future edit that + folded ``cached`` back into ``input`` here (or dropped the subtraction) + would double-count the cached tokens — index totals would balloon by the + cache-read column. Keep ``input = input_tokens - cached`` and + ``cache_read = cached`` disjoint. ``max(..., 0)`` guards a malformed record + where ``cached > input``. + + ``total_tokens`` is carried through from the record, authoritative and + never recomputed (a degenerate record with zero components but a non-zero + total must keep its stored total). ``output`` already includes + ``reasoning_output_tokens``, so reasoning is not added again. + """ + + def _as_int(value: Any) -> int: + """Every component field routes through here, so this is the ONE place + the int predicate is stated — add a new component and it is covered by + construction rather than by remembering to repeat the check. + + ``bool`` is excluded explicitly because it is an ``int`` subclass: a JSON + ``true`` would otherwise contribute a phantom 1 to a token column. The + record-selection guard in :func:`_token_totals_from_records` already + excludes bools for ``total_tokens``; it did not cover the components, + which reach this mapping on any record whose *total* is well-formed. + """ + if isinstance(value, bool): + return 0 + return value if isinstance(value, int) else 0 + + input_tokens = _as_int(usage.get("input_tokens")) + cached = _as_int(usage.get("cached_input_tokens")) + output_tokens = _as_int(usage.get("output_tokens")) + total_tokens = _as_int(usage.get("total_tokens")) + return ProviderTokenTotals( + input_tokens=max(input_tokens - cached, 0), + cache_read_tokens=cached, + output_tokens=output_tokens, + total_tokens=total_tokens, + ) + + class CodexProvider(BaseProvider): """Read active Codex rollout files from ``$CODEX_HOME/sessions``.""" def __init__(self) -> None: - # Memoize the thread-id → paths index per resolved sessions root so a + # Memoize what one tree walk learned, per resolved sessions root, so a # wholesale run (discovery + per-session loads) reads each rollout's - # header once instead of O(sessions) times. Safe within a single CLI - # run — the sessions tree does not change mid-render. - self._index_cache: dict[Path, dict[str, list[Path]]] = {} + # header once instead of O(sessions) times and computes each fork's + # inherited prefix once instead of once per phase. Safe within a single + # CLI run — the sessions tree does not change mid-render. + self._index_cache: dict[Path, _SessionIndex] = {} def detect_path(self, path: Path) -> bool: """A Codex rollout file, or a directory containing at least one.""" @@ -274,24 +458,8 @@ def discover_sessions_under(self, root: Path) -> Iterator[SessionInfo]: yield from self._discover_in(root) def _discover_in(self, sessions_root: Path) -> Iterator[SessionInfo]: - # A duplicated thread id is corrupt/ambiguous. Discovery remains - # useful and deterministic by retaining the lexicographically first - # path; loading that id reports the ambiguity instead of guessing. - identities: dict[str, CodexSessionIdentity] = {} - index = self._session_index(sessions_root) - for path in self._rollout_paths(sessions_root): - identity = self._read_identity(path) - if identity.thread_id in identities: - logger.warning( - "Duplicate Codex thread id %s; retaining first discovered rollout", - identity.thread_id, - ) - continue - identities[identity.thread_id] = self._with_inherited_prefix( - identity, index - ) - - for identity in identities.values(): + index = self._index_for(sessions_root) + for identity in self._resolve_prefixes(index): yield CodexSessionInfo( provider="codex", session_id=identity.thread_id, @@ -306,6 +474,53 @@ def _discover_in(self, sessions_root: Path) -> Iterator[SessionInfo]: inherited_prefix_records=identity.inherited_prefix_records, ) + def _resolve_prefixes(self, index: _SessionIndex) -> list[CodexSessionIdentity]: + """Every discovered identity, each fork's inherited prefix computed once + and retained on the index for the loads that follow. + + Grouped by parent: a parent shared by *k* forks is decoded **once**, not + once per fork. Peak residency is one parent's candidate list plus one + child's -- the same pair the ungrouped path already held transiently -- + so the reduction costs no memory. + """ + # A duplicated thread id is corrupt/ambiguous. Discovery remains useful + # and deterministic by retaining the lexicographically first path; + # loading that id reports the ambiguity instead of guessing. + for thread_id, paths in index.paths.items(): + for _extra in paths[1:]: + logger.warning( + "Duplicate Codex thread id %s; retaining first discovered rollout", + thread_id, + ) + + order = list(index.headers) + pending: dict[Path, list[CodexSessionIdentity]] = {} + for thread_id in order: + identity = index.headers[thread_id] + parent_id = identity.parent_thread_id + parent_paths = index.paths.get(parent_id, []) if parent_id else [] + if len(parent_paths) != 1: + # No uniquely resolvable parent: nothing to inherit, and that + # answer is final rather than merely uncomputed. + index.resolved[thread_id] = identity + continue + pending.setdefault(parent_paths[0], []).append(identity) + + for parent_path, children in pending.items(): + parent_records = self._prefix_candidates( + list(self._decode_records(parent_path)) + ) + for identity in children: + index.resolved[identity.thread_id] = self._prefix_against( + identity, parent_records + ) + # Release the parent before starting the next group, so residency is + # one parent's candidates and not the corpus's. + del parent_records + + # Emit in tree-walk order, whatever order the grouping resolved them in. + return [index.resolved[thread_id] for thread_id in order] + def load_session( self, session_id: str, max_messages: Optional[int] = None ) -> Iterator[TranscriptEntry]: @@ -321,28 +536,113 @@ def load_session_under( tree or the data dir), with sibling fork-prefix stripping.""" yield from self._load_in(root, session_id, max_messages) - def _load_in( - self, sessions_root: Path, session_id: str, max_messages: Optional[int] - ) -> Iterator[TranscriptEntry]: + def _resolve_and_decode( + self, sessions_root: Path, session_id: str + ) -> tuple[CodexSessionIdentity, list[_DecodedRecord]]: + """Resolve ``session_id`` under ``sessions_root`` and decode its rollout + once, returning the identity and the post-inherited-prefix records. + + The single expensive step in every path that reads a session: one + rollout decode. :meth:`_load_in` and :meth:`load_session_with_totals` + share it so a caller that needs both entries and totals pays for one + decode rather than two. + + Raises (rather than soft-missing) on an unresolvable or ambiguous id, + matching the loader's contract; :meth:`session_token_totals` keeps its + own tolerant resolution, since a totals lookup must never crash a + render. + """ if not session_id or _SESSION_ID_RE.fullmatch(session_id) is None: raise ValueError(f"Invalid session_id: {session_id}") - if max_messages is not None and max_messages <= 0: - return - index = self._session_index(sessions_root) - if session_id not in index: + index = self._index_for(sessions_root) + # These two checks stay AHEAD of the resolved-identity lookup, and that + # ordering is behaviour rather than style: discovery retains-and-warns on + # a duplicated thread id, so the index legitimately holds an identity for + # an id that is illegal to load. Consulting it first would quietly load + # the first rollout where the contract says raise. + if session_id not in index.paths: raise FileNotFoundError(f"Codex session {session_id} not found") - paths = index[session_id] + paths = index.paths[session_id] if len(paths) != 1: raise ValueError(f"Multiple Codex rollouts have thread id {session_id}") - identity = self._with_inherited_prefix(self._read_identity(paths[0]), index) - records = list(self._decode_records(identity.path)) + identity = self._identity_for(index, session_id, paths[0]) records = self._without_inherited_prefix( - records, identity.inherited_prefix_records + list(self._decode_records(identity.path)), + identity.inherited_prefix_records, ) + return identity, records + + def _identity_for( + self, index: _SessionIndex, session_id: str, path: Path + ) -> CodexSessionIdentity: + """The prefix-resolved identity for *session_id*, reusing discovery's + work when it ran and computing it when it did not. + + Callers must have settled ambiguity before calling this. + """ + cached = index.resolved.get(session_id) + if cached is not None: + return cached + # No discovery in this run (a standalone lookup): compute exactly as + # before. The fast path must not be the only correct path. + header = index.headers.get(session_id) or self._read_identity(path) + return self._with_inherited_prefix(header, index.paths) + + def _load_in( + self, sessions_root: Path, session_id: str, max_messages: Optional[int] + ) -> Iterator[TranscriptEntry]: + if max_messages is not None and max_messages <= 0: + # Still validate the id, so an invalid one raises regardless of + # max_messages — the check used to precede this early return. + if not session_id or _SESSION_ID_RE.fullmatch(session_id) is None: + raise ValueError(f"Invalid session_id: {session_id}") + return + + identity, records = self._resolve_and_decode(sessions_root, session_id) yield from self._normalize_records(identity, records, max_messages) + def load_session_with_totals( + self, root: Path, session_id: str, max_messages: Optional[int] = None + ) -> LoadedSession: + """Entries and cumulative totals from a SINGLE rollout decode. + + The base implementation would call the loader and then + :meth:`session_token_totals`, and the two repeat the same index lookup, + identity resolution, decode and prefix strip — measured at +118 decodes + and +478 MB re-parsed across a 34-rollout archive, purely to recompute + what the first pass had already produced. Only the *tail* differs: + normalize for entries, last cumulative ``token_count`` for totals. + + Totals are computed **before** normalizing, deliberately: the normalize + passes are free to transform their input, and computing totals first + means correctness here does not depend on whether they do. Do not + reorder these two lines. + """ + if max_messages is not None and max_messages <= 0: + # Nothing to share: with no entries requested there is no decode for + # the totals to piggyback on, so the override has no advantage here — + # and every attempt to hand-write this branch diverged from the base + # in some input. Delegating makes equivalence hold BY CONSTRUCTION + # rather than by argument, at the same one decode the base pays. + # + # Two divergences this avoids, both found by measurement rather than + # by reading: returning ``None`` reported no totals for a session + # that HAS them (base reports them, since its ``load_session_under`` + # returns early while ``session_token_totals`` still runs); and + # resolving eagerly raised ``FileNotFoundError`` on an unknown id + # where the base silently returns empty, because the base never + # resolves at all on this path. + return super().load_session_with_totals(root, session_id, max_messages) + + identity, records = self._resolve_and_decode(root, session_id) + totals = _token_totals_from_records(records) + entries: list[TranscriptEntry] = list( + self._normalize_records(identity, records, max_messages) + ) + return LoadedSession(entries=entries, token_totals=totals) + def load_session_from_path( self, path: Path, max_messages: Optional[int] = None ) -> Iterator[TranscriptEntry]: @@ -361,6 +661,36 @@ def load_session_from_path( records = list(self._decode_records(path)) yield from self._normalize_records(identity, records, max_messages) + def session_token_totals( + self, root: Path, session_id: str + ) -> Optional[ProviderTokenTotals]: + """Cumulative token totals for one Codex session, read from its + ``token_count`` events. + + Resolution mirrors :meth:`_load_in` — same index lookup and the same + ``_without_inherited_prefix`` strip — so the totals are computed over + exactly the records this session renders. A fork must not inherit its + parent's cumulative ``token_count`` (currently ``inherited_prefix_records`` + is 0 in observed corpora, but computing post-strip keeps this correct + by construction rather than by that coincidence). + + Returns ``None`` — totals OMITTED, not zeroed — when the session has no + ``token_count`` events (pre-accounting rollouts) or cannot be resolved + unambiguously. A totals lookup must never crash a wholesale render, so + ambiguity is a soft miss here, unlike the hard errors :meth:`_load_in` + raises. + """ + index = self._index_for(root) + paths = index.paths.get(session_id) + if not paths or len(paths) != 1: + return None + identity = self._identity_for(index, session_id, paths[0]) + records = self._without_inherited_prefix( + list(self._decode_records(identity.path)), + identity.inherited_prefix_records, + ) + return _token_totals_from_records(records) + def _rollout_paths(self, sessions_root: Path) -> list[Path]: # Recursive discovery supports both current date shards and old flat # layouts. archived_sessions is deliberately outside this v1 root. @@ -386,8 +716,22 @@ def _rollout_paths(self, sessions_root: Path) -> list[Path]: return sorted(paths) def _session_index(self, sessions_root: Path) -> dict[str, list[Path]]: + """The thread-id → paths index, memoized per resolved root. + + Kept as the narrow accessor its callers expect; :meth:`_index_for` is + the whole memoized record. + """ + return self._index_for(sessions_root).paths + + def _index_for(self, sessions_root: Path) -> _SessionIndex: # Memoized per resolved root: discovery and every per-session load in a # wholesale run share one index build (see ``_index_cache``). + # + # Keyed by the RESOLVED root, which is load-bearing rather than tidy: an + # inherited prefix is only meaningful against the sibling set it was + # computed within, so a prefix found under one root must never be + # answered to a lookup under another. ``load_session_from_path`` has no + # sibling set at all and correctly never consults this. try: cache_key = sessions_root.resolve() except OSError: @@ -395,10 +739,13 @@ def _session_index(self, sessions_root: Path) -> dict[str, list[Path]]: cached = self._index_cache.get(cache_key) if cached is not None: return cached - index: dict[str, list[Path]] = {} + index = _SessionIndex(paths={}, headers={}, resolved={}) for path in self._rollout_paths(sessions_root): identity = self._read_identity(path) - index.setdefault(identity.thread_id, []).append(path) + index.paths.setdefault(identity.thread_id, []).append(path) + # First path wins, matching discovery's retain-the-first rule; the + # paths are already sorted, so "first" is deterministic. + index.headers.setdefault(identity.thread_id, identity) self._index_cache[cache_key] = index return index @@ -411,12 +758,28 @@ def _with_inherited_prefix( parent_paths = index.get(parent_id, []) if parent_id else [] if len(parent_paths) != 1: return identity - child_records = self._prefix_candidates( - list(self._decode_records(identity.path)) - ) parent_records = self._prefix_candidates( list(self._decode_records(parent_paths[0])) ) + return self._prefix_against(identity, parent_records) + + def _prefix_against( + self, + identity: CodexSessionIdentity, + parent_records: list[_DecodedRecord], + ) -> CodexSessionIdentity: + """Resolve *identity*'s inherited prefix against already-decoded parent + records. + + Split out of :meth:`_with_inherited_prefix` so discovery can decode a + shared parent once and measure every one of its children against it. + The wrapper stays the entry point for a standalone lookup that never ran + discovery, which keeps the grouped path a shortcut rather than the only + correct path. + """ + child_records = self._prefix_candidates( + list(self._decode_records(identity.path)) + ) prefix_length = self._contiguous_prefix_length(child_records, parent_records) if prefix_length == 0 and identity.spawn_call_id: boundaries = [ diff --git a/dev-docs/tools-coverage.md b/dev-docs/tools-coverage.md index d653ae96..6901aced 100644 --- a/dev-docs/tools-coverage.md +++ b/dev-docs/tools-coverage.md @@ -267,6 +267,65 @@ separators, `ALL_TOOLS`-registry pipelines (no static membership yet), and cap/engine failures remain `ToolExecution`. False negatives are an acceptable compatibility cost; false reconstruction is not. +### Token accounting + +Codex records token usage as cumulative `token_count` events +(`payload.info.total_token_usage`), one emitted after nearly every agent-loop +step — unlike Claude, which carries per-assistant-message `usage`. The provider +surfaces these totals in two places: the **project-card token summary** on the +wholesale index, and the **per-session token fields on the session cache** +(`total_input_tokens` etc., which `sessions` has carried since the initial +schema — so this is Claude-schema parity, not Codex-only state). Per-session +token **rows on the index are deliberately NOT shown** — the Claude index does +not render them either (the project-summary session dicts carry no +`token_summary` key, and the drift pin +`test_index_summary_dict_shape_matches_claude_path` locks the two shapes +together), so adding them is a Claude-path UX change deferred to its own +follow-up (see `work/codex-backlog.md`). The extraction is: + +- `providers/base.ProviderTokenTotals` — the token figures a session-level + provider surfaces: billable `input`, `cache_read`, `output`, and the + authoritative `total`. `cache_creation` is deliberately absent (Codex has no + such concept; an omitted column ≠ a zero one), so of the index's four token + columns the project card shows three and never a "Cache Creation" one. +- `codex._token_totals_from_records` — takes the **last** cumulative record as + the session total (the values are cumulative and monotonic, so the final one + subsumes every prior turn; compaction lowers the live context window but does + not reset the counter). Returns `None` — totals **omitted, not zeroed** — + for pre-accounting rollouts with no `token_count`. +- `codex._map_cumulative_usage` — maps `input = input_tokens − cached`, + `cache_read = cached_input_tokens`, `output = output_tokens` (already + includes reasoning). The subtraction keeps the cached tokens in exactly one + column; folding them back into `input` would double-count. `total_tokens` is + carried through authoritatively (never recomputed — a degenerate record with + zero components but a non-zero total keeps its stored total). +- `converter.render_provider_wholesale` sums each session's cumulative total + into the index **project-card** totals (`_sum_provider_token_totals`) and + writes the per-session totals onto the **session cache**, **bypassing** the + per-message `usage` accumulators (`compute_session_data` / + `compute_project_aggregates`) that a cumulative figure must never flow + through. + +**Why only session/project granularity — an evidenced design limit, not a +TODO.** The argument is structural, not statistical. A `token_count` delta +(`last_token_usage`) measures everything consumed since the *previous* +`token_count`, and one agent-loop step bundles reasoning + assistant text + a +tool call + its (often large, cached) tool result under a single delta. That +window contains more than one rendered thing, so the delta cannot be +attributed to any single message the transcript renders — no matter which +record the step happens to end on. The corpus distribution only *confirms* that +steps overwhelmingly end on tool work: measured post-inherited-prefix-strip +(the records that actually render), n=4138 events across 34 sessions, ~75.6% of +`token_count` events follow a tool-execution step (`custom_tool_call_output`, +`mcp_tool_call_end`, `function_call_output`, `patch_apply_end`, +`web_search_end`, `sub_agent_activity`) and ~22.5% follow an assistant/agent +message — but even that 22.5% is not attributable, because the message shares +its delta with the reasoning before it and the next turn's cached context +re-read. Per-message (and even per-turn) attribution is therefore not +recoverable from this stream, so the session cumulative is the finest honest +unit. Account-level fields in the same payload (`rate_limits`, +`model_context_window`) are never read into output. + ### Refreshing the Codex census The Codex manual states that generated app-server schemas match the installed diff --git a/test/test_codex_decode_once.py b/test/test_codex_decode_once.py new file mode 100644 index 00000000..94f59ba2 --- /dev/null +++ b/test/test_codex_decode_once.py @@ -0,0 +1,354 @@ +"""Asking a provider for token totals must cost no extra rollout decodes. + +Entries and cumulative token totals both come from a rollout's decoded +records. When the walker asked the provider for them separately, the totals +call repeated the whole resolution — index lookup, identity, decode, +inherited-prefix strip — and threw the records away again. On a real 34-rollout +archive that was 354 decodes where 236 sufficed: +118 decodes and +478 MB +re-parsed to recompute what the first pass had already produced. + +That byte figure is the number of bytes actually handed to the parser. An +earlier version of it said "+636 MB", which came from `Σ(file size × decode +calls)` — a metric that bills *every* call as a full file read. It is not one: +the decoder streams lazily and `_read_identity` returns on the first record, so +102 of those 354 calls read a single line and were charged a whole file. On this +corpus that inflates the total by 1.59x. Both are quoted in full where the +figures are recorded; the parsed number is the honest one. + +The property under test is deliberately *not* "the output is correct". +**Correct output is compatible with arbitrarily redundant decoding** — that is +precisely why the redundancy survived a green suite — so these tests count the +primitive instead. + +It is equally deliberately not "once per distinct path", because that is not +this seam's claim: what this change owns is the *delta the totals seam adds*, +so that delta — measured as a two-arm comparison against the base-class +default — is what is pinned. Pinning a whole-pipeline figure here would make +this test fail for reasons the seam never claimed to fix. + +The rest of that pipeline — discovery's own decodes and a fork re-decoding its +parent, 202 of the 320 redundant decodes in that archive — is no longer +pre-existing: it is fixed by the two commits that follow this one, and pinned +separately in ``test_codex_fork_prefix_decodes.py``. The scope note above still +describes what *this* test measures; it no longer describes the state of the +provider. + +The second test guards the trap that a decode count cannot see. Totals must +stay subject to the same date filter as the messages they accompany: a session +emptied by ``--from-date`` contributes no messages and must contribute no +tokens either. Hoisting the totals out of that filter would remove decodes +*and* silently change project totals — a behaviour change wearing a +performance fix's clothing. +""" + +import json +from collections import Counter +from pathlib import Path +from typing import Iterator, Optional + +from claude_code_log.converter import render_provider_wholesale +from claude_code_log.providers import codex as codex_module +from claude_code_log.providers.base import ( + BaseProvider, + LoadedSession, + ProviderTokenTotals, +) +from claude_code_log.providers.codex import CodexProvider, _DecodedRecord + +_CWD = "/proj/decode" + + +def _rollout(tmp: Path, name: str, thread_id: str, day: str, steps: int = 2) -> Path: + """A minimal rollout on ``day`` (YYYY-MM-DD) carrying cumulative totals. + + ``day`` is a parameter because the date-filter test needs two sessions on + *different* days; a builder with a fixed timestamp could not express the + case it has to pin. + """ + records: list[dict[str, object]] = [ + { + "timestamp": f"{day}T00:00:00Z", + "type": "session_meta", + "payload": { + "id": thread_id, + "timestamp": f"{day}T00:00:00Z", + "cwd": _CWD, + }, + }, + { + "timestamp": f"{day}T00:00:01Z", + "type": "event_msg", + "payload": {"type": "user_message", "message": f"hi {thread_id[:4]}"}, + }, + ] + for i in range(steps): + records.append( + { + "timestamp": f"{day}T00:00:{2 + i:02d}Z", + "type": "event_msg", + "payload": {"type": "agent_message", "message": f"step {i}"}, + } + ) + cumulative = 100 * (i + 1) + records.append( + { + "timestamp": f"{day}T00:00:{2 + i:02d}Z", + "type": "event_msg", + "payload": { + "type": "token_count", + "info": { + "total_token_usage": { + "input_tokens": cumulative, + "cached_input_tokens": 0, + "output_tokens": cumulative, + "reasoning_output_tokens": 0, + "total_tokens": 2 * cumulative, + }, + "model_context_window": 258400, + }, + }, + } + ) + path = tmp / f"rollout-{day}T00-00-00-{name}.jsonl" + path.write_text("\n".join(json.dumps(r) for r in records) + "\n", encoding="utf-8") + return path + + +def _render_counting_decodes( + root: Path, out: Path, *, stub_totals: bool = False +) -> Counter: + """Run a wholesale render with ``_decode_records`` counted per path. + + Wraps the provider primitive at class level (the attach point used when + these figures were first measured), so the count is comparable to the + recorded baseline. + + ``stub_totals`` makes the provider report no session totals — the + base-class default, not an approximation of one — which is the second arm + of the comparison. + + It stubs **both** totals entry points, and that is load-bearing rather than + belt-and-braces: stubbing only the combined seam makes the comparison + vacuous the moment the walker stops using it, because both arms then take + the same two-call path and trivially agree. Disabling totals at the + provider level instead means arm B is "this provider records no totals" + however the walker asks — which is the property the arms are meant to + differ by. (Verified: with only the combined seam stubbed, reverting the + walker to two calls left this test GREEN.) + """ + counts: Counter = Counter() + original = CodexProvider._decode_records + original_combined = CodexProvider.load_session_with_totals + original_totals = CodexProvider.session_token_totals + + # The stubs are annotated to match what they shadow, rather than suppressed: + # a stub whose signature has drifted from the real method would silently stop + # exercising the same call, which is the failure mode these arms exist to + # avoid. Type-compatibility here is part of the test, not lint appeasement. + def wrapped(self, path: Path) -> Iterator[_DecodedRecord]: + counts[str(path)] += 1 + return iter(list(original(self, path))) + + def no_combined( + self, + root: Path, + session_id: str, + max_messages: Optional[int] = None, + ) -> LoadedSession: + return LoadedSession( + entries=list(self.load_session_under(root, session_id, max_messages)), + token_totals=None, + ) + + def no_totals(self, root: Path, session_id: str) -> Optional[ProviderTokenTotals]: + return None + + # ty: assigning a plain function to a method attribute is flagged as + # implicit shadowing even when the signature matches exactly (verified: + # ty prints both sides identically and still errors). The signatures ARE + # mirrored deliberately -- see the comment above. + codex_module.CodexProvider._decode_records = wrapped # ty: ignore[invalid-assignment] + if stub_totals: + codex_module.CodexProvider.load_session_with_totals = no_combined # ty: ignore[invalid-assignment] + codex_module.CodexProvider.session_token_totals = no_totals # ty: ignore[invalid-assignment] + try: + render_provider_wholesale("codex", root, out, use_cache=False, silent=True) + finally: + codex_module.CodexProvider._decode_records = original + codex_module.CodexProvider.load_session_with_totals = original_combined + codex_module.CodexProvider.session_token_totals = original_totals + return counts + + +def test_token_totals_cost_no_extra_decodes(tmp_path: Path) -> None: + """Asking for token totals must cost **zero** additional rollout decodes. + + The measurable property of this change, stated as the two-arm comparison + the corpus measurement used: render once with the totals seam live, render + again with it stubbed to the base-class default (``None`` — the behaviour + of every provider that records no session totals), and require the decode + counts to be *equal*, per file and in total. + + Deliberately NOT "once per distinct path" — not because that end state is + unreachable, but because it is a different claim with a different owner + (``test_codex_fork_prefix_decodes.py``, added with the commits that follow). + Asserting a whole-pipeline figure here would make this test fail for reasons + the seam never claimed to fix, and it would then be weakened by someone who + could not tell which part was load-bearing. What this change owns is the + *delta*, so the delta is what it pins. + + Three sessions, so an accidental equality on a single file cannot carry it. + """ + root = tmp_path / "sessions" + root.mkdir() + for i in (1, 2, 3): + _rollout( + root, f"s{i}", f"1000000{i}-0000-4000-8000-00000000000{i}", "2026-01-02" + ) + + with_totals = _render_counting_decodes(root, tmp_path / "out-a") + without_totals = _render_counting_decodes( + root, tmp_path / "out-b", stub_totals=True + ) + + assert len(with_totals) == 3, f"expected 3 rollouts, saw {sorted(with_totals)}" + assert sum(with_totals.values()) == sum(without_totals.values()), ( + f"totals cost {sum(with_totals.values()) - sum(without_totals.values())} " + f"extra decodes: with={dict(with_totals)} without={dict(without_totals)}" + ) + assert with_totals == without_totals, ( + "per-file decode counts differ between the two arms: " + f"with={dict(with_totals)} without={dict(without_totals)}" + ) + + +def test_override_matches_base_across_max_messages(tmp_path: Path) -> None: + """The override must be interchangeable with the base implementation. + + That interchangeability is the entire argument for this seam's shape — the + base IS the pair of calls the walker used to make, so a provider that does + not override cannot change behaviour. The claim is only worth anything if + the provider that DOES override agrees with it, and review found two inputs + where it did not: + + * ``max_messages<=0`` returned ``token_totals=None`` for a session that has + totals, because the override skipped the totals with the entries. The base + reports them: its ``load_session_under`` returns early while + ``session_token_totals`` still runs. + * resolving eagerly on that path raised ``FileNotFoundError`` for an unknown + id where the base returns empty, because the base never resolves there. + + **DO NOT rewrite this as a caller-driven test.** The subject under test is + the *pair* of implementations, not either side reached through a caller — + which is the general shape for any "override must be interchangeable with + base" claim. The walker never passes ``max_messages<=0``, so no amount of + driving it can reach the input. + + That is verified, not assumed: with the ``token_totals=None`` defect restored, + a full wholesale render still shows the totals and a caller-driven test + **passes**, while this test fails. So "simplifying" it into a render-and-assert + would silently stop covering anything. + + Both wrong versions fail this test on *different* assertions — the ``None`` + variant at ``max_messages=0``, the eager-resolve variant on the unknown id — + which is what shows it discriminates between them rather than merely + reddening when something is off. + """ + root = tmp_path / "sessions" + root.mkdir() + _rollout(root, "one", "30000001-0000-4000-8000-000000000001", "2026-01-02") + provider = CodexProvider() + sid = "30000001-0000-4000-8000-000000000001" + + def outcome(fn): + try: + loaded = fn() + return ("ok", len(loaded.entries), loaded.token_totals) + except Exception as exc: # noqa: BLE001 - the exception type IS the result + return (type(exc).__name__,) + + for max_messages in (None, 0, -1, 5): + assert outcome( + lambda: provider.load_session_with_totals(root, sid, max_messages) + ) == outcome( + lambda: BaseProvider.load_session_with_totals( + provider, root, sid, max_messages + ) + ), f"override diverges from base at max_messages={max_messages}" + + # An id the index does not contain: the base never resolves on the + # no-entries path, so it must not raise there and neither may the override. + assert outcome( + lambda: provider.load_session_with_totals( + root, "30000009-0000-4000-8000-000000000009", 0 + ) + ) == outcome( + lambda: BaseProvider.load_session_with_totals( + provider, root, "30000009-0000-4000-8000-000000000009", 0 + ) + ), "override diverges from base for an unknown id with no entries requested" + + +def test_filtered_out_session_contributes_no_tokens(tmp_path: Path) -> None: + """A session removed by the date filter must contribute neither messages + nor tokens. + + The totals travel with the entries through the same survival test. If they + were collected before it — the obvious way to fetch totals once per + session — the filtered-out session's tokens would still land in the project + card, and the decode count would look *better* while the numbers got worse. + + Two sessions on different days, filtering out exactly one, so the fixture + can express the difference: with both included the project total is the sum + of two sessions, and the assertion below would hold vacuously if the filter + removed nothing. + + Mutation notes, because the obvious mutation does *not* discriminate here: + moving the totals collection above the ``if messages:`` gate alone leaves + this test GREEN, since the survival test is enforced where the totals are + *consumed* (the comprehension over ``loaded``), not where they are + collected. The mutation that reddens it — and the actual failure mode — is + hoisting the collection **and** summing every collected total instead of + the survivors'. If you move that gate, this test is what should catch you. + """ + root = tmp_path / "sessions" + root.mkdir() + _rollout(root, "keep", "20000001-0000-4000-8000-000000000001", "2026-01-05") + _rollout(root, "drop", "20000002-0000-4000-8000-000000000002", "2026-01-02") + + both = tmp_path / "out-both" + index_both = render_provider_wholesale( + "codex", root, both, use_cache=False, silent=True + ) + html_both = index_both.read_text(encoding="utf-8") + + filtered = tmp_path / "out-filtered" + index_filtered = render_provider_wholesale( + "codex", + root, + filtered, + from_date="2026-01-04", + use_cache=False, + silent=True, + ) + html_filtered = index_filtered.read_text(encoding="utf-8") + + # Sanity: the filter actually removed a session, or the totals assertion + # below would pass for the wrong reason. + assert "20000002" not in html_filtered + assert "20000002" in html_both + + # Each session's last cumulative is input 200 / output 200, so the project + # card reads 400/400 with both sessions and must read 200/200 once one is + # filtered out. (No "Cache Read" component: it is zero, and this renderer + # OMITS a zero component rather than printing it — so asserting on it would + # fail for a reason unrelated to the property under test.) + assert "Input: 400 | Output: 400" in html_both, ( + "expected the two-session project total" + ) + assert "Input: 200 | Output: 200" in html_filtered, ( + "filtered render should show only the surviving session's totals" + ) + assert "Input: 400 | Output: 400" not in html_filtered, ( + "filtered-out session is still contributing to project totals" + ) diff --git a/test/test_codex_fork_prefix_decodes.py b/test/test_codex_fork_prefix_decodes.py new file mode 100644 index 00000000..871e417a --- /dev/null +++ b/test/test_codex_fork_prefix_decodes.py @@ -0,0 +1,288 @@ +"""Discovery already knows each fork's inherited prefix; loading must not +recompute it, and a shared parent must not be decoded once per child. + +Two redundancies, measured on a 34-rollout archive as 132 of 236 decodes: + +* the load path recomputed ``inherited_prefix_records`` that discovery had + already computed and published, re-decoding both the child and its parent to + do it; and +* ``_with_inherited_prefix`` decoded a parent once per child, so a parent shared + by 12 forks was fully decoded 12 times in one discovery pass. + +As with the totals seam, **correct output is compatible with arbitrarily +redundant decoding** — which is why this survived a green suite for as long as +it did — so these tests count the primitive rather than checking the rendering. + +They also pin the two ways the reduction can be wrong rather than slow, both of +which are silent: + +* a duplicated thread id is *retained* by discovery but *illegal* to load, so + the index holds an identity for an id whose load must raise. The ambiguity + check has to stay ahead of the fast path. +* ``inherited_prefix_records == 0`` cannot be distinguished from "not yet + computed" if membership is inferred from the value, and 0 is the common case. + Fork children whose prefix is genuinely 0 would then be recomputed forever — + a fix that silently underdelivers with nothing red. +""" + +import json +from collections import Counter +from pathlib import Path +from typing import Any, Iterator, Optional + +import pytest + +from claude_code_log.converter import render_provider_wholesale +from claude_code_log.providers import codex as codex_module +from claude_code_log.providers.codex import CodexProvider, _DecodedRecord + +_CWD = "/proj/fork" +_DAY = "2026-03-04" + + +def _meta(thread_id: str, parent: Optional[str] = None) -> dict[str, Any]: + payload: dict[str, Any] = { + "id": thread_id, + "timestamp": f"{_DAY}T00:00:00Z", + "cwd": _CWD, + } + if parent is not None: + payload["parent_thread_id"] = parent + return { + "timestamp": f"{_DAY}T00:00:00Z", + "type": "session_meta", + "payload": payload, + } + + +def _msg(text: str) -> dict[str, Any]: + """A record identified by its payload alone. + + ``_same_semantic_record`` compares kind and payload and ignores the envelope + timestamp, so a shared record must carry an identical payload — the same + text — while the timestamp is free to differ. + """ + return { + "timestamp": f"{_DAY}T00:00:01Z", + "type": "event_msg", + "payload": {"type": "agent_message", "message": text}, + } + + +def _write(root: Path, name: str, records: list[dict[str, Any]]) -> Path: + path = root / f"rollout-{_DAY}T00-00-00-{name}.jsonl" + path.write_text("\n".join(json.dumps(r) for r in records) + "\n", encoding="utf-8") + return path + + +def _tid(n: int) -> str: + return f"4000000{n}-0000-4000-8000-00000000000{n}" + + +def _fork_tree(root: Path, child_count: int) -> tuple[Path, list[Path]]: + """A parent with ``child_count`` forks inheriting its tail. + + The parent's candidate records are ``[a, b, c]``. + ``_contiguous_prefix_length`` only accepts a run reaching the **end** of the + parent and at least 2 long, so a child inherits by *starting* with the + parent's tail. Children deliberately inherit **different** amounts (2 then + 3, alternating), because a grouping bug that hands every child the same + parent-derived number would pass a fixture where they all agree. + """ + root.mkdir(parents=True, exist_ok=True) + parent_id = _tid(1) + parent = _write(root, "parent", [_meta(parent_id), _msg("a"), _msg("b"), _msg("c")]) + children: list[Path] = [] + for i in range(child_count): + inherited = ( + [_msg("b"), _msg("c")] if i % 2 == 0 else [_msg("a"), _msg("b"), _msg("c")] + ) + child_id = _tid(i + 2) + children.append( + _write( + root, + f"child{i}", + [_meta(child_id, parent=parent_id), *inherited, _msg(f"own-{i}")], + ) + ) + return parent, children + + +def _decodes_per_path(root: Path, out: Path) -> Counter: + """Per-path ``_decode_records`` counts across one wholesale render.""" + counts: Counter = Counter() + original = CodexProvider._decode_records + + def wrapped(self, path: Path) -> Iterator[_DecodedRecord]: + counts[Path(path).name] += 1 + return iter(list(original(self, path))) + + codex_module.CodexProvider._decode_records = wrapped # ty: ignore[invalid-assignment] + try: + render_provider_wholesale("codex", root, out, use_cache=False, silent=True) + finally: + codex_module.CodexProvider._decode_records = original + return counts + + +def test_load_does_not_recompute_the_inherited_prefix(tmp_path: Path) -> None: + """Every rollout is decoded a bounded number of times, none of them a + recomputation of what discovery published. + + The per-path budget after the fix, asserted as an exact map because it is + deterministic: + + * 1 header read in the index build (early exit, not a full materialisation) + * 1 full decode during discovery for a fork child, measured against its + parent's records + * 1 full decode during discovery for a rollout that *is* a parent + * 1 full decode for the session's own load + + So a plain fork child is 3. Before this change the same child cost 6: the + load path re-resolved the identity — decoding the child *and* its parent + again — and then decoded the child a third time for its records. + + The equality is deliberately exact rather than an upper bound, and it earns + that in both directions: a count that is too *low* means a child's prefix + comparison never happened. Verified — an implementation that resolves one + child per parent group and copies the answer to its siblings drops those + siblings to 2, and this test catches it there as well as at 6. + + Asserted over the *children* only. The shared parent's count is not a + property of this change: it still scales with the fan-out until discovery + groups by parent, which + :func:`test_shared_parent_decode_count_is_independent_of_child_count` owns. + Asserting it here would make this test fail for a reason it does not fix. + """ + root = tmp_path / "sessions" + _parent, children = _fork_tree(root, 2) + + counts = _decodes_per_path(root, tmp_path / "out") + + for child in children: + assert counts[child.name] == 3, f"child {child.name}: {dict(counts)}" + + +def test_duplicate_thread_id_still_raises_after_discovery(tmp_path: Path) -> None: + """The ambiguity check stays ahead of the resolved-identity lookup. + + Discovery *retains* the lexicographically first rollout for a duplicated + thread id and warns; loading that id must *raise*. So the index legitimately + holds a usable identity for an id that is illegal to load, and consulting it + before the ambiguity test would quietly load the first rollout instead. + + Discovery runs first here deliberately — that is what populates the map and + makes the fast path available to be wrongly taken. Without the preceding + discovery this test passes whatever the ordering, which is exactly the + weakening to guard against. + """ + root = tmp_path / "sessions" + root.mkdir() + duped = _tid(1) + _write(root, "first", [_meta(duped), _msg("a"), _msg("b")]) + _write(root, "second", [_meta(duped), _msg("c"), _msg("d")]) + + provider = CodexProvider() + discovered = list(provider.discover_sessions_under(root)) + assert len(discovered) == 1, "discovery should retain exactly one of the two" + + with pytest.raises(ValueError, match="Multiple Codex rollouts have thread id"): + list(provider.load_session_under(root, duped)) + + +def test_zero_prefix_fork_is_not_recomputed(tmp_path: Path) -> None: + """A computed prefix of **0** must count as computed. + + This is the fixture the obvious implementation gets wrong. If membership in + the resolved map is inferred from ``inherited_prefix_records > 0`` instead of + from presence, every session whose prefix is 0 falls back to the slow path — + and 0 is the common case. + + It has to be a fork child with a *uniquely resolvable parent* and no shared + tail. A plain non-fork session cannot detect the defect: with no parent, + ``_with_inherited_prefix`` returns before decoding anything, so recomputing + it costs zero decodes and is invisible. Here the recomputation decodes the + child and the parent again, taking both from 3 to 4. + """ + root = tmp_path / "sessions" + root.mkdir() + parent_id, child_id = _tid(1), _tid(2) + parent = _write(root, "parent", [_meta(parent_id), _msg("a"), _msg("b")]) + # Shares nothing with the parent's tail, so the prefix is a genuine 0. + child = _write( + root, "child", [_meta(child_id, parent=parent_id), _msg("x"), _msg("y")] + ) + + provider = CodexProvider() + prefixes = { + info.session_id: getattr(info, "inherited_prefix_records", None) + for info in provider.discover_sessions_under(root) + } + assert prefixes[child_id] == 0, ( + "fixture must produce a genuine zero prefix, not a missing parent" + ) + + counts = _decodes_per_path(root, tmp_path / "out") + assert counts[child.name] == 3, ( + f"zero-prefix child was recomputed at load: {dict(counts)}" + ) + assert counts[parent.name] == 3, ( + f"parent re-decoded for a zero-prefix child's recomputation: {dict(counts)}" + ) + + +def test_shared_parent_decode_count_is_independent_of_child_count( + tmp_path: Path, +) -> None: + """A parent shared by *k* forks is decoded once, not *k* times. + + Stated as invariance to *k* rather than as a magic number: the ungrouped + path decodes the parent once per child, so its count *scales* with the fan + out (4 at k=2, 6 at k=4), while grouping leaves it flat. Invariance is the + property; the absolute value is asserted too, so a regression that makes + both trees equally bad still fails. + + The absolute 3 is 1 header read (``_read_identity`` during the index build, + which early-exits on the first record) + 1 full decode as the shared parent + during discovery + 1 full decode for the parent's own load. + """ + small, _ = _fork_tree(tmp_path / "k2", 2) + large, _ = _fork_tree(tmp_path / "k4", 4) + + counts_small = _decodes_per_path(tmp_path / "k2", tmp_path / "out-k2") + counts_large = _decodes_per_path(tmp_path / "k4", tmp_path / "out-k4") + + assert counts_small[small.name] == counts_large[large.name], ( + "parent decode count scales with the number of children: " + f"k=2 -> {counts_small[small.name]}, k=4 -> {counts_large[large.name]}" + ) + assert counts_small[small.name] == 3, ( + f"expected 1 header + 1 as shared parent + 1 own load, got " + f"{counts_small[small.name]} ({dict(counts_small)})" + ) + + +def test_grouped_discovery_keeps_per_child_prefix_values(tmp_path: Path) -> None: + """Grouping must not smear one child's prefix across its siblings. + + The children inherit *different* amounts (2 and 3 records), so a grouped + implementation that computed the prefix once per parent — or reused the + previous child's answer — would be caught here rather than in a rendering + diff. This is the correctness half of the same change whose cost the tests + above measure. + """ + root = tmp_path / "sessions" + _fork_tree(root, 4) + provider = CodexProvider() + + by_id = { + info.session_id: getattr(info, "inherited_prefix_records", None) + for info in provider.discover_sessions_under(root) + } + + assert by_id[_tid(1)] == 0, "the parent inherits nothing" + # children alternate: index 0,2 inherit 2 records; index 1,3 inherit 3 + assert by_id[_tid(2)] == 2, by_id + assert by_id[_tid(3)] == 3, by_id + assert by_id[_tid(4)] == 2, by_id + assert by_id[_tid(5)] == 3, by_id diff --git a/test/test_codex_token_accounting.py b/test/test_codex_token_accounting.py new file mode 100644 index 00000000..418cf60c --- /dev/null +++ b/test/test_codex_token_accounting.py @@ -0,0 +1,786 @@ +"""Codex token accounting — session/project totals from ``token_count`` events. + +Codex records token usage as *cumulative* ``token_count`` events (one after +nearly every agent-loop step), not as per-assistant-message ``usage`` the way +Claude does. This module pins the extraction and the wholesale/index threading: + +- the mapping onto the index's four token columns (THE subtraction that must + stay disjoint, or index totals double-count the cache-read column), +- "session total = the LAST cumulative record", never a sum of the per-step + deltas (the double-count main flagged), holding across compaction, +- the degenerate-record rule (zero components, non-zero total → stored total + is authoritative, never recomputed), +- totals OMITTED (``None``) — not zeroed — for pre-accounting sessions, +- the wholesale path populating project-card and per-session-row totals while + never leaking account-level ``rate_limits`` data into output. + +Each test is written so neutering its target guard turns it RED (e.g. folding +``cached`` back into ``input`` breaks ``test_map_subtraction_is_disjoint``). +""" + +import json +from pathlib import Path +from typing import Any, Iterator, Optional + +from claude_code_log.converter import ( + _sum_provider_token_totals, + render_provider_wholesale, +) +import pytest + +from claude_code_log.models import TranscriptEntry +from claude_code_log.providers.base import ( + BaseProvider, + ProviderTokenTotals, + SessionInfo, +) +from claude_code_log.providers.codex import ( + CodexProvider, + _DecodedRecord, + _map_cumulative_usage, + _token_totals_from_records, +) + + +# -------------------------------------------------------------------------- +# Fixture builders +# -------------------------------------------------------------------------- +def _usage( + input_tokens: int, + cached: int, + output: int, + reasoning: int, + total: int, +) -> dict[str, int]: + return { + "input_tokens": input_tokens, + "cached_input_tokens": cached, + "output_tokens": output, + "reasoning_output_tokens": reasoning, + "total_tokens": total, + } + + +def _token_count_record(total_usage: dict[str, int], ts: str) -> dict[str, object]: + """A ``token_count`` event carrying a cumulative ``total_token_usage`` plus + the account-level ``rate_limits`` block a real rollout attaches (so the + no-leak pin exercises the real shape).""" + return { + "timestamp": ts, + "type": "event_msg", + "payload": { + "type": "token_count", + "info": { + "total_token_usage": total_usage, + "last_token_usage": total_usage, + "model_context_window": 258400, + }, + "rate_limits": { + "limit_id": "codex", + "plan_type": "team", + "primary": {"used_percent": 92.0, "resets_at": 1786983340}, + }, + }, + } + + +def _rollout_with_tokens( + tmp: Path, + rel: str, + thread_id: str, + cwd: str | None, + token_totals: list[dict[str, Any]], +) -> Path: + """Write a rollout whose token_count events carry the given cumulative + totals in order (the LAST one is the session total). + + Values are ``Any``, not ``int``: the malformed-record tests deliberately + feed a string / bool / absent ``total_tokens`` to exercise the skip, and a + fixture builder that could only express well-formed input could not + express the bug being pinned. + """ + payload: dict[str, object] = {"id": thread_id, "timestamp": "2026-01-02T00:00:00Z"} + if cwd is not None: + payload["cwd"] = cwd + records: list[dict[str, object]] = [ + { + "timestamp": "2026-01-02T00:00:00Z", + "type": "session_meta", + "payload": payload, + }, + { + "timestamp": "2026-01-02T00:00:01Z", + "type": "event_msg", + "payload": {"type": "user_message", "message": f"hi {thread_id[:4]}"}, + }, + ] + for i, tu in enumerate(token_totals): + records.append( + { + "timestamp": "2026-01-02T00:00:02Z", + "type": "event_msg", + "payload": {"type": "agent_message", "message": f"step {i}"}, + } + ) + # Zero-padded: an unpadded f"…:0{3 + i}" emits "00:00:010Z" from the + # 7th record on, and this fixture is load-bearing for tests that + # assert on the literal timestamp. + records.append(_token_count_record(tu, f"2026-01-02T00:00:{3 + i:02d}Z")) + rel_path = Path(rel) + path = tmp / rel_path.parent / f"rollout-{rel_path.name}" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("\n".join(json.dumps(r) for r in records) + "\n", encoding="utf-8") + return path + + +def _decoded(records_source: Path) -> list[_DecodedRecord]: + """Decoded records, typed as what ``_decode_records`` yields. + + Returning ``list[object]`` forced an ``arg-type`` suppression at every call + site, which meant the type checkers could not verify the very contract these + tests exist to pin. + """ + return list(CodexProvider()._decode_records(records_source)) + + +# -------------------------------------------------------------------------- +# _map_cumulative_usage — the mapping onto index columns +# -------------------------------------------------------------------------- +def test_no_token_field_accepts_a_boolean() -> None: + """``bool`` is an ``int`` subclass, so a JSON ``true`` must not become a 1 + in any token column. + + Raised in review for the *component* fields after the same exclusion had + been added for ``total_tokens`` only — the record-selection guard rejects a + boolean total, but a record with a well-formed total and a boolean + ``input_tokens`` reaches the mapping untouched. One supplier fixed, the + others left green: fixing only the field named in the report would repeat + exactly that. + + So the loop derives its field list from the usage dict itself rather than + naming fields inline. Add a component to ``_usage``/the mapping and it is + covered here automatically; an inline list of four names would silently + stop covering the fifth. + + ``reasoning_output_tokens`` is the one field a boolean cannot perturb, and + that is by design rather than a gap: the mapping never reads it (``output`` + already subsumes reasoning — see + :func:`test_map_output_includes_reasoning_not_added`), so True, False and + absent are all identical. If that ever stops being true, this loop starts + covering it without being edited. + """ + # THE CACHE MUST BE ZERO HERE. `input = max(input_tokens - cached, 0)`, so + # with a non-zero cache an accepted `True` (== 1) clamps to 0 — exactly what + # a correctly-rejected boolean also produces — and the field becomes + # unobservable. Two earlier versions of this test were vacuous for + # `input_tokens`, the field the report actually named: first by searching the + # output for a literal 1, then by comparing True against 0, which are + # indistinguishable once both clamp. Verified by mutation: with cached=20 the + # broken mapping was caught only on `cached_input_tokens`. + baseline = _usage(100, 0, 10, 0, 110) + + # Assert the boolean is treated as the malformed value it is — identical to + # the field being absent — rather than hunting for the wrong value in the + # output. This holds per field, so no field can hide behind another. + for field in baseline: + with_bool = _map_cumulative_usage({**baseline, field: True}) + with_absent = _map_cumulative_usage( + {k: v for k, v in baseline.items() if k != field} + ) + assert with_bool == with_absent, ( + f"boolean in {field!r} was not treated as malformed: " + f"{with_bool} != {with_absent}" + ) + + # False likewise. Same coercion, opposite value: it would silently zero a + # real column rather than inventing a 1, which no "look for a 1" assertion + # could ever see. + for field in baseline: + with_false = _map_cumulative_usage({**baseline, field: False}) + with_absent = _map_cumulative_usage( + {k: v for k, v in baseline.items() if k != field} + ) + assert with_false == with_absent, ( + f"boolean False in {field!r} was not treated as malformed" + ) + + +def test_map_subtraction_is_disjoint() -> None: + """THE double-count pin: billable input EXCLUDES the cached portion, and + cache_read is the SOLE home of the cached tokens. If a future edit folded + ``cached`` back into input (or dropped the subtraction), input+cache_read + would exceed the original input_tokens and index totals would balloon.""" + result = _map_cumulative_usage(_usage(100, 30, 50, 20, 150)) + assert result.input_tokens == 70 # 100 - 30, NOT 100 and NOT 130 + assert result.cache_read_tokens == 30 + # input + cache_read reconstructs the original input_tokens exactly — the + # cached tokens are counted once, in cache_read only. + assert result.input_tokens + result.cache_read_tokens == 100 + + +def test_map_output_includes_reasoning_not_added() -> None: + """output_tokens already subsumes reasoning_output_tokens — reasoning must + not be added a second time.""" + result = _map_cumulative_usage(_usage(100, 0, 50, 20, 150)) + assert result.output_tokens == 50 # NOT 50 + 20 + + +def test_map_reconstructs_total_for_wellformed() -> None: + """For a well-formed cumulative record the mapped columns reconstruct the + stored total: (input-cached) + cached + output == total.""" + result = _map_cumulative_usage(_usage(100, 30, 50, 20, 150)) + assert ( + result.input_tokens + result.cache_read_tokens + result.output_tokens + == result.total_tokens + == 150 + ) + + +def test_map_total_authoritative_for_degenerate_record() -> None: + """Degenerate record — every component zero but a non-zero total. The + stored total is authoritative and carried through untouched; it is NEVER + recomputed from the (zero) components down to zero.""" + result = _map_cumulative_usage(_usage(0, 0, 0, 0, 4242)) + assert result.total_tokens == 4242 + assert result.input_tokens == 0 + assert result.cache_read_tokens == 0 + assert result.output_tokens == 0 + + +def test_map_clamps_negative_input() -> None: + """A malformed record where cached > input must not yield a negative + billable-input column.""" + result = _map_cumulative_usage(_usage(10, 40, 5, 0, 15)) + assert result.input_tokens == 0 + + +def test_map_coerces_missing_fields_to_zero() -> None: + result = _map_cumulative_usage({"total_tokens": 7}) + assert result == ProviderTokenTotals( + input_tokens=0, cache_read_tokens=0, output_tokens=0, total_tokens=7 + ) + + +# -------------------------------------------------------------------------- +# _token_totals_from_records — last cumulative record wins, never a sum +# -------------------------------------------------------------------------- +def test_last_record_wins_not_sum(tmp_path: Path) -> None: + """Session total is the LAST cumulative record, NOT a sum of the per-step + records. Three monotonically-growing cumulative records → the result is the + third, not their sum.""" + path = _rollout_with_tokens( + tmp_path, + "s/one.jsonl", + "10000000-0000-4000-8000-000000000001", + "/p", + [ + _usage(100, 20, 10, 0, 110), + _usage(300, 60, 25, 0, 325), + _usage(500, 100, 40, 0, 540), # <- the session total + ], + ) + result = _token_totals_from_records(_decoded(path)) + assert result is not None + assert result.total_tokens == 540 # last record, not 110+325+540 + assert result.input_tokens == 400 # 500 - 100 + assert result.cache_read_tokens == 100 + assert result.output_tokens == 40 + + +def test_last_record_wins_across_compaction(tmp_path: Path) -> None: + """Compaction lowers the live context window but the cumulative counter + keeps climbing. Even when a mid-session record's per-step delta looks like a + reset, the LAST cumulative total_token_usage is still the session total.""" + path = _rollout_with_tokens( + tmp_path, + "s/two.jsonl", + "10000000-0000-4000-8000-000000000002", + "/p", + [ + _usage(1000, 200, 50, 10, 1050), + _usage(2000, 400, 90, 20, 2090), # pre-compaction + _usage(2100, 1800, 95, 22, 2195), # post-compaction: cumulative up + ], + ) + result = _token_totals_from_records(_decoded(path)) + assert result is not None + assert result.total_tokens == 2195 # last cumulative record, monotonic + + +def test_none_when_no_token_count(tmp_path: Path) -> None: + """A pre-accounting session (no token_count events) yields None — totals + are OMITTED, not zeroed.""" + path = _rollout_with_tokens( + tmp_path, + "s/three.jsonl", + "10000000-0000-4000-8000-000000000003", + "/p", + [], # no token_count records + ) + assert _token_totals_from_records(_decoded(path)) is None + + +def test_monotonicity_violation_omits_totals(tmp_path: Path) -> None: + """Cumulative total_tokens must never decrease. If it does (a hypothetical + future counter reset mid-session), the totals are OMITTED — not the + post-reset tail ('last', which understates) nor the pre-reset peak ('max', + which is also wrong). Fail closed: a wrong number is worse than an absent + one. Nothing in the corpus exercises this (0 violations measured), so the + guard fires only on a spec change.""" + path = _rollout_with_tokens( + tmp_path, + "s/reset.jsonl", + "10000000-0000-4000-8000-000000000004", + "/p", + [ + _usage(1000, 200, 50, 10, 1050), + _usage(2000, 400, 90, 20, 2090), + _usage(300, 60, 20, 5, 320), # DECREASE: 320 < 2090 → omit + ], + ) + assert _token_totals_from_records(_decoded(path)) is None + + +# -------------------------------------------------------------------------- +# session_token_totals — provider seam, post-strip consistency +# -------------------------------------------------------------------------- +def test_session_token_totals_matches_last_record(tmp_path: Path) -> None: + root = tmp_path / "sessions" + _rollout_with_tokens( + root, + "a/one.jsonl", + "10000000-0000-4000-8000-000000000001", + "/proj/a", + [_usage(200, 50, 20, 5, 220), _usage(400, 120, 35, 8, 435)], + ) + totals = CodexProvider().session_token_totals( + root, "10000000-0000-4000-8000-000000000001" + ) + assert totals == ProviderTokenTotals( + input_tokens=280, cache_read_tokens=120, output_tokens=35, total_tokens=435 + ) + + +def test_default_seam_returns_none() -> None: + """The base seam default is None so non-cumulative providers leave every + token surface untouched. Agy inherits the default.""" + from claude_code_log.providers.agy import AgyProvider + + assert AgyProvider().session_token_totals(Path("/nonexistent"), "whatever") is None + + +# -------------------------------------------------------------------------- +# converter helpers +# -------------------------------------------------------------------------- +def test_sum_skips_none_and_pins_cache_creation_zero() -> None: + """Summing across sessions: None (pre-accounting) sessions contribute + nothing; cache_creation stays 0 for shape parity (never displayed).""" + summed = _sum_provider_token_totals( + [ + ProviderTokenTotals(70, 30, 50, 150), + None, + ProviderTokenTotals(5, 1, 2, 8), + ] + ) + assert summed == { + "total_input_tokens": 75, + "total_output_tokens": 52, + "total_cache_creation_tokens": 0, + "total_cache_read_tokens": 31, + } + + +# -------------------------------------------------------------------------- +# Wholesale/index threading — the #296 deferral +# -------------------------------------------------------------------------- +def _render(tmp_path: Path) -> str: + root = tmp_path / "sessions" + # Two sessions in one project, one in another, one pre-accounting session. + _rollout_with_tokens( + root, + "a/one.jsonl", + "10000000-0000-4000-8000-000000000001", + "/proj/a", + [_usage(100, 20, 10, 0, 110), _usage(1000, 200, 100, 0, 1100)], + ) + _rollout_with_tokens( + root, + "a/two.jsonl", + "10000000-0000-4000-8000-000000000002", + "/proj/a", + [_usage(500, 100, 50, 0, 550)], + ) + _rollout_with_tokens( + root, + "b/one.jsonl", + "20000000-0000-4000-8000-000000000001", + "/proj/b", + [_usage(7, 2, 3, 0, 10)], + ) + _rollout_with_tokens( + root, + "c/none.jsonl", + "30000000-0000-4000-8000-000000000001", + "/proj/c", + [], # pre-accounting: no token_count + ) + out = tmp_path / "out" + index = render_provider_wholesale("codex", root, out, use_cache=True, silent=True) + return index.read_text(encoding="utf-8") + + +def test_wholesale_project_card_totals_use_last_cumulative(tmp_path: Path) -> None: + """Project /proj/a: session one's LAST cumulative is 1100 (input 800, + cache_read 200, output 100), session two is 550 (input 400, cache_read 100, + output 50). The card sums the two SESSION totals — and each session total is + its last cumulative, NOT the sum of its per-step records (else session one + would be 110+1100).""" + html = _render(tmp_path) + # /proj/a card: input 800+400=1200, output 100+50=150, cache_read 200+100=300 + assert "Input: 1200 | Output: 150 | Cache Read: 300" in html + # /proj/b card: input 5, output 3, cache_read 2 + assert "Input: 5 | Output: 3 | Cache Read: 2" in html + + +def test_wholesale_session_cache_stores_per_session_totals(tmp_path: Path) -> None: + """Per-session cumulative totals are written to the session cache (the + durable 'session totals'), each session's LAST cumulative — NOT the summed + per-message usage (which is zero for Codex) and NOT the per-step sum.""" + from claude_code_log.cache import ( + CacheManager, + get_cache_db_path, + get_library_version, + ) + + root = tmp_path / "sessions" + _rollout_with_tokens( + root, + "a/one.jsonl", + "10000000-0000-4000-8000-000000000001", + "/proj/a", + [_usage(100, 20, 10, 0, 110), _usage(1000, 200, 100, 0, 1100)], + ) + _rollout_with_tokens( + root, + "a/two.jsonl", + "10000000-0000-4000-8000-000000000002", + "/proj/a", + [_usage(500, 100, 50, 0, 550)], + ) + out = tmp_path / "out" + render_provider_wholesale("codex", root, out, use_cache=True, silent=True) + + # Locate the project dest dir by the session page it rendered, then read its + # session cache rows directly. + session_page = next(out.rglob("session-10000000-0000-4000-8000-000000000001*")) + dest = session_page.parent + cache = CacheManager(dest, get_library_version(), db_path=get_cache_db_path(out)) + project_cache = cache.get_cached_project_data() + assert project_cache is not None + cached = project_cache.sessions + one = cached["10000000-0000-4000-8000-000000000001"] + assert one.total_input_tokens == 800 # 1000 - 200, last record (not 110+1100) + assert one.total_cache_read_tokens == 200 + assert one.total_output_tokens == 100 + assert one.total_cache_creation_tokens == 0 # omitted for Codex + two = cached["10000000-0000-4000-8000-000000000002"] + assert two.total_input_tokens == 400 + assert two.total_cache_read_tokens == 100 + + +def test_wholesale_no_zero_token_line_leaks(tmp_path: Path) -> None: + """The pre-accounting project /proj/c contributes no tokens — no zero-valued + token figure leaks anywhere, and 'Cache Creation' never renders.""" + html = _render(tmp_path) + assert "Input: 0" not in html + assert "Cache Creation" not in html + + +def test_wholesale_never_leaks_account_level_data(tmp_path: Path) -> None: + """rate_limits / model_context_window live in the token_count payload but + must NEVER reach output — only the four token integers are surfaced.""" + html = _render(tmp_path) + for term in ( + "rate_limit", + "used_percent", + "resets_at", + "model_context_window", + "plan_type", + "limit_id", + ): + assert term not in html, f"account-level term leaked: {term}" + + +# -------------------------------------------------------------------------- +# Review follow-ups: malformed totals, and the asymmetric project override +# -------------------------------------------------------------------------- +# Both were raised in review on the open PR. Neither is reachable from the +# real corpus — every ``token_count`` there carries a valid int total, and the +# only non-Codex provider has no per-message usage to lose — so both need +# synthetic pins. + + +def test_malformed_total_does_not_omit_the_session( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A record whose ``total_tokens`` is absent or not an int must be SKIPPED, + not coerced to 0. + + Coercing made a data-quality problem look like a counter reset: 0 compares + less than the running total, the monotonicity guard fired, and the whole + session's totals were omitted. The session here is strictly increasing — + 110 then 1100 — with one malformed record wedged between, so nothing about + the counter is actually broken. + + Mutation-check: restore ``total = total_raw if isinstance(total_raw, int) + else 0`` (dropping the skip) and this goes RED — the guard fires on the + malformed record and ``session_token_totals`` returns None. + """ + import logging + + records = [ + _usage(100, 20, 10, 0, 110), + {"input_tokens": 500, "cached_input_tokens": 100, "output_tokens": 50}, + _usage(1000, 200, 100, 0, 1100), + ] + root = tmp_path / "sessions" + _rollout_with_tokens( + root, "a/one.jsonl", "10000000-0000-4000-8000-00000000000a", "/proj/a", records + ) + + with caplog.at_level(logging.WARNING): + totals = CodexProvider().session_token_totals( + root, "10000000-0000-4000-8000-00000000000a" + ) + + # Not omitted: the valid records still describe the session. + assert totals is not None + # And the LAST VALID record wins — the malformed one is out of last-record + # selection too, so its zero components never reach _map_cumulative_usage. + assert totals.input_tokens == 800 + assert totals.cache_read_tokens == 200 + assert totals.output_tokens == 100 + assert totals.total_tokens == 1100 + + # Degraded visibly, naming what was seen — ONE line per session (a + # pathological file must not emit thousands), but carrying the count AND + # the first offending record's timestamp, so the reader can open the + # rollout at that record instead of re-scanning it. The malformed record + # is the second token_count, at ...:04Z. + # Filter and collect through the SAME accessor. ``r.message`` is only + # populated once a formatter has run over the record, so collecting it while + # filtering on ``r.getMessage()`` can see something different from what was + # matched — and only the count is used here anyway, so collect the records. + warnings = [r for r in caplog.records if "total_tokens was" in r.getMessage()] + assert len(warnings) == 1 + assert "absent" in caplog.text + assert "skipped 1 record" in caplog.text + assert "2026-01-02T00:00:04Z" in caplog.text + # The monotonicity guard did NOT fire — that message must be absent. + assert "monotonicity broken" not in caplog.text + + +def test_malformed_total_names_the_type_it_saw( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """The warning names the actual shape, so the next surprise is diagnosable + from the log line. A JSON string total reports ``str``. + + ``True`` is reported as ``bool`` rather than silently accepted as the int + 1 — ``bool`` is an ``int`` subclass, so a bare ``isinstance(x, int)`` + would let it through and record a session total of 1. + """ + import logging + + root = tmp_path / "sessions" + _rollout_with_tokens( + root, + "a/one.jsonl", + "10000000-0000-4000-8000-00000000000b", + "/proj/a", + [ + _usage(100, 20, 10, 0, 110), + {**_usage(0, 0, 0, 0, 0), "total_tokens": "1200"}, + {**_usage(0, 0, 0, 0, 0), "total_tokens": True}, + ], + ) + + with caplog.at_level(logging.WARNING): + totals = CodexProvider().session_token_totals( + root, "10000000-0000-4000-8000-00000000000b" + ) + + assert totals is not None + assert totals.total_tokens == 110 # the only well-formed record + assert "bool" in caplog.text + assert "str" in caplog.text + + +def test_real_counter_reset_still_omits_totals( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """The boundary of the change: skipping malformed records must not have + disarmed the guard for a GENUINE decrease between two valid records.""" + import logging + + root = tmp_path / "sessions" + _rollout_with_tokens( + root, + "a/one.jsonl", + "10000000-0000-4000-8000-00000000000c", + "/proj/a", + [_usage(1000, 200, 100, 0, 1100), _usage(100, 20, 10, 0, 110)], + ) + + with caplog.at_level(logging.WARNING): + totals = CodexProvider().session_token_totals( + root, "10000000-0000-4000-8000-00000000000c" + ) + + assert totals is None + assert "monotonicity broken" in caplog.text + + +class _PerMessageUsageProvider(BaseProvider): + """A provider whose usage lives ON THE MESSAGES, with NO cumulative seam. + + This is the discriminating fixture for the project-aggregate override. + The obvious candidate — the other real non-Codex provider — cannot express + the bug: it has no token accounting at all, so its per-message totals are + already zero and overwriting them with zeros is a no-op. A test built on it + passes whether or not the guard exists. + + So this double reports real per-assistant-message ``usage`` (the Claude + shape, which the ordinary accumulators do sum) and inherits + ``session_token_totals`` from the base — i.e. ``None``, the DEFAULT that + every provider but Codex has. + """ + + SESSION_ID = "40000000-0000-4000-8000-000000000001" + + def get_provider_name(self) -> str: + return "permsg" + + def get_session_format(self) -> str: + return "permsg" + + def get_data_dir(self) -> Optional[Path]: + return None + + def discover_sessions(self) -> Iterator[SessionInfo]: + return iter(()) + + def load_session( + self, session_id: str, max_messages: Optional[int] = None + ) -> Iterator[TranscriptEntry]: + return iter(()) + + def _info(self, root: Path) -> SessionInfo: + return SessionInfo( + provider="permsg", + session_id=self.SESSION_ID, + project_path=Path("/proj/permsg"), + source_path=root / "permsg.jsonl", + ) + + def discover_sessions_under(self, root: Path) -> Iterator[SessionInfo]: + yield self._info(root) + + def load_session_under( + self, root: Path, session_id: str, max_messages: Optional[int] = None + ) -> Iterator[TranscriptEntry]: + from claude_code_log.factories.transcript_factory import create_transcript_entry + + for index in (1, 2): + entry = create_transcript_entry( + { + "type": "assistant", + "uuid": f"m{index}", + "parentUuid": None, + "isSidechain": False, + "userType": "external", + "cwd": "/proj/permsg", + "sessionId": self.SESSION_ID, + "version": "1.0.0", + "timestamp": f"2026-07-11T07:0{index}:00.000Z", + "requestId": f"req-{index}", + "message": { + "id": f"msg-{index}", + "type": "message", + "role": "assistant", + "model": "test-model", + "content": [{"type": "text", "text": f"reply {index}"}], + "usage": { + "input_tokens": 100, + "output_tokens": 10, + "cache_read_input_tokens": 5, + }, + }, + } + ) + if entry is not None: + yield entry + + +def _render_permsg(tmp_path: Path, monkeypatch, *, use_cache: bool) -> str: + from claude_code_log.providers.registry import ProviderRegistry + + registry = ProviderRegistry() + registry.register(_PerMessageUsageProvider()) + # render_provider_wholesale imports this lazily from the package, so the + # package attribute is the one that must be patched. + monkeypatch.setattr( + "claude_code_log.providers.discover_providers", lambda: registry + ) + root = tmp_path / "sessions" + root.mkdir(parents=True, exist_ok=True) + # The cache keys source-mtime staleness off SessionInfo.source_path, so the + # file must exist even though this double parses nothing from it. + (root / "permsg.jsonl").write_text("", encoding="utf-8") + index = render_provider_wholesale( + "permsg", root, tmp_path / "out", use_cache=use_cache, silent=True + ) + return index.read_text(encoding="utf-8") + + +def test_project_card_keeps_per_message_totals_without_cumulative_seam( + tmp_path: Path, monkeypatch +) -> None: + """A provider with no ``session_token_totals`` seam must keep its + per-message project totals on the index card. + + ``_sum_provider_token_totals`` returns an ALL-ZERO dict when every session + returned None, and the override used to apply it unconditionally — zeroing + real aggregates. Two messages x (input 100, output 10, cache_read 5). + + Mutation-check: drop the ``has_provider_token_totals`` guard at either + project-level site and this goes RED (the card renders no token line at + all, because a zero column is omitted rather than printed as 0). + """ + html = _render_permsg(tmp_path, monkeypatch, use_cache=False) + assert "Input: 200 | Output: 20 | Cache Read: 10" in html + + +def test_cached_project_aggregates_keep_per_message_totals( + tmp_path: Path, monkeypatch +) -> None: + """The cache-side sibling of the same override, which writes the durable + project aggregates. Guarded symmetrically with the session-level override + twelve lines above it, which was already gated on ``is not None``.""" + from claude_code_log.cache import ( + CacheManager, + get_cache_db_path, + get_library_version, + ) + + _render_permsg(tmp_path, monkeypatch, use_cache=True) + out = tmp_path / "out" + session_page = next(out.rglob(f"session-{_PerMessageUsageProvider.SESSION_ID}*")) + cache = CacheManager( + session_page.parent, get_library_version(), db_path=get_cache_db_path(out) + ) + project_cache = cache.get_cached_project_data() + assert project_cache is not None + assert project_cache.total_input_tokens == 200 + assert project_cache.total_output_tokens == 20 + assert project_cache.total_cache_read_tokens == 10 diff --git a/work/codex-backlog.md b/work/codex-backlog.md index 6495eedc..3adb3403 100644 --- a/work/codex-backlog.md +++ b/work/codex-backlog.md @@ -57,6 +57,23 @@ families and the provider contract are in - Evaluate app-server `thread/read` as a compatibility oracle or a future supported input backend; it should not silently replace local rollout support. +- **Separate "malformed session id" from "session not found".** + `_SESSION_ID_RE` is `[A-Za-z0-9_-]+`, a *character-set* filter and not a UUID + validator: `abc`, `1234` and `not-a-uuid` all `fullmatch`. So an id that could + never name a session passes validation and only surfaces further down as + `FileNotFoundError` from the index lookup. A caller needing to distinguish + *you typed nonsense* from *that session is not here* currently cannot, and the + two deserve different messages. **This is a behaviour change at a boundary — + an exception type callers may already branch on — not a tidy-up**, which is + why it was held out of the decode work rather than folded in. + + The same pattern and the same use exist in **both** providers + (`providers/codex.py:57`, used at `:555` and `:599`; + `providers/claude.py:12`, used at `:49`), so a fix has to cover both or state + why they should differ — changing only the Codex copy would leave the defect + live *and* make the two providers disagree about what a bad id does. Note the + Codex side now has **two** validation call sites rather than one, so "fix the + provider" means both of them. ## Product integration gaps @@ -70,12 +87,48 @@ families and the provider contract are in lineage and strips inherited history but `load_session()` emits one thread. - Decide how native image-view results should render, independently of the already-supported user-message image references. -- Codex token accounting → index totals. The wholesale walker emits zero - input/output/cache token totals per project because Codex rollouts carry no - token accounting the provider currently surfaces; the index token summary is - therefore always blank for Codex projects. If/when token counts are extracted - from rollout records, thread them into the walker's project summaries so the - index totals populate like the Claude path. +- Index token-display gaps (both providers, pre-existing — two faces of one + thing). Current parity: per-session token totals are CACHED for both formats, + but project totals are DISPLAYED on the HTML index only. Not a Codex-specific + hole. + 1. **Per-session token ROWS on the HTML index.** The session-nav macro already + renders `{% if summary.token_summary %}` (`index.html:45-48`) and the cache + has stored per-session token totals since the initial schema on BOTH paths + — the key is simply never populated into the project-summary session dicts, + so no index shows per-session token rows today. Feeding it is small and + needs no template work, but it is a CLAUDE-path UX change (every user's + cards gain per-session rows) that intentionally updates the Claude index + snapshot — its own PR, where that snapshot delta is the reviewable point, + NOT bundled into Codex token accounting (which keeps its "0 `.ambr` + changes" byte-stability signal). The drift pin + `test_index_summary_dict_shape_matches_claude_path` stays green because + both paths gain the key together. + 2. **Markdown index token display (a feature, not a fix).** `token_summary` is + consumed only by the HTML index template (`index.html:45-48`, `:90-91`); + the Markdown projects-index emits project/session/message counts only, so + project token totals never render on the MD index — for Codex OR Claude. + The surface has never existed in the Markdown renderer. This matters for + the vault/Obsidian workflow specifically: `--expand-paths` defaults + `--combined=no` for Obsidian use, so anyone rendering a vault in Markdown + gets no token totals at all. +- Codex per-turn / per-message token accounting. Session and project token + totals now populate the wholesale index (project cards; per-session totals + are stored on the session cache — parity with the Claude schema — pending the + per-session-row display item above), + extracted from the LAST cumulative `token_count` record per session — see + `providers/base.ProviderTokenTotals`, `codex._token_totals_from_records` / + `_map_cumulative_usage`, and the threading in `render_provider_wholesale`. + What remains deferred is FINER-grained attribution (per-turn, per-message). + It is an evidenced design limit, not a TODO: Codex emits a `token_count` + after nearly every agent-loop step — measured over the real corpus (n=4138 + events, 34 sessions) ~75.6% follow a tool-execution step and only ~22.5% + follow an assistant/agent message — so the per-step delta (`last_token_usage`) + spans reasoning + tool I/O + the next turn's cached re-read and does not slice + onto the messages the transcript renders. The session cumulative is the + finest honest unit. Do NOT "fix" this into per-message numbers without a way + to attribute a delta to exactly one rendered message; the impossibility is + documented at `codex._token_totals_from_records` and in + `dev-docs/tools-coverage.md`. - Cache-backed load + paginated combined for the wholesale walker. v1 participates in the SQLite cache for render-SKIP only: `render_provider_wholesale` populates the messages table via `save_cached_entries` for schema uniformity @@ -162,9 +215,138 @@ These refactors were intentionally deferred until behavior was pinned: 1. **Split provider responsibilities.** Extract rollout catalog, tolerant decoder, reconstruction passes, and transcript normalizer; keep `CodexProvider` as a thin orchestration facade. -2. **Remove repeated rollout scans.** Cache identities/decoded records within - one discovery/load operation and add an operation-count test proving - constrained/linear behavior without timing flakes. +2. **Remove repeated rollout scans. — DONE (PR #302).** Both halves landed: the + totals seam (returning entries and totals from one decode) and the + fork-prefix fan-out (reusing the identity discovery already computed, and + decoding a shared parent once per discovery rather than once per child). + + Measured over a frozen 34-rollout archive, 158.6 MB of source, page cache + pre-warmed, `use_cache=False`: + + | | before | after | + |---|---|---| + | `_decode_records` calls | 236 | **104** | + | per-path factor (34-path floor) | 6.94x | **3.06x** | + | bytes actually parsed | 798.4 MB | **327.8 MB** | + | render peak (tracemalloc) | 727.3 MB | **641.8 MB** | + | retained identity map | — | **+20 KB** (725 B × 34) | + + Across the whole arc, including the two-call walker that preceded the seam + fix, parsed bytes fall **1276.6 → 327.8 MB**. Phase split: discovery + 118 → 70, load 118 → 34. + + **Read the hottest-file figures with their labels.** The 11.6 MB parent of 12 + forks — the file that *was* worst — falls **28x → 3x**. The *new* maximum is + **4x**, held by rollouts that are both a fork child and a shared parent. Both + count every `_decode_records` call, header reads included; the earlier "3x as + hottest" was one file tracked across the change while the superlative moved. + + **The operation-count test this item asked for now exists**: + `test/test_codex_fork_prefix_decodes.py`, fully synthetic (`tmp_path` only, + no reach into a real data dir), counting the primitive rather than timing + anything. It pins per-path decode budgets as *exact* equalities — two-sided + on purpose, since a count that is too **low** means a child's prefix + comparison never ran — plus k-invariance of a shared parent's cost, the + ambiguity contract, and the absent-vs-zero case below. + + **What genuinely remains, stated as unmeasured.** The per-file ceiling is + **4** and it is structural, not redundant: a rollout is decoded once as a + fork child and once as a shared parent when it is both, plus one header read + and one decode for its own load. Going lower needs either candidate lists + retained across parent groups — the retention rejected below on measurement — + or a **bounded tail read** of the parent, since the prefix comparison only + needs the parent's tail. **Neither is measured; do not quote a figure for + either.** + + **Why the cache shape was rejected**, and it still constrains any future + attempt: unbounded, a decoded-record cache holds **264 MB resident for + 152 MB of source**, and an entry-count LRU cannot bound it because a single + rollout decodes to **124 MB**. Any cache here needs a byte budget with + explicit eviction. What landed instead retains only identities — a fixed + handful of scalars and two `Path`s each — so bounding the entry *count* + bounds the memory. **Entry size is the discriminator, not lifetime**: the map + lives for the whole run, exactly as the path index already did. + + Two invariants that fail *silently* if a later change disturbs them: + a duplicated thread id is **retained** by discovery but **illegal** to load, + so no fast path may skip the ambiguity raise; and + `inherited_prefix_records == 0` is indistinguishable from *not computed* if + membership is inferred from the value — and 0 is the common case, so + presence-in-the-resolved-map is what signals "computed". + + **This is resource use, not wall clock.** Describe it as amplification, never + as a speedup: rendering dominates the run, so removing decodes buys I/O and + CPU rather than a visible improvement. An earlier profiling pass put roughly + 70s of a ~93s instrumented run in HTML/Markdown generation, with interleaved + medians of 54.6s vs 56.4s — quoted as that run's numbers, not re-derived here, + and deliberately not the justification for anything. Wall clock on this work + has already produced one retraction: a +40.7% regression that did not + reproduce (+9.9% controlled) because the harness was launched on a box under + load. **Timing figures here need an idle machine and interleaved arms, or they + measure the machine.** The deterministic figures above carry the case on their + own. + + **METRIC CAVEAT — the byte figures published before 2026-07-31 over-charge by + 1.59x, and the error flatters this exact fix.** `Σ(file size × decode calls)` + bills every call as a full file read, but `_decode_records` streams lazily + from an open handle and `_read_identity` returns on the first `session_meta`, + so a one-line read was charged a whole file: + + CHARGED Σ(size × calls) 1272.5 MB 8.0x + CONSUMED lines actually parsed 798.4 MB 5.0x + over-charge 1.59x + + It closes to the byte: **102 of those 236 calls are early exits** (34 each at + `_session_index`, `_discover_in`, `_load_in`), so the phantom charge is + 3 × 158.6 = **475.8 MB** against an observed charged−consumed gap of + **474.1 MB**, leaving 1.7 MB as what those calls really read. The over-charge + is **not a fixed discount** — it measured 1.495x / 1.594x / 1.482x across the + three states — so charged bytes cannot be converted to parsed bytes by any + constant, and two figures are comparable only within one metric. + + The consequence for scoping anything here: collapsing the 3N `_read_identity` + calls to N scores **~317 MB charged and ~1.1 MB real**. Report **parsed** as + the honest number; keep charged only to compare against older charged + figures. Where the bytes actually were: decoding each parent once per child + cost **277.5 MB**, against **41.6 MB** for all 25 fork children put together, + and the 11 *distinct* parents are only **127.1 MB**. That is why grouping by + parent was the byte win independently of any call count. (Those two sum to + 319.1 MB against a measured discovery total of 320.2 MB, the 1.1 MB remainder + being the 34 early-exit header reads — which is also an independent check that + the phase attribution is right.) + + **The measurement METHOD, recorded because the corpus is not being kept.** + The 152 MB archive these figures came from is real user data with no owner and + is being retired; the synthetic test above is its successor for *regression*, + but it cannot reproduce these *magnitudes*. To re-measure credibly: + + - Freeze the input first and hash it. Comparing against a live archive makes + drift indistinguishable from regression. Hash file *contents* in sorted-name + order, not `md5sum` output — that embeds paths, so a byte-identical tree at + a different path yields a different digest and looks exactly like drift. + - Attach at `_decode_records` — the single primitive every path funnels + through — and count per path, tagging the discovery and load phases + separately. The phase split is what localises a regression. + - Run both arms **in one process** with the page cache pre-warmed, and + `use_cache=False` so a render-skip cannot mask a decode. Interleave arms + rather than sequencing them: on a loaded machine, sequenced arms measure the + machine — a sequenced run once produced a stable-looking +50% for the + *faster* arm while call counts were byte-identical. + - For parsed bytes, count the lines actually handed to the parser (a counting + proxy around the file object works, and avoids re-implementing the decoder + — a lookalike decoder that disagrees is indistinguishable from a + regression). Do **not** infer bytes from call counts. + - Anchor memory claims on the **max** decoded size, not a median or a ratio: + the max (124 MB) reproduces to the digit, while the median wobbles ~0.9–1.05 + MB with allocator overhead, so any max/median ratio is not reproducible. + One earlier "~120x" figure was retracted for exactly this. + - Treat the figures above as a **historical snapshot of one archive**, not a + target. A different corpus with different fork density will not reproduce + them, and should not be expected to. + + The 127-line measurement analysis on the unpushed `dev/codex-decode-backlog` + branch is **superseded by this item** — it predates the metric correction and + states the charged figures as bytes read. Do not merge it as-is. 3. **Simplify registry/discovery ownership.** Choose one discovery facade, validate factories/instances, and remove or exercise unused provider hooks. 4. **Centralize entry construction.** Introduce a context/builder for session, @@ -173,7 +355,12 @@ These refactors were intentionally deferred until behavior was pinned: Architecture work must preserve the provider contract, Codex adversarial suite, and HTML/Markdown exports after every extraction. Avoid long-lived cache -state until invalidation semantics are explicit. +state until invalidation semantics are explicit — noting that item 2 landed a +run-lifetime *identity* map under that rule rather than as an exception to it: +it shares the key, lifetime and staleness assumption of the path index that was +already there, and its entries are a fixed size, so bounding the count bounds +the memory. The rule still forbids what it was written to forbid, which is +retaining decoded **records**. ## Refresh checkpoints