Repository navigation
fix: reduce repeated history processing and token estimation - #250
Merged
Merged
Conversation
Collaborator
|
Overall: this is a well-scoped performance optimization PR. I would approve with follow-up. Main benefits:
Key concerns:
Validation: remote CodeQL, source audit, and macOS/Ubuntu verify checks pass. The reported synthetic benchmark shows roughly 11.7–14.2x speedup for value-only snapshots and 2.9–3.2x for fingerprinting, but it is not an end-to-end or RSS benchmark. Recommendation: merge after confirming provider-contract coverage; a production-adapter/compaction regression test would be a useful follow-up. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change
Long histories repeatedly paid for JavaScript SHA-256, eager fingerprinting of value-only snapshots, BPE tokenization of unchanged text, and full post-commit history rereads.
structuredCloneof the entire input graph. Compute fingerprints on demand and reuse only an already-owned, deeply frozen root. Native graph cloning preserves shared references even across side-effecting getters and normalized class instances.This reduces repeated work; it does not eliminate every history scan or file read. No user-facing configuration or documentation changes are required.
Tradeoffs
Validation
pnpm verifyon clean commit761b4c683ab1e8dfac4edaa490cfe27f8644d592, macOS arm64 / Node v26.4.0. All 14 gates passed, including 3,886 capability tests across 144 files.8ad1e09f7653d7e97b83b4f5b4d347ca2fa129c7, using its unchanged 100-turn, approximately 4 KiB/turn, 64-character chunk, zero-delay scenario and 100 ms process-tree sampler. Basea55d629and this commit each received one excluded warmup and three serial, interleaved measured runs on Apple M5 / 24 GB / macOS 26.6.2 / Node v26.4.0 / Bun 1.4.2. Every run completed with 101 mock requests, 100 successful tool calls, exit code 0 and the same final response.These measurements are for this scripted workload and final commit; RSS is sampled and workload-dependent. They supersede the earlier snapshot-optimization measurements.
Publication and contribution checks
release/public-source.json; new tests are declared intest/vitest-suites.jsonwhere applicable.Maintainer handoff
Publication scope or license changes: none beyond the registered regression test.
Shared-source port: pending.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.