This repository was archived by the owner on Aug 19, 2026. It is now read-only.
fix(chips): rate-limit the GitHub PR chip's GraphQL probe - #228
Merged
Conversation
The GithubPullRequest chip shells out to `gh pr view --json url`, a GitHub GraphQL call, on a 30s periodic refresh whose fingerprint is additionally invalidated by every `git`, `gh` and `gt` command. Nothing bounded how often it could actually run: the `Duration::from_secs(5)` in its policy is the shell command timeout, not a throttle. In a session that runs `git` constantly the fingerprint never matches, so the chip re-ran on essentially every prompt render. Measured 2026-08-06: ~7,600 GraphQL calls/hour against the 5,000/hour budget across 29 panes, exhausting the pool repeatedly and breaking unrelated `gh` usage for the rest of each hour. The REST pool sat at 4,999/5,000 unused throughout. Add `ChipRuntimePolicy::min_refresh_interval`, a floor on how often a chip's generator may START, and give the GitHub PR chip five minutes. Chips that do not declare one are unaffected. The floor throttles command VOLUME in one place, not context changes. The limiter compares a context fingerprint — the chip's inputs excluding `InvalidatingCommandCount` — so moving to another repo or branch still refreshes immediately rather than showing a stale PR for five minutes. A failed run returns its allowance, so one dropped connection does not blank the chip for the interval; persistent failures are still held off by `suppress_on_failure`. Both of those came from existing tests failing against a first, blunter version that floored everything unconditionally. `clear_cache` deliberately does not reset the rate-limit clock: it runs from `clear_chips_and_cache` on every new block, so clearing it there would hand out a fresh allowance after each command and leave the floor doing nothing. A test pins that, because the omission reads like an oversight in isolation. Five minutes is chosen against the measurement: a 30s floor merely matches the periodic rate and still costs ~3,480/hour at 29 panes. A PR URL for the current branch changes when a PR opens or closes, not while you work. Switching the probe to the idle REST pool is worth doing separately; it needs owner/repo/branch derivation that `gh pr view` does automatically, across three shell variants. Closes #227
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a per-chip execution floor to context chips so expensive generators (notably the GitHub PR chip’s gh pr view GraphQL probe) cannot start more frequently than a configured minimum, even when command-driven invalidation would otherwise force constant re-execution.
Changes:
- Introduces
ChipRuntimePolicy::min_refresh_intervaland a builder method to configure it. - Implements runtime enforcement in
CurrentPrompt, including a “context fingerprint” (excludingInvalidatingCommandCount) so repo/branch changes refresh immediately while repeatedgitactivity is throttled. - Adds tests to pin the GitHub PR chip floor and ensure unfloored chips remain unchanged; documents the
clear_cacheclock invariant.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| app/src/context_chips/mod.rs | Applies a 5-minute minimum refresh interval to the GitHub PR chip runtime policy. |
| app/src/context_chips/current_prompt.rs | Adds per-chip rate-limit state and enforcement using a context fingerprint and generator start stamping. |
| app/src/context_chips/current_prompt_tests.rs | Adds regression tests for the new floor behavior and clear_cache clock semantics. |
| app/src/context_chips/context_chip.rs | Extends ChipRuntimePolicy with min_refresh_interval and documents intended behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // A PR URL for the current branch is close to static; it changes when a PR is | ||
| // opened or closed, not while you work. Waiting up to five minutes to notice is | ||
| // the right trade against making `gh` unusable. | ||
| .with_min_refresh_interval(Duration::from_secs(300)); |
Comment on lines
+544
to
+556
| /// Give back the refresh allowance after a failed run. | ||
| /// | ||
| /// The floor is meant to bound how often a chip successfully spends a network quota, not to | ||
| /// punish it for a blip: a run that produced no value should not lock the chip out for the | ||
| /// whole interval, or one dropped connection leaves it blank for five minutes. Failures that | ||
| /// are NOT transient are already held off by `suppress_on_failure`, which refuses to re-run | ||
| /// while the failing fingerprint is unchanged — so releasing the clock here cannot turn a | ||
| /// persistent failure into a retry storm. | ||
| fn release_refresh_allowance_after_failure(&mut self, chip_kind: &ContextChipKind) { | ||
| if let Some(state) = self.states.get_mut(chip_kind) { | ||
| state.last_generator_started_at = None; | ||
| } | ||
| } |
| ); | ||
| } | ||
|
|
||
| /// A chip with no declared floor keeps its previous behaviour exactly. |
Comment on lines
+241
to
+242
| /// Shortest time that may pass between two executions of this chip's generator, whatever | ||
| /// asked for them. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #227.
The problem
The
GithubPullRequestchip runsgh pr view --json url— a GitHub GraphQLcall — on a 30s periodic refresh, with its fingerprint additionally invalidated
by every
git,ghandgtcommand. Nothing bounded how often it couldactually run. The
Duration::from_secs(5)in its policy is the shell commandtimeout, not a throttle.
In a session that runs
gitconstantly the fingerprint never matches, so thechip re-ran on essentially every prompt render. Measured 2026-08-06: ~7,600
GraphQL calls/hour against the 5,000/hour budget across 29 panes, exhausting
the pool repeatedly and breaking unrelated
ghusage for the rest of each hour.The REST pool sat at 4,999/5,000 unused throughout.
The fix
ChipRuntimePolicy::min_refresh_interval— a floor on how often a chip'sgenerator may start. The GitHub PR chip declares five minutes. Chips that do
not declare one behave exactly as before.
Two refinements, both of which came from existing tests failing against a
first, blunter version that floored everything unconditionally:
fingerprint — the chip's inputs excluding
InvalidatingCommandCount— somoving to another repo or branch refreshes immediately. Without this,
cdinginto a different repo showed the previous repo's PR for up to five minutes.
blanks the chip for the whole interval. Persistent failures are still held off
by the existing
suppress_on_failure, so this cannot become a retry storm.Why five minutes
Against the measurement, not by feel. A 30s floor merely matches the periodic
refresh and still costs ~120/hour/pane — about 3,480/hour at the observed pane
count, most of the budget for one chip. Five minutes puts it near ~350/hour at
that scale. A PR URL for the current branch changes when a PR opens or closes,
not while you work.
One non-obvious invariant
clear_cachedeliberately does not reset the rate-limit clock. It runs fromclear_chips_and_cache, which fires on every new block — after every command theuser runs. Clearing it there would hand every chip a fresh allowance per command
and leave the floor doing nothing at all. That reads like an oversight when you
look at
clear_cachealone, so there is a test pinning it.Verification
./script/presubmitgreen end to end (fmt, clippy, clang-format, wgslfmt,PSScriptAnalyzer, nextest, doc tests).
./script/check_ai_attributionand./script/check_rebrandpass.context_chips::current_prompt, including 3 new tests: the chipdeclares a floor meaningfully longer than the periodic interval, unthrottled
chips stay unthrottled, and
clear_cachepreserves the clock.Deliberately not in this PR
Switching the probe to the idle REST pool (issue #227, option 2). It needs
owner/repo/branch derivation that
gh pr viewdoes automatically, across the.sh,.ps1and.fishvariants — complementary to this change rather than analternative, and better reviewed separately.