perf(dashboard-api): cheaper LCM display token counts via existing cache - #549
Conversation
Seat retries and the seat/reconcile duplicate publication of one sealed generation rebuilt every graph entity and relation per attempt, with each edge serialized and hashed three times; interactive adjacency hydrated the far endpoint of every edge, including kinds the query excluded, and re-read shared endpoints once per incident edge. - memoize the complete published-graph manifest on the immutable generation, keyed by projection identity and projector revision; only a fully successful build is recorded, so a deadline mid-build memoizes nothing and a memo hit still refuses an expired request - derive each edge/symbol/chunk/file/import stable identity exactly once per projection build and reuse it across the entity and its relations - stage semantic adjacency hydration: excluded edge kinds stop at the validated edge payload without reading their far endpoint, and each surviving unique endpoint hydrates once per batch Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
td usecases cross-review (crate-only comments; I did not edit, merge, or flatten.)
Cheaper path is real and not a deadline: LCM display counts reuse TokenCountCache instead of re-running count_text_tokens on every poll. Separate LCM display map vs Savings map is the right split (o200k display vs model-mapped stored text). Compiled-out path stays unavailable and never invents a count.
Isolated tests actually show less work: the sentinel overwrite pins zero re-encodes for unchanged content, and a content rewrite under the same message id misses. Stronger than a timing bound.
Stay draft until the feature-off (default) run is actually green. #[cfg(not(feature = "token-counting"))] not executing on a token-counting job is skipped-only, not green.
Leftover on the hit path: displayed_tokens allocates two owned Strings on every lookup. Timeline clamp is 2000 messages per poll — still far cheaper than BPE, but the cheap path can be cheaper still.
| } | ||
| let fingerprint = content_fingerprint(&message.content); | ||
| let token_count = token_counts | ||
| .displayed_tokens(&message.provider, &message.message_id, fingerprint) |
There was a problem hiding this comment.
This is the right cheaper path: consult the shared cache, encode only on miss, never a tighter deadline. Provenance O200kApproximate only when a number exists (fresh or cached from the same o200k_base encode); compiled-out / failed stays absent.
The sentinel tests under token-counting pin less work (zero re-encodes). Stay draft until the feature-off twin is actually run and green — skipped-only ≠ green.
There was a problem hiding this comment.
Confirmed on the current tip 23f6bfc ("bound LCM display token cache"). Feature-off run actually executed (not skipped):
cargo test -p tracedecay-dashboard-api --lib -- lcm_api::aggregates::tests token_count::tests
Result: 15 passed, 0 failed with default features (not(token-counting)), including compiled_out_counting_stays_unavailable_and_never_consults_the_cache, timeline_does_not_publish_a_partial_total_when_any_message_is_unknown, and a_compiled_out_tokenizer_is_unavailable_instead_of_zero. Feature-on twin (--features token-counting, same filter): 17 passed, 0 failed — provenance stays o200k_approximate only when a BPE-derived number exists, null/unavailable otherwise.
| .lcm_display | ||
| .lock() | ||
| .unwrap_or_else(std::sync::PoisonError::into_inner); | ||
| map.get(&(provider.to_owned(), message_id.to_owned())) |
There was a problem hiding this comment.
Every cache hit still allocates (provider.to_owned(), message_id.to_owned()) just to look up. Search/session/overview and timeline (clamp 1..=2000) all go through this on every poll.
Not a deadline issue and still cheaper than BPE, but this is leftover work on the path you just made cheap. A borrow-friendly key (or a two-level map) would keep the hit path allocation-free. Unbounded lcm_display matches the existing Savings map; fine if the process is short-lived, worth a bound if the dashboard stays up.
|
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
td usecases deep review (comments only; I did not edit, restack, flatten, or change the base.)
Cheaper path still holds and is not a deadline. Display map stays separate from Savings (o200k hydrated display vs model-mapped stored text). Feature-off still early-returns and never invents a count. 074eb21 landed the two-level / borrow-keyed hit path — lookup no longer to_owneds on every poll.
Stay draft until both token-counting and feature-off (default) runs are actually green. Skipped-only ≠ green.
Leftover, not a blocker: content_fingerprint still hashes full message text on every render, including cache hits. Far cheaper than BPE; only worth shaving if a poll is still hot after the encode is gone. Unbounded lcm_display still matches the existing Savings map.
| .lcm_display | ||
| .lock() | ||
| .unwrap_or_else(std::sync::PoisonError::into_inner); | ||
| map.get(provider) |
There was a problem hiding this comment.
Two-level map is the right simplification for a borrow-keyed hit. get(provider).and_then(get(message_id)) allocates nothing on the poll path. Display vs Savings maps stay separate — do not merge them (different tokenizer, different text). Store still owns Strings on miss only; that is correct.
| token_counts: &TokenCountCache, | ||
| ) -> DisplayedContentTokenCount { | ||
| match count_text_tokens(&message.content, "") { | ||
| if !counting_available() { |
There was a problem hiding this comment.
Feature-off still short-circuits before cache consult or store. Keep that. A planted cache entry must never surface as an invented count (the compiled_out_counting_stays_unavailable_and_never_consults_the_cache pin). Stay draft until that cfg-off run actually executes and is green — a token-counting job skipping it is skipped-only, not green.
There was a problem hiding this comment.
Kept: the compiled-out path returns early before any cache consult or store, and a planted entry never surfaces. Pinned by compiled_out_counting_stays_unavailable_and_never_consults_the_cache, which executed (not skipped) on the current tip 23f6bfc:
cargo test -p tracedecay-dashboard-api --lib -- lcm_api::aggregates::tests token_count::tests
Result: 15 passed, 0 failed with default features (not(token-counting)). The test renders with an empty cache (asserts null count, null provenance, nothing stored), then plants a cache entry under the exact key/fingerprint and re-renders — still null/null, no invented count.
…a84' into td/pr550-update
…work-shrink-6b34
…kpoint-f76e' into td/pr549-update
2 participants
What
Crate-only cheaper path in
crates/tracedecay-dashboard-api: the LCM render path (displayed_content_token_countinlcm_api/aggregates.rs) now reuses the crate's existingTokenCountCacheinstead of re-running the BPE (count_text_tokens) on everymessage_jsonand every timeline message on every dashboard poll. This is not a deadline change —request_deadline.rs, HTTP contracts, and envelope shapes are untouched; no deadlines added or tightened.Why
Every LCM search/session/overview
message_jsonand every message intimeline_json(default limit 400, clamp 1..=2000) re-encoded the same message bodies witho200k_baseon each repeat render. The crate already hadTokenCountCacheso Savings doesn't re-scan; the LCM render bypassed it.How
TokenCountCachegains an LCM display map keyed by provider then message id (two levels, so a cache hit borrows the caller's&strkeys with zero String allocations; only inserts own keys), guarded by a content fingerprint (byte length + full-content hash). It is kept separate from the Savings map because Savings counts stored text with the model-mapped tokenizer while LCM counts hydrated display content witho200k_basespecifically — entries are not interchangeable, so no mislabeled counts. The map is bounded the same way as the existing Savings map: one entry per(provider, message_id), replaced in place on content change.displayed_content_token_countconsults the cache first; only a miss (new or changed content) callscount_text_tokens, and the result is stored for the next render. Search, session, overview matches, and timeline all flow through this one function.O200kApproximateonly when the count came from the BPE (fresh or cached — every cached value originated from the sameo200k_baseencode);None/Unavailablewhen counting is compiled out or fails. No invented counts: the compiled-out path returns early and never consults or populates the cache.Isolated tests (this crate only) — less work, deterministically
repeat_message_render_reads_the_shared_cache_instead_of_reencoding: first render equals a directcount_text_tokensresult witho200k_approximateprovenance; then the cached entry is overwritten with a sentinel the BPE cannot produce and the repeat render returns the sentinel — a single re-invocation of the BPE would have returned (and re-stored) the real count, so this pins zero re-encodes for unchanged content, stronger than a timing bound. Changed content under the same message id misses the cache and is recounted (no stale counts served).timeline_and_message_renders_share_one_display_cache: the message path populates the shared cache with exactly the direct BPE counts, and sentinels planted under the message keys surface in timeline bucket sums — both paths hit the same cache.compiled_out_counting_stays_unavailable_and_never_consults_the_cache(not(token-counting)): renders staynull/unavailable, nothing is stored, and even a planted cache entry never surfaces as an invented count.display_cache_is_guarded_by_content_fingerprint(token_count.rs, runs in both feature variants): lookups use&strkeys borrowed from caller-owned data (hit path requires no owned Strings); the guard rejects other message ids, other providers, and same-length content rewrites.Verification (both feature variants actually ran)
cargo test -p tracedecay-dashboard-api --features token-counting --lib -- lcm_api::aggregates::tests token_count::tests— 16 passed, 0 failed (includes the sentinel zero-re-encode and shared-cache tests).cargo test -p tracedecay-dashboard-api --lib -- lcm_api::aggregates::tests token_count::tests(default features,not(token-counting)) — 14 passed, 0 failed, with the compiled-out-specific tests executed (not skipped):compiled_out_counting_stays_unavailable_and_never_consults_the_cache,timeline_does_not_publish_a_partial_total_when_any_message_is_unknown,a_compiled_out_tokenizer_is_unavailable_instead_of_zero.cargo check -p tracedecay-dashboard-api --features token-counting— pass.Review follow-up applied
usecases review leftover:
displayed_tokensallocated(provider.to_owned(), message_id.to_owned())on every lookup including hits. Fixed with the two-level map above (074eb21); hit path is now allocation-free.Stack
Appended to stack #543: #509 → #421 → #540 → #541 → #538 → #546 → #545 → #548 → #544 → this PR (
gh stack link 543 549now reports it registered). Branched offorigin/codex/tracedecay-total-redesign-planby name; base is the stack topcursor/extract-borrowed-source-0346(contains the #421 tip; merge-base is exactly the fork point, so the diff stays this one crate-only change). Does not reopen #535 or #531; does not touch #509 or other crates; no merges, no flattening, nogh stack merge.