#450: Use one rate-limit counter source

[akmhatey-ai]

What changed

• Let Fbe::Middleware::RateLimit expose its tracked remaining count for core and search resources.
• Make off_quota?(resource: :search) read the search count from that middleware-owned source instead of reparsing last_response.body.
• Keep the existing fallback to core quota when GitHub's /rate_limit payload has no resources.search entry.
• Fold lower X-RateLimit-Remaining values from normal API responses into the middleware counter so the cached /rate_limit response stays conservative.

Why

off_quota? and Fbe::Middleware::RateLimit were tracking search quota in two places. If last_response.body no longer had resources.search, off_quota?(resource: :search) could fall back to the core count even though the middleware had already tracked the search count.

Validation

• bundle exec ruby -Itest test/fbe/test_octo.rb -n test_off_quota_search_uses_middleware_count_when_last_response_loses_search_resource - passed
• bundle exec ruby -Itest test/fbe/test_octo.rb -n test_print_trace - passed
• bundle exec ruby -Itest test/fbe/test_octo.rb -n test_pauses_when_quota_is_exceeded - passed
• bundle exec ruby -Itest test/fbe/middleware/test_rate_limit.rb - passed, 17 tests / 34 assertions
• bundle exec rubocop lib/fbe/middleware/rate_limit.rb lib/fbe/octo.rb test/fbe/test_octo.rb test/fbe/middleware/test_rate_limit.rb - passed, no offenses
• git diff --check - passed
• gitleaks detect --source . --redact --no-banner - passed, no leaks found

Limitation

bundle exec ruby -Itest test/fbe/test_octo.rb still has six Windows-local Errno::EACCES errors while deleting SQLite temp DB files under %LOCALAPPDATA%\Temp; the non-SQLite tests in that file pass.

[edmoffo]

The change consolidates the search-quota source in the rate-limit middleware and removes the last_response parsing in off_quota?, which is the fix the linked issue asked for. The new update step folds the lower x-ratelimit-remaining header into the cached count, which keeps the cached rate_limit response conservative between refreshes. The two added tests cover the search-quota fallback path and the header-driven decrement. CI is green.

[edmoffo]

Scope creep: this branch reverts unrelated upgrades on master rather than only addressing #450. The diff downgrades crate-ci/typos from v1.47.1 to v1.46.3 in .github/workflows/typos.yml, drops TargetRubyVersion from 3.3 back to 3.2 in .rubocop.yml, and removes several Elegant/* cop entries that exist on master. Gemfile.lock, fbe.gemspec, test/fbe/test_iterate.rb, test/fbe/test_overwrite.rb, and test/fbe/middleware/test_sqlite_store.rb are also touched without any link to the rate-limit work described in the title and body. Rebase onto current master and drop everything that does not belong to the #450 fix, then resubmit.

Separately on the core change: update at lib/fbe/middleware/rate_limit.rb:122 takes the min of the stored count and the value from x-ratelimit-remaining, so @remaining becomes monotonically non-increasing between /rate_limit polls. This is fine if every reset is observed by the next /rate_limit refresh (every 100 requests in handle_rate_limit_request), but please add a test that exercises the reset path so a future change does not turn the conservative-min into a permanent floor.

[edmoffo]

The change folds two divergent counter sources into one and the test pins the regression off_quota? could fall through to. CI is green on every checker and the rate-limit suite passes locally (17/17). The new constructor signature stays backward-compatible via the optional tracker arg, and the middleware retains its tracked search count when last_response loses the resources.search hash.

[edmoffo]

Read the diff against issue #450. The middleware now owns both @remaining and @searchleft, and off_quota? reads them through the new remaining(resource) accessor instead of reparsing last_response.body. The change matches the stated intent and removes the divergence the issue describes.

Local validation on the checked-out head:

• bundle exec ruby -Itest test/fbe/middleware/test_rate_limit.rb — 17 tests, 34 assertions, 0 failures.
• bundle exec ruby -Itest test/fbe/test_octo.rb -n test_off_quota_search_uses_middleware_count_when_last_response_loses_search_resource — 1 test, 1 assertion, 0 failures.
• bundle exec ruby -Itest test/fbe/test_octo.rb -n test_pauses_when_quota_is_exceeded — 1 test, 2 assertions, 0 failures.

All 10 exposed checks are green. No inline comments.

[bibonix]

Two notes inline: the new search test does not exercise the failure mode named in its method name, since the new off_quota? path never reads last_response; and update's min-fold on @Remaining keeps it pinned at the zero initial value until a /rate_limit request seeds it, mirroring the nil-guard used for @searchleft.

[bibonix]

If a non /rate_limit request lands before any /rate_limit call, @Remaining is still 0 from initialize and [0, count].min stays 0 forever, even though the response header reports the real remaining count. The :search path avoids this with a nil sentinel and the @searchleft.nil? ? count : [@searchleft, count].min idiom one line above; the :core path needs the same shape, either by initializing @Remaining to nil or by special-casing the zero case here.

[bibonix]

The name promises coverage of the case where last_response loses resources.search, but off_quota? in this PR no longer reads last_response at all. The monkey-patched rate_limit! / last_response on the client is dead weight; the assertion passes purely because the middleware tracked search=1 from the initial /rate_limit stub. Either rename the test to what it actually verifies (middleware count flows through to off_quota?) or drop the singleton method overrides.

[0crat]

@edmoffo Hey! Nice work on that review 👍 You snagged +13 points this time: started with the standard +18 base points, but lost -5 since we only saw 2 comments (our policy encourages more discussion to help everyone improve). Your running score is now +1549 - keep those reviews coming and don't forget to peek at your Zerocracy account for the latest updates!

[akmhatey-ai]

Addressed both review points. Latest head is e7b1862.

Changes:

• Fbe::Middleware::RateLimit now treats the core remaining count as unknown until a /rate_limit payload or response header is observed, so the first non-/rate_limit response can seed it instead of being pinned at zero.
• The search-resource regression now makes an actual search request first, decrementing the middleware counter from 5 to 4, then verifies off_quota?(resource: :search) still uses the middleware value even if last_response loses resources.search.

Validation:

• bundle exec ruby -Itest test/fbe/middleware/test_rate_limit.rb -> 17 tests, 33 assertions, 0 failures
• bundle exec ruby -Itest test/fbe/test_octo.rb -n test_off_quota_search_uses_middleware_count_when_last_response_loses_search_resource -> 1 test, 2 assertions, 0 failures
• bundle exec rubocop lib/fbe/middleware/rate_limit.rb lib/fbe/octo.rb test/fbe/middleware/test_rate_limit.rb test/fbe/test_octo.rb -> no offenses
• git diff --check origin/master...HEAD -> clean
• git merge-tree --write-tree origin/master HEAD -> 9ad35bdecbdb52f767f1d8abf3441dca16075da8
• diff-scoped gitleaks stdin --no-banner --redact -> no leaks
• GitHub Actions at e7b1862: actionlint, copyrights, markdown-lint, pdd, rake on macOS and Ubuntu, reuse, typos, xcop, and yamllint all passed

I also tried the full test/fbe/test_octo.rb file locally. The changed quota tests passed, but the full file still hits existing Windows temp SQLite cleanup errors (Errno::EACCES deleting %TEMP%/*.db) in unrelated sqlite-cache tests, so I am not claiming that full-file run as green here.

[GHX5T-SOL]

Reviewed current head e7b18625f4b66c702f3e89ffb78071093ebdd780 after the follow-up for the earlier review notes.

The branch is now scoped to #450: Fbe::Middleware::RateLimit owns the core/search counters, Fbe.octo.off_quota? reads through that middleware instead of reparsing last_response, and the non-/rate_limit response-header path seeds the counter when the cached /rate_limit payload has not done so yet. The new search regression now makes a real search call first, so the middleware-owned search count is what drives the later off_quota?(resource: :search) decision.

Validation:

• bundle check
• ruby -c lib/fbe/middleware/rate_limit.rb && ruby -c lib/fbe/octo.rb && ruby -c test/fbe/middleware/test_rate_limit.rb && ruby -c test/fbe/test_octo.rb
• git diff --check origin/master...HEAD
• bundle exec ruby -Itest test/fbe/middleware/test_rate_limit.rb -- --no-cov -> 17 tests, 33 assertions, 0 failures, 0 errors
• bundle exec ruby -Itest test/fbe/test_octo.rb -- --no-cov -> 64 tests, 182 assertions, 0 failures, 0 errors, 2 skips
• bundle exec rake test -> 285 tests, 805 assertions, 0 failures, 0 errors, 9 skips
• bundle exec rubocop -> 65 files inspected, no offenses

Hosted checks are green on this head, including both rake jobs.

[0crat]

@GHX5T-SOL Hey! Nice work on that review! 🎉 You snagged +13 points this time: started with the standard +18 base reward, but lost -5 for having only 2 comments (we need at least 9 to avoid the penalty). Your running total is now +1143 - keep those detailed reviews coming to maximize your bonus points! Don't forget to peek at your Zerocracy account too.

[yegor256]

@akmhatey-ai merge conflicts here

[yegor256]

Please fix the merge conflicts so this pull request can be merged.

[akmhatey-ai]

Merged current master and pushed head 72e2fd9 to clear the merge conflict reported above.

Notes:

• Kept upstream Faraday on_complete rate-limit sync.
• Preserved the PR's middleware-owned core/search remaining counters.
• Fixed the post-merge over? search quota threshold so search checks use threshold: 10.
• Updated the trace expectation to the middleware-decremented quota value after three real API calls.

Validation:

• bundle exec ruby -Itest test/fbe/test_over.rb -> 9 tests, 13 assertions, 0 failures.
• bundle exec ruby -Itest test/fbe/middleware/test_rate_limit.rb -> 19 tests, 36 assertions, 0 failures.
• bundle exec ruby -Itest test/fbe/test_octo.rb -n test_print_trace -> 1 test, 12 assertions, 0 failures.
• bundle exec ruby -Itest test/fbe/test_octo.rb -n test_off_quota_search_uses_middleware_count_when_last_response_loses_search_resource -> 1 test, 2 assertions, 0 failures.
• bundle exec rubocop lib/fbe/over.rb lib/fbe/middleware/rate_limit.rb lib/fbe/octo.rb test/fbe/test_over.rb test/fbe/middleware/test_rate_limit.rb test/fbe/test_octo.rb -> no offenses.
• git diff --check origin/master...HEAD -> clean.
• diff-scoped gitleaks stdin --no-banner --redact -> no leaks.
• GitHub Actions on 72e2fd9: all 10 checks pass, including rake on macOS and Ubuntu.

I did not claim the full local Windows rake run as green because the unrelated SQLite temp-file cleanup tests still hit Windows-specific Errno::EACCES; the hosted macOS/Ubuntu rake jobs are green on the pushed head.