-
-
Notifications
You must be signed in to change notification settings - Fork 51
Anthropic endpoint for claude code #45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Anthropic endpoint for claude code #45
Conversation
|
Starting my review of the Anthropic endpoint implementation. This is a significant addition bringing Claude Code compatibility via format translation between Anthropic and OpenAI APIs. Analyzing the endpoint handlers, model definitions, and streaming wrapper now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall Assessment
This PR delivers a well-structured Anthropic API compatibility layer, enabling Claude Code and other Anthropic clients to use the proxy via format translation. The implementation demonstrates solid understanding of both API formats and handles edge cases thoughtfully (e.g., forcing high thinking for Opus models, defensive error handling in streaming).
Verdict: The code quality is good with comprehensive error handling and proper Anthropic error format responses. There are two minor improvements suggested inline, but neither is blocking.
Architectural Feedback
The approach of translating Anthropic format → OpenAI format → processing → translating back is sound and aligns with the existing proxy architecture. The streaming wrapper correctly maintains state across the SSE event lifecycle, and the block index tracking for tool calls is well-implemented.
One architectural note: The code adds ~940 lines to main.py, which is already substantial. Consider whether these Anthropic-specific handlers could be extracted to a separate module (e.g., anthropic_compat.py) in a future refactor to improve maintainability.
Key Suggestions
-
Tool ID collision risk (inline comment): The fallback
int(time.time())for tool IDs could collide with parallel calls. Using UUID is recommended. -
Unused state variables (inline comment):
accumulated_textandaccumulated_thinkingin the streaming wrapper are tracked but never used.
Minor Observations (Not Inline)
-
Token counting for tools: The
count_tokensendpoint serializes tool definitions to JSON text and counts that. This is an approximation since tokenizers may handle function schemas differently. Consider documenting this limitation. -
Reasoning effort thresholds: The budget_tokens → reasoning_effort mapping (32000/10000/5000) works but lacks inline documentation explaining the rationale.
-
Testing: No tests are included in this PR for the new endpoints. Consider adding test coverage for the format translation functions and endpoint handlers.
Questions for the Author
-
Are the
accumulated_textandaccumulated_thinkingvariables in the streaming wrapper intended for future use (e.g., logging), or are they leftover from development? -
Is there a plan to add integration tests for the Anthropic endpoint compatibility?
This review was generated by an AI assistant.
src/proxy_app/main.py
Outdated
| content_blocks.append( | ||
| { | ||
| "type": "tool_use", | ||
| "id": tc.get("id", f"toolu_{int(time.time())}"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using int(time.time()) as a fallback tool ID could cause collisions if multiple tool calls are processed within the same second. Since uuid is already imported in this file, consider using a UUID for more robust uniqueness.
| "id": tc.get("id", f"toolu_{int(time.time())}"), | |
| "id": tc.get("id", f"toolu_{uuid.uuid4().hex[:12]}"), |
src/proxy_app/main.py
Outdated
| thinking_block_started = False | ||
| current_block_index = 0 | ||
| accumulated_text = "" | ||
| accumulated_thinking = "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variables accumulated_text and accumulated_thinking are initialized and updated throughout the streaming wrapper but are never read. If these are intended for debugging or future use (e.g., logging the complete response), consider adding a comment explaining their purpose. Otherwise, removing them would reduce cognitive overhead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds Anthropic API endpoint support to enable compatibility with Claude Code and other Anthropic API clients. The implementation translates between Anthropic's message format and the existing OpenAI-compatible format used internally.
Key changes:
- Added Anthropic Messages API endpoint (
/v1/messages) with streaming and non-streaming support - Implemented format translation between Anthropic and OpenAI request/response structures
- Added Claude extended thinking parameter validation to ensure max_tokens > thinking_budget
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 14 comments.
| File | Description |
|---|---|
src/rotator_library/providers/antigravity_provider.py |
Adds validation logic to ensure max_tokens exceeds thinking_budget for Claude models with extended thinking enabled |
src/proxy_app/main.py |
Implements Anthropic Messages API endpoint with request/response translation, streaming support, token counting endpoint, and dual authentication (x-api-key and Bearer token) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/proxy_app/main.py
Outdated
| anthropic_usage["cache_read_input_tokens"] = details["cached_tokens"] | ||
|
|
||
| return { | ||
| "id": openai_response.get("id", f"msg_{int(time.time())}"), |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using time.time() as a fallback for message IDs can generate duplicate IDs if multiple messages are processed within the same second. Consider using uuid or a more robust unique ID generation strategy for the fallback case, similar to request_id generation at line 1672.
| # CRITICAL: For Claude with extended thinking, max_tokens MUST be > thinking.budget_tokens | ||
| # Per Claude docs: https://docs.claude.com/en/docs/build-with-claude/extended-thinking | ||
| # If this constraint is violated, the API returns 400 INVALID_ARGUMENT |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment references Claude documentation, but the logic is being applied to all Claude models via the antigravity provider. Consider clarifying which specific models or model versions this constraint applies to, as extended thinking may not be available for all Claude models.
| # CRITICAL: For Claude with extended thinking, max_tokens MUST be > thinking.budget_tokens | |
| # Per Claude docs: https://docs.claude.com/en/docs/build-with-claude/extended-thinking | |
| # If this constraint is violated, the API returns 400 INVALID_ARGUMENT | |
| # CRITICAL: For Claude models that support extended thinking (e.g., Sonnet 4.5 and Opus 4.5 | |
| # via the Antigravity proxy), max_tokens MUST be > thinking.budget_tokens when | |
| # extended thinking is enabled. Per Claude docs: | |
| # https://docs.claude.com/en/docs/build-with-claude/extended-thinking | |
| # If this constraint is violated for those models, the API can return 400 INVALID_ARGUMENT |
| # Check x-api-key first (Anthropic style) | ||
| if x_api_key and x_api_key == PROXY_API_KEY: | ||
| return x_api_key | ||
| # Fall back to Bearer token (OpenAI style) | ||
| if auth and auth == f"Bearer {PROXY_API_KEY}": | ||
| return auth | ||
| raise HTTPException(status_code=401, detail="Invalid or missing API Key") |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When PROXY_API_KEY is not set or empty (open access mode), this function will always raise an HTTPException because neither condition will match. This is inconsistent with verify_api_key at line 794 which allows access when PROXY_API_KEY is not set. Consider adding a check similar to line 794 to allow open access mode.
src/proxy_app/main.py
Outdated
| elif "opus" in body.model.lower(): | ||
| # Force high thinking for Opus models when no thinking config is provided | ||
| # Opus 4.5 always uses the -thinking variant, so we want maximum thinking budget | ||
| # Without this, the backend defaults to thinkingBudget: -1 (auto) instead of high | ||
| openai_request["reasoning_effort"] = "high" | ||
| openai_request["custom_reasoning_budget"] = True |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The model name matching logic uses a simple substring check ("opus" in body.model.lower()), which could match unintended model names. Consider using a more specific pattern or a list of known Opus model names to avoid false positives with models that might contain "opus" in their name but aren't Claude Opus models.
src/proxy_app/main.py
Outdated
| # Build OpenAI-compatible request | ||
| openai_request = { | ||
| "model": body.model, | ||
| "messages": openai_messages, | ||
| "max_tokens": body.max_tokens, | ||
| "stream": body.stream or False, | ||
| } | ||
|
|
||
| if body.temperature is not None: | ||
| openai_request["temperature"] = body.temperature | ||
| if body.top_p is not None: | ||
| openai_request["top_p"] = body.top_p | ||
| if body.stop_sequences: | ||
| openai_request["stop"] = body.stop_sequences | ||
| if openai_tools: | ||
| openai_request["tools"] = openai_tools | ||
| if openai_tool_choice: | ||
| openai_request["tool_choice"] = openai_tool_choice |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The top_k parameter from the Anthropic request (defined in AnthropicMessagesRequest at line 301) is not being mapped to the OpenAI request. Anthropic's top_k parameter controls sampling diversity and should be included if provided. Consider adding a check similar to lines 1699-1702 to include top_k in the openai_request when it's not None.
src/proxy_app/main.py
Outdated
| openai_request["tools"] = openai_tools | ||
| if openai_tool_choice: | ||
| openai_request["tool_choice"] = openai_tool_choice | ||
|
|
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The metadata parameter from the Anthropic request (defined in AnthropicMessagesRequest at line 306) is not being mapped or handled. While metadata may not directly map to OpenAI format, it could be useful for logging or tracking purposes. Consider whether this parameter should be preserved for request logging or passed through as custom fields.
| # Preserve Anthropic metadata for logging/tracking or downstream use | |
| metadata = anthropic_request.get("metadata") | |
| if metadata is not None: | |
| openai_request["metadata"] = metadata |
| if current_max_tokens <= thinking_budget: | ||
| lib_logger.warning( | ||
| f"max_tokens ({current_max_tokens}) must be > thinkingBudget ({thinking_budget}). " | ||
| f"Adjusting to {min_required_tokens}" | ||
| ) | ||
| gen_config["maxOutputTokens"] = min_required_tokens |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition checks if current_max_tokens <= thinking_budget, but the adjustment only occurs in this case. However, the comment states that max_tokens MUST be > thinking_budget. If current_max_tokens is between thinking_budget + 1 and thinking_budget + 1024 (exclusive), it satisfies the API requirement (> thinking_budget) but might not provide enough buffer for the actual response content. Consider changing the condition to check if current_max_tokens < min_required_tokens instead.
src/proxy_app/main.py
Outdated
| # Close any open thinking block | ||
| if thinking_block_started: | ||
| yield f'event: content_block_stop\ndata: {{"type": "content_block_stop", "index": {current_block_index}}}\n\n' | ||
| current_block_index += 1 | ||
| thinking_block_started = False | ||
|
|
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable thinking_block_started is not used.
| # Close any open thinking block | |
| if thinking_block_started: | |
| yield f'event: content_block_stop\ndata: {{"type": "content_block_stop", "index": {current_block_index}}}\n\n' | |
| current_block_index += 1 | |
| thinking_block_started = False |
src/proxy_app/main.py
Outdated
| if content_block_started: | ||
| yield f'event: content_block_stop\ndata: {{"type": "content_block_stop", "index": {current_block_index}}}\n\n' | ||
| current_block_index += 1 | ||
| content_block_started = False |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable content_block_started is not used.
| content_block_started = False |
src/proxy_app/main.py
Outdated
| continue | ||
|
|
||
| delta = choices[0].get("delta", {}) | ||
| finish_reason = choices[0].get("finish_reason") |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable finish_reason is not used.
| finish_reason = choices[0].get("finish_reason") |
|
Problem I noticed from the start - only files edited are antigravity and main.py. main.py does not do anything by itself - logic should be in client.py or helper files called by it. Tldr the logic should be in the library. |
|
Sure, go ahead Edit: oh you want me to make these changes, maybe i misunderstood? |
Preferably you, but it can go either way. I didn't set up any guidance for issues or PRs, contributing guidelines, etc. - so it is technically my fault you didn't do that. Not sure when I'll get around to this, but soon-ish I'll review and work on making this be a part of the library proper. This is full on anthropic translation layer, right? If it works like the bot says - it should be compatible with all providers automatically. |
|
@mirrobot-agent what will it take to implement what I said? To move it to the library and make it a separate method for anthropic endpoint proper. Converting to and from anthropic to openai, and from opeanai any existing provider can work as normal. |
|
@Mirrowel, I'm analyzing the codebase to provide a detailed plan for moving the Anthropic translation layer from Give me a moment to review the architecture... |
|
@Mirrowel, I've completed my analysis of the codebase. Here's a detailed plan for moving the Anthropic translation layer from SummaryThe goal is to extract the ~700 lines of Anthropic format translation code from
What Needs to MoveFrom 1. Pydantic Models (~130 lines, lines 218-348)
2. Format Translation Functions (~200 lines, lines 822-1068)
3. Streaming Wrapper (~280 lines, lines 1071-1351)
Proposed Library ArchitectureNew Files to CreateIntegration Point:
|
| Component | Lines of Code | Complexity |
|---|---|---|
Create models.py |
~130 | Low (copy/move) |
Create translator.py |
~200 | Medium (refactor for reuse) |
Create streaming.py |
~280 | Medium (remove FastAPI deps) |
Update client.py |
~80 | Medium (new methods) |
Simplify main.py |
-650 / +50 | Low (remove/replace) |
| Total | ~560 new, -650 removed | Medium |
Questions
- Should the Anthropic models/functions be exposed in the library's public API (via
__init__.py), or kept internal? - Do you want me to implement this refactoring, or just provide this plan for FammasMaz or yourself to implement?
Let me know how you'd like to proceed!
This analysis was generated by an AI assistant.
|
@mirrobot-agent implement this, and open a separate PR with that against this PR. Make sure it is comprehensive and integrated with any provider library and proxy work with. Maintain opeanai- compatible format internally, and convert to and from it for providers, so they can essentially be unchanged. Try to do this iteratively, so make a plan and go along with it, committing as you go. You are not allowed to commit to this PR - only the new one you will open. |
|
@Mirrowel, I'm on it! I'll implement the refactoring plan to move the Anthropic translation layer from My PlanI'll work iteratively, committing as I go:
Starting now... 🚀 This task is being handled by an AI assistant. |
Refactoring PR CreatedI've created PR #47 which refactors the Anthropic translation layer from this PR into the Key Changes in #47:
Benefits:
See: #47 |
…atibility - Add /v1/messages endpoint with Anthropic-format request/response - Support both x-api-key and Bearer token authentication - Implement Anthropic <-> OpenAI format translation for messages, tools, and responses - Add streaming wrapper converting OpenAI SSE to Anthropic SSE events - Handle tool_use blocks with proper stop_reason detection - Fix NoneType iteration bug in tool_calls handling
- Add AnthropicThinkingConfig model and thinking parameter to request - Translate Anthropic thinking config to reasoning_effort for providers - Handle reasoning_content in streaming wrapper (thinking_delta events) - Convert reasoning_content to thinking blocks in non-streaming responses
When no thinking config is provided in the request, Opus models now automatically use reasoning_effort=high with custom_reasoning_budget=True. This ensures Opus 4.5 uses the full 32768 token thinking budget instead of the backend's auto mode (thinkingBudget: -1) which may use less. Opus always uses the -thinking variant regardless, but this change guarantees maximum thinking capacity for better reasoning quality.
…ling - Add validation to ensure maxOutputTokens > thinkingBudget for Claude extended thinking (prevents 400 INVALID_ARGUMENT API errors) - Improve streaming error handling to send proper message_start and content blocks before error event for better client compatibility - Minor code formatting improvements
Track each tool_use block index separately and emit content_block_stop for all blocks (thinking, text, and each tool_use) when stream ends. Fixes Claude Code stopping mid-action due to malformed streaming events.
…nabled - Fixed bug where budget_tokens between 10000-32000 would get ÷4 reduction - Now any explicit thinking request sets custom_reasoning_budget=True - Added logging to show thinking budget, effort level, and custom_budget flag - Simplified budget tier logic (removed redundant >= 32000 check) Before: 31999 tokens requested → 8192 tokens actual (÷4 applied) After: 31999 tokens requested → 32768 tokens actual (full "high" budget)
When using /v1/chat/completions with Opus and reasoning_effort="high" or "medium", automatically set custom_reasoning_budget=true to get full thinking tokens instead of the ÷4 reduced default. This makes the OpenAI endpoint behave consistently with the Anthropic endpoint for Opus models - if you're using Opus with high reasoning, you want the full thinking budget. Adds logging: "🧠 Thinking: auto-enabled custom_reasoning_budget for Opus"
…treaming Claude Code and other Anthropic SDK clients require message_start to be sent before any other SSE events. When a stream completed quickly without content chunks, the wrapper would send message_stop without message_start, causing clients to silently discard all output.
Signed-off-by: Moeeze Hassan <[email protected]>
This reverts commit e80645e.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow-up Review
This incremental update adds a tool result thinking reminder feature for Claude and strengthens the interleaved thinking enforcement.
Assessment of New Changes:
The implementation is clean and well-integrated:
-
Tool Result Thinking Reminder: New feature that appends a
<system-reminder>block to tool results for Claude, reinforcing the interleaved thinking requirement. The implementation correctly handles both multimodal (list) and string content paths. Configurable viaANTIGRAVITY_ENABLE_CLAUDE_TOOL_RESULT_REMINDERenv var. -
Strengthened Interleaved Thinking Hint: The hint is now both prepended AND appended to system instructions. The prompt language was also strengthened ("IS UNCOMPROMISINGLY A MUST DO"). This is a reasonable strategy to improve Claude's compliance with thinking requirements.
-
Code Formatting: Import reordering and consistent line-break formatting throughout. No functional impact.
Minor Observations (not blocking):
-
The duplicate hint injection (prepend + append) doubles token overhead for this instruction. If token budget ever becomes a concern, one injection point may suffice.
-
For dict tool results, the
_system_reminderkey could theoretically collide with tool output. A more unique key like__anthropic_system_reminder__would reduce this edge case risk.
Previous Feedback Status:
Two non-blocking suggestions from my 2026-01-02 review remain open but are unrelated to this update:
⚠️ Code duplication:_reorder_assistant_content()duplicated betweentranslator.pyandantigravity_provider.py⚠️ Mutation side effect:_merge_all_of()mutates input schema in-place
Overall Status:
The new changes are solid and ready to merge. No blocking issues identified.
This review was generated by an AI assistant.
- Integrate refactored utilities from dev (gemini_shared_utils, etc.) - Keep Antigravity system prompts (critical for API) - Add Claude interleaved thinking features from feature branch - Ensure interleaved thinking hint comes AFTER Antigravity prompts
- Add explicit_budget parameter to _get_thinking_config - Cap Claude thinking budget at 31999 when explicit budget provided - Pass thinking_budget kwarg from Anthropic translator to provider
Ignore client's budget_tokens value and always use 31999 for Claude via Anthropic routes to ensure full thinking capacity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow-up Review
This update is a significant refactoring that extracts shared logic between Antigravity and Gemini CLI providers into reusable utility modules, reducing code duplication and improving maintainability.
Key Changes Reviewed:
-
New Utility Modules (
providers/utilities/):gemini_shared_utils.py- Common constants (FINISH_REASON_MAP,DEFAULT_SAFETY_SETTINGS) and helpers (env_bool,env_int,inline_schema_refs, etc.)gemini_file_logger.py- Transaction logging classesgemini_tool_handler.py- Tool schema transformation mixingemini_credential_manager.py- Credential/tier management mixinbase_quota_tracker.py- Abstract base for quota trackinggemini_cli_quota_tracker.py- Gemini CLI specific quota tracking
-
Antigravity System Instruction Injection:
- New
ANTIGRAVITY_AGENT_SYSTEM_INSTRUCTIONprepended to requests (mimics CLIProxyAPI Go implementation) - Identity override instruction to neutralize the Antigravity persona for user-defined identities
- Configurable via
ANTIGRAVITY_PREPEND_INSTRUCTION,ANTIGRAVITY_INJECT_IDENTITY_OVERRIDE,ANTIGRAVITY_USE_SHORT_PROMPTS
- New
-
Simplified Thinking Logic:
- Removed
_is_valid_thinking_signature()validation checks - Removed
custom_reasoning_budgetparameter (simplified to justreasoning_effort) - Removed
_merge_all_of()schema helper (addresses previous feedback ✅) - Added fallback signature:
"skip_thought_signature_validator"when signature is missing
- Removed
-
Quota Tracking Updates:
- Changed from percentage-based (
DEFAULT_QUOTA_COSTS) to max-requests-based (DEFAULT_MAX_REQUESTS) - Added
normalize_model_for_tracking()for public-facing model name normalization - Updated quota groups:
g3-pro,g3-flash,g25-flash,g25-lite(separated Flash Lite)
- Changed from percentage-based (
-
env:// Credential Support:
- Multiple environment credentials via
PROVIDER_N_ACCESS_TOKENpattern - Documented in README and DOCUMENTATION.md
- Multiple environment credentials via
Previous Feedback Status:
| Feedback | Status |
|---|---|
_merge_all_of() mutation side effect |
✅ Resolved (function removed) |
_reorder_assistant_content() duplication |
translator.py:18 and antigravity_provider.py:466 (low priority) |
Potential Behavioral Changes to Verify:
-
Cached Token Tracking Removed (
antigravity_provider.py:_build_usage()):cachedContentTokenCountis no longer extracted from Gemini metadataprompt_tokens_details.cached_tokensis no longer populated- This field is used by the Anthropic translator to calculate
cache_read_input_tokens - Impact: Requests via Anthropic compat layer will not show cache hit information
- Question: Intentional simplification or regression?
-
anthropic-beta Header Removed:
- The
anthropic-beta: interleaved-thinking-2025-05-14header is no longer added for Claude thinking models - Previously added in acompletion() when
thinking_enabled=True - Question: Is this no longer required by the API?
- The
-
Claude Interleaved Thinking Hint:
- Now only prepends the hint (removed the additional append at end of system instructions)
- Simplified from prepend+append to just prepend
Overall Status:
The refactoring is well-structured and significantly improves code organization. The behavioral changes noted above are non-blocking but worth verifying they were intentional.
This review was generated by an AI assistant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow-up Review
This incremental update contains code formatting improvements and a thinking budget simplification in translator.py.
Assessment of New Changes:
-
Formatting cleanup (lines 9-13, 20-24, 32-35, 43-46, 83-88): Multi-line formatting for better readability on signature checks, image_url dicts, and regex patterns. No functional impact.
-
Thinking budget simplification (lines 53-76): Now always uses
31999as the thinking budget for Claude via Anthropic routes, removing the previous pass-through of client'sbudget_tokens. This is a reasonable simplification since 31999 is Claude's max thinking budget. Note that clients specifying lower budgets will now receive max thinking regardless. -
Opus model handling: Also applies the fixed budget for Opus models, ensuring consistent behavior.
Previous Feedback Status:
| Feedback | Status |
|---|---|
_reorder_assistant_content() duplication |
translator.py:18 and antigravity_provider.py (low priority) |
_merge_all_of() mutation side effect |
✅ Resolved (function removed) |
| Cached token tracking removal | ℹ️ Noted in previous review - awaiting clarification |
Overall Status:
Clean incremental update with no issues identified. The changes improve code formatting and simplify thinking configuration. Ready to merge.
This review was generated by an AI assistant.
…y lacks thinking blocks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow-up Review
This incremental update adds thinking history compatibility checking to the Anthropic translator, ensuring Claude API compliance when thinking is enabled mid-conversation.
Assessment of New Changes:
The implementation is clean and addresses a real edge case:
-
_history_supports_thinking()(lines 500-512): Correctly detects when conversation history contains assistant messages without thinking blocks. The logic properly handles the first-turn case (no assistant messages = compatible) and iterates through all assistant messages to verify thinking consistency. -
_inject_continue_for_fresh_thinking_turn()(lines 515-517): Simple utility that appends a synthetic user message to close the current turn, allowing Claude to start fresh with thinking enabled. -
Integration (lines 576-588): The
[Continue]injection is correctly gated onnot thinking_compatiblefor both explicit thinking config and implicit Opus model handling.
This mirrors similar logic in antigravity_provider.py (lines 1537-1610) but at the translator layer, providing earlier detection before messages reach the provider. Good layered approach.
Previous Feedback Status:
| Feedback | Status |
|---|---|
_reorder_assistant_content() duplication |
translator.py:18, antigravity_provider.py) - low priority |
_merge_all_of() mutation side effect |
✅ Resolved (function removed) |
| Cached token tracking removal | ℹ️ Noted previously - awaiting clarification |
Overall Status:
Clean implementation. One minor inline comment for consideration. Ready to merge.
This review was generated by an AI assistant.
| def _inject_continue_for_fresh_thinking_turn(openai_messages: List[dict]) -> List[dict]: | ||
| openai_messages.append({"role": "user", "content": "[Continue]"}) | ||
| return openai_messages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor (non-blocking): This function mutates the input list in-place while also returning it. Consider using return openai_messages + [{"role": "user", "content": "[Continue]"}] to avoid side effects, which would be safer for future maintainability.
The current implementation works correctly - this is just a stylistic suggestion.
Token counting endpoints (/v1/token-count and /v1/messages/count_tokens) were returning inaccurate counts because they didn't include the Antigravity preprompts that get injected during actual API calls. - Add get_antigravity_preprompt_text() helper to expose preprompt text - Update RotatingClient.token_count() to add preprompt tokens for Antigravity provider models Signed-off-by: Moeeze Hassan <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow-up Review
This incremental update adds preprompt-aware token counting for Antigravity provider models, ensuring token count endpoints accurately reflect actual API usage.
Assessment of New Changes:
The implementation is clean and well-designed:
-
get_antigravity_preprompt_text()(antigravity_provider.py:452-485): New helper function that correctly mirrors the logic from_inject_antigravity_system_instruction(), respecting all three configuration flags (PREPEND_INSTRUCTION,USE_SHORT_ANTIGRAVITY_PROMPTS,INJECT_IDENTITY_OVERRIDE). Good docstring explaining the purpose. -
token_count()enhancement (client.py:2489-2504): Properly detects Antigravity provider models and adds preprompt tokens to the base count. The lazy import pattern (from .providers.antigravity_provider import ...) avoids circular dependency issues, and thetry/except ImportErrorprovides graceful fallback.
Technical Notes:
- Token counts won't include Claude-specific hints (interleaved thinking reminder) that are added conditionally at runtime—this is an acceptable simplification since those hints are relatively small and model-dependent.
Previous Feedback Status:
| Feedback | Status |
|---|---|
_reorder_assistant_content() duplication |
translator.py, antigravity_provider.py) - low priority cleanup |
| All other previous feedback | ✅ Addressed |
Overall Status:
The changes are solid, well-documented, and ready to merge. No issues identified.
This review was generated by an AI assistant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow-up Review
This incremental update adds full URL support to the Quota Viewer TUI, allowing users to configure remotes using complete URLs (e.g., https://api.example.com/v1) instead of just host:port pairs.
Assessment of New Changes:
The implementation is clean and well-structured:
is_full_url()helper: Simple and correct check for URL prefixes_build_endpoint_url()with path deduplication: Intelligent handling to prevent/v1/v1/...duplication when base URL already contains a path segment. Good docstring with clear examples.check_connection()update: Correctly hits the root domain rather than the path (which would 404)- Display formatting: Properly handles display of full URLs vs host:port pairs throughout the UI
- Config type updates:
portnow acceptsOptional[Union[int, str]]with proper normalization
The user experience improvements (hints in add/edit dialogs, wider display columns for long URLs) are thoughtful additions.
Previous Feedback Status:
| Feedback | Status |
|---|---|
_reorder_assistant_content() duplication |
|
| All other previous feedback | ✅ Addressed |
Overall Status:
No issues identified. The changes are solid and ready to merge.
This review was generated by an AI assistant.
Resolve conflicts by adopting dev's interleaved thinking implementation and removing duplicate code from this branch.
Remove remaining references to removed interleaved thinking attributes that were brought in during merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow-up Review
This incremental update brings significant improvements across workflow reliability, logging architecture, and Antigravity provider capabilities.
Key Changes Reviewed:
-
Workflow Hardening (bot-reply.yml, pr-review.yml, compliance-check.yml):
- Bot user checks prevent infinite loops when bot comments trigger bot actions
- Comment validation filters triggers in code blocks/quotes (prevents false positives)
- Clean conditional propagation via
should_proceedoutput - Build workflow now correctly scoped to dev/main branches
-
Logging Architecture Refactoring:
- New
TransactionLoggerintransaction_logger.pyprovides unified, correlated logging - Clear separation: "Transaction Logging" (library-level with provider correlation) vs "Raw I/O Logging" (proxy boundary HTTP capture)
RawIOLogger(renamed fromDetailedLogger) clarifies purposeProviderLoggerbase class withAntigravityProviderLoggerextension- Removed scattered provider-specific loggers in favor of centralized system
- New
-
Antigravity Provider Enhancements:
- Tool name sanitization: New
_sanitize_tool_name()/_restore_tool_name()handles invalid characters (slashes) with collision detection - Schema handling: New
_score_schema_option(),_try_merge_enum_from_union(),_merge_all_of()for intelligent anyOf/oneOf/allOf handling - Interleaved thinking:
CLAUDE_INTERLEAVED_THINKING_HINTand user-message reminder injection - Unified streaming: Non-streaming now collects from streaming internally (simpler code path)
- Thinking model detection: Opus/-thinking suffix models always enable thinking
- Tool name sanitization: New
-
Library Updates:
- Version bump: 1.05 → 1.15
- Python requirement: 3.7 → 3.10
Previous Feedback Status:
| Feedback | Status |
|---|---|
_reorder_assistant_content() duplication |
translator.py:18 and antigravity_provider.py:538 (low priority cleanup) |
_merge_all_of() mutation concern |
ℹ️ Re-introduced with shallow copy; nested mutation still possible but likely acceptable given usage context |
Overall Status:
Excellent incremental update with no blocking issues. The logging consolidation significantly improves debuggability, and the workflow hardening prevents potential bot loops. Ready to merge.
This review was generated by an AI assistant.
Bring in latest dev changes including: - Docker support and workflows - Fair cycle rotation and custom usage caps - Smart cooldown waiting and fail-fast logic - Centralized library defaults - Dynamic custom OpenAI-compatible provider system - Interactive connection recovery
…remove legacy hacks Replaces ad-hoc thinking logic with a structured mapping from Anthropic `budget_tokens` to `reasoning_effort` levels. This change aligns the translation layer with standard provider capabilities and cleans up deprecated workarounds. - Implement `_budget_to_reasoning_effort` to convert token counts to reasoning levels (e.g., "low", "medium", "high", "granular"). - Remove legacy logic that forced max thinking budget for Claude Opus models. - Remove workaround for injecting "[Continue]" messages into conversation history. - Delete unused helper functions in `AntigravityProvider` (signature validation, content reordering, and explicit budget overrides).
|
Hey @FammasMaz! 👋 I've been reviewing this PR and did some refactoring to make the Anthropic translation layer more provider-agnostic. Changes proposed in #76:
The goal is to keep the translation layer pure - just format conversion between Anthropic ↔ OpenAI, without provider-specific logic baked in. See: #76 Ai-generated^^ |
…ompatibility This change introduces a hierarchical logging structure to better trace requests passing through the translation layer. - Update `TransactionLogger` to support nested directories (`parent_dir`) and custom filenames, allowing internal OpenAI transactions to be logged as children of the original Anthropic request. - Implement full response reconstruction in `anthropic_streaming_wrapper` to accumulate and log the final state of streaming interactions (including thinking blocks and tool calls). - Modify `RotatingClient` to pass logging context down to the translation layer. - Switch `proxy_app` to use `RawIOLogger` when enabled for better debugging of the proxy boundary.
The previous implementation using `delta.get("tool_calls", [])` would return `None` if the provider explicitly sent `"tool_calls": null`, bypassing the default value. This change ensures `tool_calls` always resolves to a list using the `or []` pattern, preventing potential errors during iteration.
Updates project documentation to reflect the new Anthropic API compatibility features: - **README.md**: Add setup guides for Claude Code and Anthropic Python SDK, plus API endpoint details. - **DOCUMENTATION.md**: Add deep dive into the `anthropic_compat` architecture, including translation logic and streaming behavior. - **Library Docs**: Document `anthropic_messages` and `anthropic_count_tokens` methods in `rotator_library`.
Important
Adds Anthropic endpoint for Claude code with format translation and updates to Antigravity provider for compatibility.
/v1/messagesand/v1/messages/count_tokensendpoints inmain.pyfor Anthropic API compatibility.anthropic_messagesandanthropic_count_tokensfunctions to handle requests and responses in Anthropic format.AnthropicTextBlock,AnthropicImageSource,AnthropicImageBlock,AnthropicToolUseBlock,AnthropicToolResultBlock,AnthropicMessage,AnthropicTool,AnthropicThinkingConfig,AnthropicMessagesRequest,AnthropicUsage,AnthropicMessagesResponse,AnthropicCountTokensRequest, andAnthropicCountTokensResponseinmain.py.anthropic_to_openai_messages,anthropic_to_openai_tools,anthropic_to_openai_tool_choice, andopenai_to_anthropic_responseinmain.pyfor format conversion.anthropic_streaming_wrapperinmain.pyto convert OpenAI streaming format to Anthropic streaming format._transform_to_antigravity_formatinantigravity_provider.pyto ensuremax_tokensis greater thanthinking.budget_tokensfor Claude models.This description was created by
for aef492b. You can customize this summary. It will automatically update as commits are pushed.