fix(moa): apply prompt-caching decoration to the aggregator's one-shot synthesis call - #58284
Closed
srojk34 wants to merge 1 commit into
Closed
fix(moa): apply prompt-caching decoration to the aggregator's one-shot synthesis call#58284srojk34 wants to merge 1 commit into
srojk34 wants to merge 1 commit into
Conversation
…t synthesis call 22c5048 restored Anthropic-style cache_control for two of MoA's three call paths: the acting aggregator (MoAChatCompletions.create, the persistent `provider: moa` model) and the advisor fan-out (_run_reference). aggregate_moa_context() -- the /moa <prompt> one-shot command's synthesis call -- is the third, independent call path and was never covered: its call_llm(task="moa_aggregator", ...) sent a single undecorated user message containing the full joined reference output, re-billing the entire input on every invocation even when the resolved aggregator slot is a cache-honoring route (Claude on OpenRouter/native Anthropic, MiniMax, Qwen/DashScope). - Generalize _maybe_apply_advisor_cache_control to _maybe_apply_moa_cache_control (it never had advisor-specific logic -- same policy function, same breakpoint layout as the main loop, judged purely on the passed-in runtime) and reuse it in aggregate_moa_context the same way _run_reference already does. - Compute _slot_runtime(aggregator) once and reuse it for both the decoration call and the call_llm kwargs, instead of calling it twice. Mutation-verified: reverting the moa_loop.py change makes the new regression test fail by asserting a plain string aggregator-message content where the cache-honoring case expects native cache_control content blocks.
Collaborator
|
Merged via #58334 (commit 2d3eac5 on main) — cherry-picked to preserve your authorship. Thanks @srojk34! This closes the last of the three MoA prompt-caching call paths (after #57675 covered the acting aggregator and advisor fan-out). Verified with an E2E run against the real |
tonydwb
reviewed
Jul 5, 2026
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment
Applies the prompt-caching decoration to the MoA aggregator's one-shot synthesis call so that cached prompt prefixes are reused for the aggregation step.
Observations
max_tokens=max_tokensat line 85 — this passes the parameter through. Verify the aggregation call actually needsmax_tokens(the synthesis output is typically short).
Looks Good
- No hardcoded secrets
- No debug artifacts
- No test changes (behavioral fix)
Reviewed by Hermes Agent
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.
Summary
22c5048d9("fix(moa): restore prompt caching for the aggregator and advisors") restored Anthropic-stylecache_controlfor two of MoA's three call paths: the acting aggregator (MoAChatCompletions.create, the persistentprovider: moavirtual model) and the advisor fan-out (_run_reference).aggregate_moa_context()— the/moa <prompt>one-shot command's synthesis call — is a third, independent call path and was never covered: itscall_llm(task="moa_aggregator", ...)sends a single undecorated user message containing the full joined reference output, re-billing the entire input on every invocation even when the resolved aggregator slot is a cache-honoring route (Claude on OpenRouter/native Anthropic, MiniMax, Qwen/DashScope) — the same cost-impact bug class22c5048d9measured at up to 85% cache share lost.No test file references
aggregate_moa_contextat all currently, so this gap had zero regression coverage.Changes
_maybe_apply_advisor_cache_controlto_maybe_apply_moa_cache_control— it never had advisor-specific logic (same policy functionanthropic_prompt_cache_policy, same breakpoint layoutapply_anthropic_cache_control, judged purely on the passed-inruntimedict) — and reused it inaggregate_moa_contextthe same way_run_referencealready does._slot_runtime(aggregator)once inaggregate_moa_contextand reused it for both the decoration call and thecall_llmkwargs, instead of calling it twice.Test plan
test_aggregator_synthesis_gets_cache_control_on_native_anthropic_route— a cache-honoring aggregator slot (native Anthropic) getscache_controlcontent-block decoration on its synthesis messagetest_aggregator_synthesis_untouched_on_non_caching_route— a plain OpenAI aggregator slot stays undecorated (plain string content), proving the guard doesn't over-fireagent/moa_loop.pymakes the first test fail (asserts plain string content where the fix produces native cache_control blocks)scripts/run_tests.sh tests/hermes_cli/test_moa_config.py tests/agent/test_moa_trace_streamed_capture.py tests/agent/test_moa_aggregator_cost_slot.py tests/agent/test_moa_aggregator_cache_control.py tests/agent/test_moa_switch_api_mode.py tests/agent/test_moa_slot_api_mode.py tests/cli/test_moa_command.py tests/tui_gateway/test_moa_reference_emit.py tests/run_agent/test_moa_streaming.py tests/run_agent/test_moa_loop_mode.py tests/gateway/test_moa_one_shot_restore.py— 84 passedruff checkclean on all changed filesty checkbefore/after diff on changed files — only line-number shifts, no new diagnostics