rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 6 Wiki Activity

feat: add native SAP AI Core support #50

Closed
rodin wants to merge 12 commits from feat/aicore-provider into main
pull from: feat/aicore-provider
merge into: rodin:main
Conversation 27 Commits 12 Files Changed 8
+1
1 changed files with 1 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit b92a968d93 - Show all commits
llm/aicore.go
+1
View File
51
 @@ -276,6 +276,7 @@ func (c *AICoreClient) CompleteAnthropic(ctx context.Context, model string, mess
req.Header.Set("Authorization", "Bearer "+token)
req.Header.Set("AI-Resource-Group", c.config.ResourceGroup)
req.Header.Set("Content-Type", "application/json")
req.Header.Set("anthropic-version", "2023-06-01")
sonnet-review-bot commented 2026-05-10 06:49:21 +00:00
Review
Copy Link
Copy Source

[MINOR] The TODO.md self-review correctly notes that CompleteAnthropic and CompleteOpenAI lack the Content-Length body validation that doRequest has in client.go. Given these methods duplicate the HTTP execution pattern rather than going through doRequest, any future fix to the retry/validation logic in doRequest won't automatically apply here. Consider extracting a shared HTTP execution helper for AI Core, or routing through doRequest via an adapter.

**[MINOR]** The `TODO.md` self-review correctly notes that `CompleteAnthropic` and `CompleteOpenAI` lack the Content-Length body validation that `doRequest` has in `client.go`. Given these methods duplicate the HTTP execution pattern rather than going through `doRequest`, any future fix to the retry/validation logic in `doRequest` won't automatically apply here. Consider extracting a shared HTTP execution helper for AI Core, or routing through `doRequest` via an adapter.
resp, err := c.http.Do(req)
if err != nil {
security-review-bot commented 2026-05-10 05:31:39 +00:00
Review
Copy Link
Copy Source

[MINOR] Unbounded response body read for Anthropic invoke responses (io.ReadAll). To defend against large or malformed payloads from AI Core or misconfiguration, consider adding a read limit or Content-Length checks like in llm.Client.doRequest.

**[MINOR]** Unbounded response body read for Anthropic invoke responses (io.ReadAll). To defend against large or malformed payloads from AI Core or misconfiguration, consider adding a read limit or Content-Length checks like in llm.Client.doRequest.
gpt-review-bot commented 2026-05-10 06:51:34 +00:00
Review
Copy Link
Copy Source

[MINOR] AI Core request paths do not validate response body length against Content-Length like client.doRequest does. Adding a similar check (and treating mismatches as retryable) would harden against truncated responses. This applies to both CompleteAnthropic and CompleteOpenAI.

**[MINOR]** AI Core request paths do not validate response body length against Content-Length like client.doRequest does. Adding a similar check (and treating mismatches as retryable) would harden against truncated responses. This applies to both CompleteAnthropic and CompleteOpenAI.
19