refactor(anthropic): extract 7 helpers from convert_messages_to_anthropic (salvage #27784)#30386
Merged
Conversation
Split convert_messages_to_anthropic (complexity 79) into 7 focused helpers: - _convert_assistant_message — assistant msg to content blocks - _convert_tool_message_to_result — tool msg to tool_result + merge - _convert_user_message — user msg validation + conversion - _strip_orphaned_tool_blocks — orphan tool_use + tool_result removal - _merge_consecutive_roles — role alternation enforcement - _manage_thinking_signatures — strip/preserve/downgrade by endpoint - _evict_old_screenshots — keep only 3 most recent images Main function complexity: 79 → 10 (below C901 threshold). Zero logic changes — pure extraction. Net -4 lines (refactor itself); +45/-17 follow-up polish for annotation tightening (List[Dict] → List[Dict[str, Any]]), restored rationale comments in _manage_thinking_signatures (third-party endpoint examples, #13848/#16748 issue refs, redacted_thinking 'data'-as-signature note), and "Mutates ``result`` in place." docstring lines on the four mutating helpers.
Contributor
🔎 Lint report:
|
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
3 participants
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
Pure refactor:
convert_messages_to_anthropicinagent/anthropic_adapter.pywas the largest cyclomatic-complexity hotspot in the file (C=79, 185 statements). Extracts 7 single-responsibility helpers; the main function drops to ~63 LOC and C<10.Salvage of #27784 (which was a rebase of #23968) onto current
main. The contributor's branch was stale; one conflict resolved (the inline thinking-block management comment block inconvert_messages_to_anthropicwas extracted into_manage_thinking_signatures's docstring, which already had stricter wording — "Azure AI Foundry" + "AWS Bedrock" supersedes main's "Microsoft Foundry" comment).The 3 recent commits that landed on
anthropic_adapter.pysince the contributor's branch point (#9df9816d Azure Entra ID, #f0c6d591 MiniMax beta scoping, #73407b1e Bearer auth for Azure Foundry) all live outsideconvert_messages_to_anthropicand merge cleanly through.Changes
agent/anthropic_adapter.py— extract 7 helpers; main function shrinks 395 → 63 LOC._convert_assistant_message_convert_tool_message_to_result_convert_user_message_strip_orphaned_tool_blocks_merge_consecutive_roles_manage_thinking_signatures_evict_old_screenshotsMutating helpers update
resultin place (matches inline original)._merge_consecutive_rolesreturns a new list — caller rebinds.Honest accounting
The +5 violation-count "regression" is the rule working as intended — one C=79 monolith becomes 4 still-above-threshold helpers (C=12/13/15/18). Max complexity drops 77%, each new helper fits on one screen and is individually testable. That's the actual goal of C901.
Test plan
tests/agent/test_anthropic_adapter.py: 152/152 passtests/agent/test_auxiliary_client.py: 172/172 passtests/agent/test_azure_identity_adapter.py: 39/39 passtests/agent/test_bedrock_1m_context.py: 2/2 passpy_compileclean onagent/anthropic_adapter.pyOriginal PR author (@kshitijk4poor) noted 14 OAuth-credential tests failing on their branch — those all pass cleanly on current
mainafter this cherry-pick.Attribution
Authorship preserved via cherry-pick: commit by @kshitijk4poor, committer by @teknium1. AUTHOR_MAP already has
82637225+kshitijk4poor@users.noreply.github.com → kshitijk4poor.Closes #27784.
Infographic