Cache trace branches by node count#1799
Conversation
ApprovabilityVerdict: Approved This PR adds a straightforward cache for the You can customize Macroscope's approvability policy. Learn more. |
af6db01 to
5f30b53
Compare
5f30b53 to
d6aef07
Compare
d6aef07 to
9f93543
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f935437d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
9f93543 to
c0fea64
Compare
c0fea64 to
ec04697
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec04697fd7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| subagents). Branching falls out of walking each leaf's parents back to its root.""" | ||
| node_count = len(self.nodes) | ||
| if self._branch_cache_node_count == node_count: | ||
| return self._branch_cache.copy() |
There was a problem hiding this comment.
Return defensive Branch copies from the cache
Fresh evidence after the prior review: this returns only a shallow copy of the cached list, so callers still receive the exact cached Branch instances. If any post-processing mutates a branch object or its public nodes list, such as trace.branches[0].nodes.pop(), later reads while len(self.nodes) is unchanged reuse the corrupted branch and can make prompt_len, completion_len, or tool_messages wrong; before this change each access rebuilt new Branch objects. Cache immutable data or return defensive Branch copies.
Useful? React with 👍 / 👎.
Overview
Cache the derived
Trace.branchesview bylen(trace.nodes)so repeated trace properties reuse the same branch materialization. Token-limit evaluation keeps using the existingTraceAPI and preserves its current semantics.What changed
Trace.num_branchesreuse the cached branch count when warm, while retaining its cheap leaf scan on cold count-only access.Why
prompt_len,completion_len, andtotal_tokenseach readTrace.branches. Token-limit checks can therefore reconstruct the same graph paths three times. Caching the derived view at its owner removes those repeated graph walks for every caller without adding topology-specific counting logic to the interception server.The public property still returns an independent outer list, so caller operations such as sorting, popping, or clearing do not mutate the internal cache.
Performance
Median wall time with all three token limits enabled:
A cache miss represents the normal case after the trace grows; a hit represents repeated reads at the same node count.
Warm
num_branchesreads improve from 346.4 µs to 0.659 µs for a 10k-node linear trace and from 100.2 µs to 0.672 µs for a shared-prefix trace with 1k leaves. Cold count-only reads continue usinggraph.leavesand avoid materializing full branch paths.Note
Low Risk
Localized performance optimization on a derived view; semantics stay the same as long as branches only change when nodes are appended (existing rollout model).
Overview
Adds a private branch cache on
Tracekeyed bylen(nodes)so the expensive root→leaf branch materialization runs once per graph size instead of on every access.branchesnow returns a cached copy when the node count is unchanged; otherwise it rebuilds, stores the result, and returns a copy (same outward behavior as before).num_branchesreadslen(_branch_cache)when the cache is warm and still falls back tograph.leaves(self)for cold count-only access.This mainly speeds repeated reads of
prompt_len,completion_len, andtotal_tokens(each iteratesbranches), including token-limit checks that previously walked the graph multiple times at the same node count.Reviewed by Cursor Bugbot for commit ec04697. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Cache trace branches by node count in
TraceAdds memoization to
Trace.branchesandTrace.num_branchesin trace.py to avoid recomputing branches on every access. The cache is keyed by node count and invalidated automatically when the graph grows. Both properties return results from_branch_cachewhen the node count matches, and fall back to full recomputation otherwise.Macroscope summarized ec04697.