Make Executor ReAct loop append-only for prompt-prefix caching#187
Draft
yzx9 wants to merge 1 commit into
Draft
Conversation
Rewrite Executor.run to seed [system, user(task)] once and append one
assistant turn (via the shared reconstruct_react_text) plus one user
observation turn per step, instead of rebuilding a single growing user
message each iteration. The static system prompt and all prior turns now
form a byte-stable prefix, so provider-side prefix caching hits on every
iteration. Previously only the system block cached; the whole rebuilt user
message (Previous-Steps JSON + Iteration counter + last response) was a
cache miss every step, with hit rate declining as the loop grew.
Folded correctness fixes (surfaced in plan-eng-review + /review):
- _generate_final_summary: render the string action_summary directly
instead of calling .get('name') on it (AttributeError on exhaustion).
- tool_retry_counter: reset at run() start. Executors are reused across
delegated tasks, so the counter leaked and could poison later runs.
- max_syntax_errors=3 budget with reset-on-success: abort a model stuck
emitting malformed ReAct instead of spamming up to 15 correction turns.
The budget resets after any cleanly-parsed turn, so only 3 CONSECUTIVE
bad turns abort (not 3 slips scattered across a long run).
Extract reconstruct_react_text into copilotj/multiagent/react_format.py
(LeaderAgent and Executor both import it; leader_multiagent imports
Executor, so it could not live there without a circular import). Document
in _build_enhanced_system_prompt that the tool text is load-bearing: the
ReAct wrapper strips the tools= API param before the provider call, so the
system-prompt text is the model's only view of the tools.
Document pre-existing dead branch in _suggest_tool_based_on_context and
deferred correction-path cache trade-off (codex P2 #2)
Add copilotj/test/multiagent/test_executor.py (9 cases incl. a post-format
prefix-stability cache-contract test that runs messages through the
Anthropic formatter rather than asserting raw list-append). Full suite
206 passed; ruff clean.
45cf031 to
a32d7a3
Compare
00JackLu
approved these changes
Jul 2, 2026
00JackLu
left a comment
Collaborator
There was a problem hiding this comment.
The code looks good, but I haven’t tested it.
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.
Rewrite Executor.run to seed [system, user(task)] once and append one assistant turn (via the shared reconstruct_react_text) plus one user observation turn per step, instead of rebuilding a single growing user message each iteration. The static system prompt and all prior turns now form a byte-stable prefix, so provider-side prefix caching hits on every iteration. Previously only the system block cached; the whole rebuilt user message (Previous-Steps JSON + Iteration counter + last response) was a cache miss every step, with hit rate declining as the loop grew.
Folded correctness fixes (surfaced in plan-eng-review + /review):
Extract reconstruct_react_text into copilotj/multiagent/react_format.py (LeaderAgent and Executor both import it; leader_multiagent imports Executor, so it could not live there without a circular import). Document in _build_enhanced_system_prompt that the tool text is load-bearing: the ReAct wrapper strips the tools= API param before the provider call, so the system-prompt text is the model's only view of the tools.
Add copilotj/test/multiagent/test_executor.py (9 cases incl. a post-format prefix-stability cache-contract test that runs messages through the Anthropic formatter rather than asserting raw list-append). Full suite 206 passed; ruff clean.
Assisted-by: Claude-Code:GLM-5.2