The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
docs	Ready	Preview	Comment	Nov 5, 2025 8:44am

Pull Request Review: Update API Response Format in SDKs

Summary

This PR updates both Python and TypeScript SDKs to handle the new guard API response format while maintaining backward compatibility with the legacy format. The changes are well-structured and demonstrate good software engineering practices.

Code Quality and Best Practices

Strengths

• Excellent backward compatibility: The implementation gracefully handles both new (object) and legacy (JSON string) formats for content, preventing breaking changes for existing users
• Consistent implementation: Both Python and TypeScript SDKs follow the same patterns, making the codebase maintainable
• Clear documentation: The CHANGELOG.md clearly documents the breaking changes and migration path
• Type safety: TypeScript types are properly updated with GuardMessageContent interface, and Python uses TypedDict appropriately
• Deprecation handling: The code supports both reasoning (new) and reasoning_content (deprecated) fields with proper fallback logic

Suggestions

1. Type annotations in Python: In sdk/python/src/superagent_ai/client.py:115, consider adding a return type annotation to improve clarity:

   def _parse_decision(content: Optional[Union[GuardMessageContent, str]]) -> Optional[GuardDecision]:

   (Already correct - good job!)

2. Error handling: The parseDecision function silently returns undefined for invalid JSON. Consider logging a warning for debugging purposes, especially during the transition period:

   } catch (error) {
     // Consider: console.warn('Failed to parse guard content as JSON:', error);
     return undefined;
   }

Potential Bugs or Issues

Minor Issues

1. TypeScript line 204-211 (sdk/typescript/src/index.ts): The logic for handling both object and string content is correct, but there's a subtle issue - if the API returns a malformed object (not a string, but also not a valid GuardMessageContent), it will still try to process it. Consider adding validation:

   if (typeof content === "object") {
     payload = content as Record<string, unknown>;
   } else if (typeof content === "string") {
     // ...
   } else {
     return undefined; // Handle unexpected types
   }

2. Python line 119-127 (sdk/python/src/superagent_ai/client.py): Similar to TypeScript, the isinstance(content, dict) check assumes any dict is valid. The code handles this gracefully downstream, but you might want to add early validation.

Edge Cases Handled Well

• Empty content returns None/undefined appropriately
• Non-JSON strings are caught and handled
• Missing fields in the payload are handled gracefully

Performance Considerations

Positive

• No unnecessary parsing: The code correctly checks isinstance(content, dict) / typeof content === "object" before attempting JSON parsing, avoiding redundant work for the new API format
• Efficient field access: Using ?? operator (TypeScript) and or operator (Python) for fallback logic is performant

Neutral

• The double lookup (message.get("reasoning") or message.get("reasoning_content")) is negligible but could be cached if the message object is used extensively elsewhere (not the case here)

Security Concerns

Good Practices

• No injection vulnerabilities: The code doesn't execute or eval any user-provided content
• Type safety: Strong typing prevents many common security issues
• Safe JSON parsing: Using try-catch blocks prevents crashes from malformed data

No Major Concerns

• The changes don't introduce any new security vulnerabilities
• The API response parsing is defensive and safe

Test Coverage

Current State

• Good coverage: Existing tests cover the main use cases (guard pass/block, redact, verify)
• Tests run against live API: Tests use real API endpoints, which validates end-to-end functionality

Gaps and Recommendations

IMPORTANT: Missing explicit tests for the new response format changes:

1. Missing: New API format tests - Add tests that explicitly verify handling of the new object-based content:

   # Python test
   def test_parse_decision_handles_object_content():
       from superagent_ai.client import _parse_decision
       
       # New API format - content as dict
       content = {
           "classification": "block",
           "violation_types": ["injection"],
           "cwe_codes": ["CWE-89"]
       }
       decision = _parse_decision(content)
       assert decision["status"] == "block"
       assert decision["violation_types"] == ["injection"]

2. Missing: Legacy format tests - Add tests that verify backward compatibility:

   // TypeScript test
   it("handles legacy JSON string content format", () => {
     const legacyContent = '{"status": "pass"}';
     const decision = parseDecision(legacyContent);
     expect(decision?.status).toBe("pass");
   });

3. Missing: Reasoning field migration tests - Test both reasoning and reasoning_content fields:

   def test_reasoning_field_fallback():
       # Test that new 'reasoning' field takes precedence
       # Test that 'reasoning_content' is used as fallback

4. Missing: Edge case tests:

   • Test with content being null/undefined
   • Test with malformed object content
   • Test with both reasoning and reasoning_content present (verify precedence)

Test Coverage Summary

• Unit tests: NEEDED - Add unit tests for _parse_decision / parseDecision functions
• Integration tests: GOOD - Existing tests cover API integration
• Edge cases: PARTIAL - Some edge cases handled in code but not explicitly tested

Overall Assessment

Strengths

This is a well-implemented PR that demonstrates:

• Strong backward compatibility design
• Consistent cross-language implementation
• Good defensive programming practices
• Clear documentation of breaking changes

Recommendations Priority

High Priority:

1. Add unit tests for the format conversion logic (both Python and TypeScript)
2. Add tests verifying backward compatibility with legacy format
3. Add tests for the reasoning field migration

Medium Priority:
4. Consider adding type validation for object content
5. Consider adding debug logging for JSON parse failures (helpful during migration)

Low Priority:
6. Document the migration timeline (when will reasoning_content be fully deprecated?)

Verdict

The code quality is high and the implementation is solid. The main concern is the lack of explicit test coverage for the new functionality. I recommend adding the suggested unit tests before merging to ensure the backward compatibility logic works as intended and to prevent regressions.

Once tests are added, this PR will be ready to merge. Great work on maintaining backward compatibility!

🤖 Review generated with Claude Code

Pull Request Review: Update API Response Format in SDKs

Overview

This PR updates both TypeScript and Python SDKs to handle a breaking change in the Guard API response format. The implementation maintains backward compatibility while supporting the new response structure.

✅ Strengths

1. Backward Compatibility

• Excellent approach to handle both old and new API formats gracefully
• The dual handling of reasoning vs reasoning_content fields ensures no disruption for existing users
• TypeScript: sdk/typescript/src/index.ts:370
• Python: sdk/python/src/superagent_ai/client.py:265

2. Type Safety Improvements

• Added proper TypeScript interfaces for the new response structure (GuardMessageContent)
• Python TypedDict definitions maintain type clarity
• Union types appropriately used for backward compatibility: content?: GuardMessageContent | string

3. Comprehensive Changelog

• Breaking changes clearly documented with migration guidance
• Users are informed about the changes without requiring immediate code updates

🔍 Code Quality Observations

1. Consistent Implementation
Both SDKs implement the same logic pattern:

• Check for dict/object content first (new format)
• Fall back to JSON string parsing (legacy format)
• Gracefully handle parsing errors

2. Error Handling
The error handling in both implementations is solid, with try-catch blocks protecting against malformed JSON in the legacy format.

🐛 Potential Issues

1. Type Narrowing in TypeScript (Minor) - sdk/typescript/src/index.ts:205-215

The type check if (typeof content === "object") could be more precise. In JavaScript, null is also an object, and arrays are objects. Consider:

if (typeof content === "object" && content !== null && !Array.isArray(content)) {
  payload = content as Record<string, unknown>;
}

However, given the API contract and the subsequent validation, this is likely not a practical issue but worth noting for defensive programming.

2. Python Type Checking - sdk/python/src/superagent_ai/client.py:120

The isinstance(content, dict) check is good, but the TypedDict definition includes total=False, which means all fields are optional. Consider if you need stricter validation to ensure the required fields (classification) are present before treating it as the new format.

3. Missing Tests for New Format

The existing tests appear to test the SDK methods but don't explicitly verify:

• That the new object-based content format is handled correctly
• That the new reasoning field (vs reasoning_content) is properly extracted
• That both model and finish_reason fields are accessible

Consider adding test cases that mock responses with the new format explicitly.

🔒 Security Considerations

No major security concerns identified:

• JSON parsing is properly wrapped in try-catch
• No eval() or unsafe deserialization
• Type validation prevents injection of unexpected data types
• The backward compatibility doesn't open attack vectors

⚡ Performance Considerations

Minimal Performance Impact:

• The additional type check (isinstance in Python, typeof in TypeScript) is negligible
• No additional network calls or heavy computation
• The fallback to JSON.parse only occurs for legacy responses

🧪 Test Coverage Assessment

Current State:

• Existing tests cover the main happy paths and edge cases for guard, redact, and verify methods
• Tests validate the structure of responses and callback invocations
• Good coverage for validation error cases

Gaps:

1. No specific tests for the new response format (object-based content)
2. No tests verifying the reasoning field is preferred over reasoning_content
3. No tests for the new model and finish_reason fields
4. No explicit tests for backward compatibility (legacy string content format)

Recommendation: Add integration or unit tests that:

# Python example
async def test_guard_handles_new_response_format():
    # Mock response with new format
    mock_response = {
        "choices": [{
            "message": {
                "content": {
                    "classification": "pass",
                    "violation_types": []
                },
                "reasoning": "New format reasoning"
            },
            "finish_reason": "stop"
        }],
        "model": "gpt-4",
        "usage": {...}
    }
    # Test that it's handled correctly

📝 Recommendations

Priority: Low to Medium

1. Add explicit tests for the new response format to ensure the migration works as expected
2. Consider adding runtime validation to log when legacy format is detected (for monitoring adoption)
3. Improve TypeScript type guard for the content field check
4. Document the deprecation timeline for reasoning_content if applicable

✨ Overall Assessment

Verdict: Approve with Minor Suggestions ✅

This is a well-executed backward-compatible API migration. The code changes are clean, consistent across both SDKs, and maintain the existing test coverage. The changelog is comprehensive and user-friendly.

The suggestions above are minor improvements rather than blockers. The PR is ready to merge, though adding tests for the new format would strengthen confidence in the migration.

Great work on maintaining backward compatibility! 👏