feat(plugin): Add proactive rate limiting to prevent 429 errors - #14029
feat(plugin): Add proactive rate limiting to prevent 429 errors#14029LVT382009 wants to merge 4 commits into
Conversation
- Add /ratelimit slash commands (status, clear, enable, disable, set) - Provides runtime controls for the cross-session rate limit guard - Core enforcement is always active via pre_llm_call and post_llm_call hooks - This plugin adds /ratelimit runtime controls without editing config.yaml - Supports enable/disable functionality with config persistence - Configurable default cooldown via /ratelimit set command - Config file stored at $HERMES_HOME/rate_limits/config.json - State file stored at $HERMES_HOME/rate_limits/nous.json - Comprehensive validation for all input types - Atomic writes for safe concurrent access
1 similar comment
- Updated pre_llm_call hook to inject context when rate-limited - Fixed hook signatures to match Hermes plugin system - Updated documentation to reflect correct behavior - pre_llm_call cannot block LLM calls, only inject context - Users will see warning message when rate-limited
- Add check_rate_limit_before_call() function that tracks request counts per minute - Modify run_agent.py to call rate limit check BEFORE making API calls - Plugin now actively blocks requests when approaching rate limits - Update README with new proactive rate limiting behavior - Update plugin.yaml to v2.0.0 with new description This fixes the issue where the plugin could only inject context but not actually prevent 429 errors. Now it proactively tracks request counts and waits when approaching the configured rate limit.
- Add agent/rate_limiter.py with FixedWindowRateLimiter and ProviderRateLimiterRegistry - Update plugins/rate-limiter/commands.py with RPM-based controls (status, enable, disable, set) - Update plugins/rate-limiter/__init__.py for plugin registration - Update plugins/rate-limiter/plugin.yaml with new metadata This provides client-side rate limiting that prevents 429 errors by pacing API calls to stay within configurable requests-per-minute limits.
|
ts PR so complex so tek dont want ts |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for pursuing proactive throttling. Current main already prevents retry amplification for confirmed Nous account limits (agent/conversation_loop.py:1100-1149, 3242-3308) and covers the auxiliary Nous path (agent/auxiliary_client.py:1966-1981), but it does not provide this PR's generic pacing feature.
Problems
run_agent.py:9086importsplugins.rate_limiter, but this PR addsplugins/rate-limiter/; the caughtImportErrorat lines 9109-9113 silently disables the proposed guard.plugins/rate-limiter/rate_limiter.py:303-341has an unlocked cross-process read/check/write sequence. Atomic rename does not make reservation atomic, so concurrent sessions can oversubscribe the configured RPM.plugins/rate-limiter/__init__.py:14-20registers only the slash command, not the claimed LLM hooks. The test also calls a nonexistentcommands.ratelimit_status()attests/plugins/test_rate_limiter_plugin.py:195.- The changed retry loop moved to
agent/conversation_loop.pyin053025238434cfbf121873977b39888d7f27d1c1.
Suggested changes
- Rework the active
conversation_loopseam, make slot reservation inter-process atomic, and add a concurrent-process regression test. - Use the current plugin hook contracts or remove the unused hook path; keep behavioral configuration in
config.yaml.
Automated hermes-sweeper review.
| # the API call. This prevents retry amplification and 429 errors. | ||
| # Works for all providers, not just Nous. | ||
| try: | ||
| from plugins.rate_limiter.rate_limiter import ( |
There was a problem hiding this comment.
This import cannot resolve the added plugins/rate-limiter/ directory as plugins.rate_limiter; the following except ImportError silently disables the general guard. Rework this through the plugin loader or a real core-owned module.
| # Get or create request tracker for this provider | ||
| tracker_key = f"{provider_key}:{model}" if model else provider_key | ||
|
|
||
| # Load existing tracker state |
There was a problem hiding this comment.
Atomic rename protects against a torn file but not this read/check/write reservation. Two processes can both read an under-limit tracker and both append, exceeding the claimed cross-session RPM limit; reserve the slot under an inter-process lock or transactional store.
|
|
||
| def register(ctx) -> None: | ||
| """Register the rate limiter plugin with the plugin system.""" | ||
| ctx.register_command( |
There was a problem hiding this comment.
Only the slash command is registered here. check_rate_limit and record_rate_limit_hook are never registered with ctx.register_hook, so the documented pre_llm_call and post_llm_call behavior cannot run.
| commands = _load_commands() | ||
|
|
||
| # Initially no rate limit | ||
| result = commands.ratelimit_status() |
There was a problem hiding this comment.
The changed commands module exposes handle() and _status(), not ratelimit_status(), so this test calls a nonexistent API. Exercise the registered slash-command handler instead.
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two PRs address generic proactive request pacing: #13307 adds a compact in-process provider-aware limiter around API calls, while #14029 expands the idea into two overlapping limiter implementations plus cross-session state, plugin controls, tests, and documentation, but does not connect that design correctly to the active request path.
Related pull requests
- #13307 [closed]
duplicate— (+374/-0) — keep closed: This remains relevant as the smaller prior implementation of the same provider-specific RPM pacing concept, but its integration targets the former run_agent.py request path and it has been superseded in scope by #14029 and subsequent main-branch retry-guard changes. - #14029
related— (+1370/-4) — close rather than merge: Despite the keep_open review on #14029, the diff imports plugins.rate_limiter from a hyphenated plugins/rate-limiter directory, leaves the claimed hooks unregistered, tests nonexistent command functions, modifies a retry loop that has moved to agent/conversation_loop.py, and performs cross-process request reservation through an unlocked read/check/write sequence; these are architectural integration failures, not a small salvage patch.
Duplicates
#13307 and #14029 substantially duplicate the same generic provider-aware proactive RPM limiter; #14029 additionally attempts cross-session 429 state and plugin controls, but those additions are not correctly wired or concurrency-safe.
Suggested consolidation
Merge neither — keep #13307 closed and close #14029 as the larger, stale duplicate. If generic pacing is still desired beyond the Nous-specific guards already on main, extract the focused limiter concept into a fresh PR against the active agent/conversation_loop.py path with an importable module layout, atomic cross-process reservation or explicitly session-local semantics, registered hooks, and tests that exercise the real command and request paths.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup13307 ["PRs duplicating each other"]
P13307["PR #13307 (closed)"]
P14029["PR #14029 (open)"]
end
class P13307 closed
class P14029 open
class P14029 target
click P13307 "https://github.com/NousResearch/hermes-agent/pull/13307"
click P14029 "https://github.com/NousResearch/hermes-agent/pull/14029"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed or no verify verdict yet (state tag in the node label).
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 65 kB of PR diffs, 16 kB of issue/PR text, 2 kB of discussion (4 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
Implements proactive rate limiting that tracks request counts per minute and blocks requests BEFORE they hit the API, preventing 429 errors and retry amplification. Provides runtime controls via slash commands to enable/disable the rate limiter and configure rate limits per provider.
Core Features
$HERMES_HOME/rate_limits/accessible by all Hermes sessionspre_llm_call(inject context when rate-limited),post_llm_call(record 429 errors)/ratelimitslash commands for status, clear, enable, disable, and set cooldownConfiguration
$HERMES_HOME/rate_limits/config.jsonstores:enabled: Boolean - whether rate limiter is active (default: true)default_cooldown: Float - default cooldown in seconds when 429 is hit (default: 300)limits: Object - per-provider rate limitsdefault.requests_per_minute: Default RPM for all providers{provider}.requests_per_minute: Provider-specific RPM (e.g.,nvidia.requests_per_minute: 2)$HERMES_HOME/rate_limits/nous.jsonstores rate limit state from 429 errors$HERMES_HOME/rate_limits/tracker_{provider}.jsonstores request timestamps for proactive limitingIntegration
run_agent.py: Modified to callcheck_rate_limit_before_call()before making API requestsplugins/rate-limiter/plugin.yaml: Plugin manifest with hooks and commands (v2.0.0)plugins/rate-limiter/rate_limiter.py: Core rate limiting logic with proactive blockingplugins/rate-limiter/commands.py: Slash command handlers (status, clear, enable, disable, set)plugins/rate-limiter/README.md: Plugin usage documentationWhat does this PR do?
This PR adds proactive rate limiting that prevents 429 errors by tracking request counts per minute and blocking requests BEFORE they hit the API. When a configured RPM limit is reached, the plugin automatically waits and retries, eliminating retry amplification where each 429 error triggers up to 9 additional API calls.
Problem Solved: Side clients (CLI, gateway, cron jobs) hit rate limits and retry aggressively, causing up to 9x API call amplification. This wastes quota and can lead to further rate limit violations. The previous implementation only recorded 429 errors after they occurred, but couldn't prevent them.
Why This Approach:
Key Changes from Original Implementation
Related Issue
Prevents retry amplification for side clients when rate limits are hit.
Type of Change
Changes Made
run_agent.py(modified): Addedcheck_rate_limit_before_call()call in retry loop before API requestsplugins/rate-limiter/rate_limiter.py(modified): Addedcheck_rate_limit_before_call()function for proactive blockingplugins/rate-limiter/plugin.yaml(modified): Updated to v2.0.0 with new descriptionplugins/rate-limiter/README.md(modified): Updated with proactive rate limiting behaviorplugins/rate-limiter/__init__.py(existing): Plugin registration with register() function and session hooksplugins/rate-limiter/commands.py(existing): Slash commands for /ratelimit (status, clear, enable, disable, set)How to Test
Enable Plugin:
hermes plugins enable rate-limiterConfigure Rate Limit:
Test Proactive Rate Limiting:
Test Cross-Session Tracking:
Test Clear Command:
/ratelimit clear /ratelimit status # Expected: No rate limit informationTest Enable/Disable:
Test Set Cooldown:
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AFor New Skills
N/A - This is a built-in plugin, not a skill.
Screenshots / Logs
Example Output
Rate Limit Status
Rate Limit Active