Repository navigation
Add Tool calls and tool Result in DB - #331
sanskaraggarwal2025 wants to merge 28 commits into
Conversation
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 56 minutes and 47 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (27)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces agentic chat functionality, enabling models to utilize tools such as Brave Search and Cloudflare-based web scraping. The implementation includes updates to the database schema and DAO layer to track tool usage and costs, a new agentic service layer, and frontend configuration options. Review feedback identified several improvement opportunities, including making rate-limit sleep durations context-aware, implementing more robust detection for structured message content, and adopting safer URL construction practices for API requests.
| var waitSeconds int | ||
| fmt.Sscanf(retryAfter, "%d", &waitSeconds) | ||
| if waitSeconds > 0 && waitSeconds <= 60 { // Max 60 seconds wait | ||
| time.Sleep(time.Duration(waitSeconds) * time.Second) |
There was a problem hiding this comment.
time.Sleep is not context-aware. If the request context is cancelled while the tool is sleeping during a rate-limit backoff, it will continue to block the goroutine until the sleep duration expires. Use a select statement with ctx.Done() and time.After() instead.
select {
case <-ctx.Done():
return nil, ctx.Err()
case <-time.After(time.Duration(waitSeconds) * time.Second):
}| if strings.HasPrefix(msg.Content, "[") && strings.HasSuffix(msg.Content, "]") { | ||
| var textContents []*pb.MessageContent | ||
| if err := json.Unmarshal([]byte(msg.Content), &textContents); err != nil { | ||
| return nil, fmt.Errorf("failed to parse text content: %w", err) | ||
| } | ||
| contents = append(contents, textContents...) |
There was a problem hiding this comment.
The check for JSON content in msg.Content is fragile. If a user sends a message that happens to start with [ and end with ], json.Unmarshal will be attempted and might fail, causing the entire agentic session build to error out. Consider a more robust way to distinguish between raw text and structured message content, perhaps by checking the msg.Type or using a specific prefix.
| return nil, fmt.Errorf("query parameter is required") | ||
| } | ||
|
|
||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, t.apiURL+"?q="+url.QueryEscape(query), nil) |
There was a problem hiding this comment.
Constructing the URL by simple string concatenation is risky if t.apiURL already contains query parameters. It is safer to use url.Parse and url.Values to manage query parameters.
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, t.apiURL+"?q="+url.QueryEscape(query), nil) | |
| u, err := url.Parse(t.apiURL) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to parse api url: %w", err) | |
| } | |
| q := u.Query() | |
| q.Set("q", query) | |
| u.RawQuery = q.Encode() | |
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil) |
No description provided.