-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fix: support sidebar max rate-limit handling restored #28706
Conversation
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.
PR Summary
This PR improves rate-limit handling in the Sidebar Max feature by removing unnecessary configuration files and implementing more robust error handling with automatic retries.
- Increased default retry duration from 15s to 180s in
ee/support_sidebar_max/views.py
for better rate limit recovery - Added automatic message retry functionality in
sidePanelMaxAILogic.ts
after rate limit period expires - Switched from parsing response content to using HTTP status codes for more reliable rate limit detection
- Removed
.flox
configuration directory and its contents (env.json, .gitignore, direnv-setup.sh) as they're no longer needed - Enhanced UI feedback in
sidePanelMaxChatInterface.tsx
with clearer rate limit and error state messaging
💡 (2/5) Greptile learns from your feedback when you react with 👍/👎!
6 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
.flox/.gitignore
Outdated
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.
logic: Removing .gitignore could cause temporary and generated files to be tracked. Consider keeping the file to prevent accidental commits of build artifacts, logs, and cache files.
.flox/env/direnv-setup.sh
Outdated
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.
logic: Removing this script will break environment setup for new developers. If direnv is still a requirement, ensure there is an alternative setup method documented or provided.
error.message.includes('524') || | ||
error.message.includes('529') | ||
) { | ||
const retryPeriod = (error as any).data?.retry_after || 180 |
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.
style: Unsafe type assertion to any. Consider adding proper error type handling or interface
Size Change: +5 B (0%) Total Size: 1.21 MB ℹ️ View Unchanged
|
Restored those three .flox files I'd accidentally omitted: |
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.
Somewhat low-context stamp
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
Problem
Sidebar Max rate-limit handling wasn't working. Now working in an MVP way.