Repository navigation
perf(agents): reduce repeated tool-result budgeting - #139156
Merged
Merged
Conversation
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
Contributor
|
Codex review: needs maintainer review before merge. Reviewed September 5, 2026, 10:56 AM ET / 14:56 UTC. ClawSweeper reviewWhat this changesThe PR reuses tool-output size measurements during prompt preparation and compaction planning, reducing repeated text scans. Merge readiness✅ Ready for maintainer review Keep open: this is a useful optimization not present on inspected main. No actionable correctness or security findings were found. Priority: P2 Review scores
Verification
How this fits togetherOpenClaw budgets tool outputs before sending conversation history to a model and when deciding whether truncation can avoid compaction. This planner produces bounded messages and estimates how much context can be recovered. flowchart TD
A[Conversation history] --> B[Measure tool outputs]
B --> C[Apply per-result limits]
C --> D[Apply aggregate budget]
D --> E[Bounded model history]
D --> F[Recovery estimates]
F --> G[Truncate or compact]
Before mergeNone. Agent review detailsSecurityNone. PR surfaceSource +33. Total +33 across 1 file. View PR surface stats
Review metrics
Technical reviewBest possible solution: Keep measurement ownership inside the synchronous planner while preserving message shapes, frozen projections, spill references, and recovery accounting. Do we have a high-confidence way to reproduce the issue? Yes, for the performance premise: current main visibly repeats text budgeting, and the supplied body specifies a concrete CJK benchmark fixture; no benchmark was executed during this review. Is this the best way to solve the issue? Yes. The shared planner is the appropriate owner; the neighboring context estimator counts different content, and cross-call caching would introduce unnecessary mutation invalidation. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 951074675a44. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
steipete
deleted the
codex/team-tool-result-budget-facts-74cd-20260905
branch
September 5, 2026 15:05
Contributor
Author
|
Merged via squash.
|
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Sep 6, 2026
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
1 participant
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.
What Problem This Solves
Tool-heavy histories spend unnecessary synchronous work preparing prompts and estimating whether tool-result truncation can avoid compaction. Even when every result fits, the same text is measured twice during projection and three times during reduction estimation.
Why This Change Was Made
The replacement planner now measures each input once and carries that fact through its oversized and aggregate phases. Producers measure replacement messages once; application consumes those measurements. The reduction estimator reuses the planner's original totals.
Measurements are private to one synchronous plan. Returned messages, replacement descriptors, projection state, truncation policies and persisted data retain their existing shapes and behavior. This avoids a cache attached to mutable messages or shared across calls. Production delta: +94/-61 (net +33), for the explicit measurement ownership and boundary conversion; tests are unchanged.
User Impact
Less CPU work during tool-result projection and recovery planning, particularly for CJK-heavy results. Truncation output, frozen prompt prefixes, trailing-result protection, spill references and recovery counters are preserved.
Evidence
"漢字カナ한글".repeat(1500), with 64,000 per-result and 2,000,000 aggregate budgets. These are local synthetic measurements of the uninstrumented owner with explicit dormant-import adapters, not production latency. Peak process RSS across the runs was 114,352 → 113,936 KiB.git diff --checkpass. The complete required changed check passes on the isolated Linux source bundle (12m57s), and exact-head CI 33972991872 is green. Native preparation also passes.No permanent private-helper spy or wall-clock assertion was added; the retained failing diagnostic and before/after benchmark prove the performance premise, while existing public-boundary suites protect behavior.
The completed ClawSweeper review was read in full: no findings, no pre-merge requirements, and no Rank-up moves to apply.