-
-
Notifications
You must be signed in to change notification settings - Fork 55
fix(chutes): enable quota display in TUI #90
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
Conversation
- Add model_quota_groups class attribute for client.py integration - Add get_models_in_quota_group() to map quota groups to virtual models - Add get_quota_groups() to expose available quota groups - Add get_usage_reset_config() to enable per_model tracking mode These changes enable the TUI quota viewer to correctly display the Chutes provider's daily quota (e.g., 297/300, 99% remaining).
|
I'm starting my review of the Chutes provider quota system. Looking into the new methods in |
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.
Overall Assessment
This PR correctly implements the necessary hooks to enable quota display for the Chutes provider in the TUI. The addition of the model_quota_groups attribute and the background job logic for updating the quota baseline are solid and follow the intended design for credential-level quota tracking via a virtual _quota model.
The main suggestions below are focused on aligning the implementation more closely with the established patterns in the ProviderInterface. By using class attributes instead of direct method overrides where possible, we maintain consistency across providers and preserve built-in functionality like environment variable overrides.
Architectural Feedback
The approach of using a virtual _quota model to track the global credential-level quota is excellent. It allows the generic TUI logic to display usage stats without needing to know provider-specific details about how credits are consumed across different models.
Key Suggestions
- Consistency with
ProviderInterface: Useusage_reset_configsclass attribute instead of overridingget_usage_reset_configdirectly. - Remove Redundant Overrides:
get_models_in_quota_groupis already handled by the base class ifmodel_quota_groupsis defined. - Enable Env Overrides: Set
provider_env_nameto allow users to customize quota groups via environment variables.
Questions for the Author
None. The implementation logic for the background job and quota fetching looks robust.
This review was generated by an AI assistant.
- Add provider_env_name for env var overrides support - Replace get_usage_reset_config() with usage_reset_configs class attribute - Remove redundant get_models_in_quota_group() (uses base class implementation) - Remove unused get_quota_groups() method Addresses review feedback from mirrobot-agent.
|
All issues resolved. |
|
@mirrobot-agent review it again, all issues resolved |
|
@mirrobot-agent re-review |
|
@b3nw, I'm on it! I'll take another look at the changes and see how the Chutes provider quota system is shaping up. Back in a moment with the results. 🔍 |
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.
Overall Assessment
Excellent work. The Chutes provider implementation now fully aligns with the ProviderInterface patterns while maintaining the robust background job logic for quota tracking.
The move to declarative class attributes for quota groups and usage reset configurations is much cleaner and ensures that built-in functionality (like environment variable overrides) works as expected. The virtual _quota model approach remains a highlight of this PR, providing a elegant way to handle credential-level limits.
Architectural Feedback
None. The implementation is now idiomatic and consistent with the rest of the codebase.
Key Changes Verified
- Declarative Config: Replaced method overrides with
usage_reset_configsandmodel_quota_groupsattributes. - Env Variable Support: Enabled via
provider_env_name = "chutes". - Code Cleanup: Removed redundant overrides for
get_models_in_quota_groupandget_quota_groups.
This review was generated by an AI assistant.
| """ | ||
|
|
||
| # Enable environment variable overrides (e.g., QUOTA_GROUPS_CHUTES_GLOBAL) | ||
| provider_env_name = "chutes" |
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.
Fix confirmed: Added provider_env_name as suggested to enable environment variable overrides for quota groups.
| usage_reset_configs = { | ||
| "default": UsageResetConfigDef( | ||
| window_seconds=86400, # 24 hours (daily quota reset) | ||
| mode="per_model", | ||
| description="Chutes daily quota", | ||
| field_name="daily", | ||
| ) | ||
| } |
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.
Fix confirmed: Switched to usage_reset_configs class attribute and removed the redundant get_usage_reset_config override. This keeps the implementation declarative and consistent with the ProviderInterface patterns.
|
@b3nw, I've completed the re-review. SummaryThe changes look great! You've successfully addressed all the points from the previous review. Key Changes Verified
The implementation is solid and ready to go. Great work! This summary was generated by an AI assistant. |
|
@mirrobot-agent everything looks great LGTM go go go! |
|
@b3nw, Message received! "Go go go!" it is. I'm checking the PR status and CI results now. If everything is green, I'll proceed with the merge. 🚀 |
|
@b3nw, I tried to merge the PR, but it's currently blocked by repository rules. It's waiting on a required code owner review from Mirrowel. Since I'm operating as In the meantime, I'll keep an eye on it. Let me know if there's anything else I can assist with! 🔍 |
fix(chutes): enable quota display in TUI
Summary
Fixes the Chutes provider quota display in the TUI Quota Viewer. Previously, the quota status showed
-even though the provider was tracking usage correctly.Changes
model_quota_groupsclass attribute - Defines thechutes_globalquota group mapping to the_quotavirtual model, enablingclient.py'sget_quota_stats()to populate thequota_groupsresponseget_models_in_quota_group()- Returns the models belonging to a given quota groupget_quota_groups()- Exposes the available quota groups for the providerget_usage_reset_config()- Enablesper_modeltracking mode required for proper quota baseline storageResult
The TUI now correctly displays quota status for Chutes:
297/300with percentage barTesting
Important
Fixes TUI quota display for Chutes provider by adding methods for quota groups and usage reset configuration in
chutes_provider.py.297/300with a percentage bar.model_quota_groupsattribute inChutesProviderforchutes_globalmapping to_quota.get_models_in_quota_group()to return models in a quota group.get_quota_groups()to expose available quota groups.get_usage_reset_config()forper_modeltracking mode.This description was created by
for f904477. You can customize this summary. It will automatically update as commits are pushed.