-
Notifications
You must be signed in to change notification settings - Fork 66
Modify Claude agent #165
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
base: main
Are you sure you want to change the base?
Modify Claude agent #165
Conversation
|
📝 Documentation updates detected! New suggestion: Add comprehensive Claude agent documentation with PDF support for PR #165 |
The agent now handles EmbeddedResource and converts it to Claude's BetaDocumentBlock format. Also fixed a small bug where the URI needed to be converted from Pydantic AnyUrl to string before processing
| "media_type": mime_type, | ||
| "data": base64_data | ||
| } | ||
| } |
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.
Bug: Document Blocks Missing Crucial Filename
The document_to_content_block function accepts a filename parameter but never includes it in the returned document block dictionary. The filename is extracted and passed to this function at line 339 but is silently discarded, which likely breaks PDF document handling since Claude's document blocks typically require the filename field to properly identify the document.
Parth220
left a comment
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 changes to how the model ingests the data is reasonable, but I won't add the logging since the telemetry tracks it.
| def _log_anthropic_request(request_kwargs: dict[str, Any]) -> None: | ||
| """Log Anthropic API request JSON to file.""" | ||
| try: | ||
| # Serialize the request data | ||
| log_data = _serialize_for_json(request_kwargs) | ||
|
|
||
| # Remove API key if present (for security) | ||
| if "api_key" in log_data: | ||
| log_data["api_key"] = "***REDACTED***" | ||
|
|
||
| request_json = json.dumps(log_data, indent=2, ensure_ascii=False, default=str) | ||
|
|
||
| with open(_ANTHROPIC_LOG_FILE, "a", encoding="utf-8") as f: | ||
| f.write("=" * 80 + "\n") | ||
| f.write("ANTHROPIC API REQUEST\n") | ||
| f.write("=" * 80 + "\n") | ||
| f.write(request_json + "\n") | ||
| f.write("\n") | ||
| except Exception as e: | ||
| logger.warning(f"Failed to log Anthropic request: {e}") | ||
| import traceback | ||
| logger.debug(traceback.format_exc()) | ||
|
|
||
|
|
||
| def _log_anthropic_response(response: Any) -> None: | ||
| """Log Anthropic API response JSON to file.""" | ||
| try: | ||
| # Convert response to dict | ||
| response_dict = {} |
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.
Can you remove this?
This info is already tracked via the telemetry through the trace decorator.
Add PDF document support
Note
Adds PDF/document handling in tool results and logs Anthropic API requests/responses to a file with robust serialization.
types.EmbeddedResource(e.g., PDFs) informat_tool_results, emittingdocumentblocks viaBetaDocumentBlock(with fallback type).tool_use_content_blockto acceptBetaDocumentBlock; adddocument_to_content_blockhelper.beta.messages.create._serialize_for_json,_serialize_content,_serialize_usageutilities and file logging helpers_log_anthropic_request/_log_anthropic_response.ANTHROPIC_API_LOG_FILE(defaults toanthropic_api_logs.txt).Written by Cursor Bugbot for commit ef92791. This will update automatically on new commits. Configure here.