Split out from the review on #174, which added the archive/events read that makes this cost real. Raised twice there by the Claude reviewer; declined for that PR on design grounds, but the cost is genuine and worth tracking rather than dismissing.
Problem
log reads archive/events on every invocation and cannot scope it. log_event_rows calls log_archived_event_rows unconditionally, which lists and JSON-parses every record under the prefix — recursively for the local backend — regardless of --work-item, --since, or --limit. Narrowing happens afterwards, in memory.
The live events prefix has the same unscoped shape, but it stays small because gc continuously compacts and drains it. archive/events is the opposite: it accumulates every compacted generation for the whole archive_days retention window, 30 days by default, across the entire state root. So in steady state the archive listing is comparable to or larger than the live one.
Concretely, agent-coord log shakacode/example#104 --since 1h on a busy fleet walks and parses the entire 30-day compacted history to answer a question about one work item in the last hour. On the HTTP backend that is a full paginated listing plus a body fetch per envelope; on the legacy GitHub backend, GitHubStore#list_json issues one API call per blob, so it is one call per archived envelope on every log.
Why it was not fixed in #174
The obvious short-circuit — skip the archive when the queried work item has no archived history — is not implementable with the current layout. Archive paths are keyed by batch and content digest (archive/events/<batch>/compact-<target-digest>-<source-digest>.json), not by repo or work item, and the mapping from a work item to its batches lives inside the records. Deciding "this item has nothing archived" requires reading the archive to find out.
Reading by default was still the right call there: the trail vanishing for completed work is a correctness defect, and putting continuity behind a flag would leave it defaulted-broken for exactly the audit case it exists to serve. But that traded a correctness fix for a real and unbounded-in-time read cost, which is what this issue tracks.
Options
- An index. A mapping from work item to the batches that hold its events, maintained on write, so
log can open only the envelopes that can possibly match. Most effective and the largest change; it is a store change, not a log change.
- Identity in the archive path. Have compaction encode enough identity in the envelope filename (repo/target, or a digest of the work items it covers) that
log can skip an envelope without fetching and parsing its body. Cheaper than a full index, and it helps the listing-only backends most. Needs care: one envelope can span several work items.
- A retention-window bound on the read. When
--since is given, skip envelopes whose archived_at is older than the window. Cheap and helps the common recent-history query, but only if archived_at is available from the listing rather than the body — otherwise it saves the parse but not the fetch.
Option 3 is the smallest useful step and could land independently. Options 1 and 2 want a design discussion first.
Not in scope
The read-by-default behavior itself. That is the fix for #139 and should not be reverted to a flag; this is about making it cheap, not about making it optional.
Split out from the review on #174, which added the
archive/eventsread that makes this cost real. Raised twice there by the Claude reviewer; declined for that PR on design grounds, but the cost is genuine and worth tracking rather than dismissing.Problem
logreadsarchive/eventson every invocation and cannot scope it.log_event_rowscallslog_archived_event_rowsunconditionally, which lists and JSON-parses every record under the prefix — recursively for the local backend — regardless of--work-item,--since, or--limit. Narrowing happens afterwards, in memory.The live
eventsprefix has the same unscoped shape, but it stays small becausegccontinuously compacts and drains it.archive/eventsis the opposite: it accumulates every compacted generation for the wholearchive_daysretention window, 30 days by default, across the entire state root. So in steady state the archive listing is comparable to or larger than the live one.Concretely,
agent-coord log shakacode/example#104 --since 1hon a busy fleet walks and parses the entire 30-day compacted history to answer a question about one work item in the last hour. On the HTTP backend that is a full paginated listing plus a body fetch per envelope; on the legacy GitHub backend,GitHubStore#list_jsonissues one API call per blob, so it is one call per archived envelope on everylog.Why it was not fixed in #174
The obvious short-circuit — skip the archive when the queried work item has no archived history — is not implementable with the current layout. Archive paths are keyed by batch and content digest (
archive/events/<batch>/compact-<target-digest>-<source-digest>.json), not by repo or work item, and the mapping from a work item to its batches lives inside the records. Deciding "this item has nothing archived" requires reading the archive to find out.Reading by default was still the right call there: the trail vanishing for completed work is a correctness defect, and putting continuity behind a flag would leave it defaulted-broken for exactly the audit case it exists to serve. But that traded a correctness fix for a real and unbounded-in-time read cost, which is what this issue tracks.
Options
logcan open only the envelopes that can possibly match. Most effective and the largest change; it is a store change, not alogchange.logcan skip an envelope without fetching and parsing its body. Cheaper than a full index, and it helps the listing-only backends most. Needs care: one envelope can span several work items.--sinceis given, skip envelopes whosearchived_atis older than the window. Cheap and helps the common recent-history query, but only ifarchived_atis available from the listing rather than the body — otherwise it saves the parse but not the fetch.Option 3 is the smallest useful step and could land independently. Options 1 and 2 want a design discussion first.
Not in scope
The read-by-default behavior itself. That is the fix for #139 and should not be reverted to a flag; this is about making it cheap, not about making it optional.