Skip to content

api(chat-qa): the route spends the shared rate-limit budget when chat Q&A is off #9714

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

POST /v1/repos/:owner/:repo/pulls/:number/chat-qa (src/api/routes.ts:3568-3625) runs, in order: auth + PR lookup; if policy !== "off", counts prior invocations and records a new COMMAND_RATE_LIMIT_EVENT_TYPE audit row (:3591-3600); planNextWork (:3610-3615), a multi-query grounding-bundle build; then generateChatQaAnswer (:3616-3624).

The route comment at :3565-3567 states that the audit row is "the EXACT SAME counter the PR-comment @loopover chat command uses ... so a maintainer's dashboard questions and their own PR-comment usage on the same PR share one limit".

generateChatQaAnswer short-circuits at src/services/ai-chat-qa.ts:85-87: if (req.advisoryAiRouting?.chatQa !== true) return { status: "disabled", ... }. So on a repo where chat Q&A is not enabled, every request is guaranteed to return disabled — but only after it has consumed a slot in the shared budget and paid for the grounding bundle.

The predicate for that gate already exists next to the route's three other helpers: isRepoChatQaEnabled(settings) in src/api/maintainer-chat-qa.ts:4-8. routes.ts imports it at :186 and uses it only to populate the maintainer dashboard's capability map at :1853 — the route it was written alongside never consults it. The sibling helpers from the same module (resolveChatQaRateLimit, resolveChatQaActor, resolveChatQaGroundingLogin) are all used by the route.

Consequence: a UI or script hitting a chat-disabled repo drives the shared @loopover chat counter to its maxPerWindow (default 5 per 24h, src/api/maintainer-chat-qa.ts:17-18), after which the maintainer's genuine PR-comment @loopover chat usage on that PR is throttled — for questions that never produced an answer.

Requirements

  • The chat-qa route must call isRepoChatQaEnabled(settings) after resolving repository settings and, when it returns false, return the disabled result without recording a COMMAND_RATE_LIMIT_EVENT_TYPE audit row and without calling planNextWork.
  • The disabled response body and HTTP status must be byte-identical to what the route returns today for a disabled repo — i.e. the exact { status: "disabled", reason: ... } object generateChatQaAnswer produces, with the same 200 status. Reuse generateChatQaAnswer's own disabled result rather than hand-writing a second copy of the reason string.
  • No change to the enabled path: rate-limit counting, the audit row, planNextWork, and the rate_limited response at :3603-3607 all behave exactly as today.
  • No change to src/services/ai-chat-qa.ts.

⚠️ Required pattern: use isRepoChatQaEnabled from src/api/maintainer-chat-qa.ts — the predicate the maintainer dashboard already reads at src/api/routes.ts:1853 — so the dashboard's "chat available for this repo" flag and the route's own gate can never disagree. It does NOT satisfy this issue to inline settings.advisoryAiRouting?.chatQa === true at the route; to add a new predicate; to return a hand-authored {status:"disabled"} object with a different reason string; or to move the gate into generateChatQaAnswer (it is already there — the point is to stop paying for the call).

Deliverables

  • The chat-qa handler short-circuits on isRepoChatQaEnabled(settings) === false before the rate-limit block at :3587.
  • A new test asserts that for a repo with advisoryAiRouting.chatQa unset, a chat-qa request returns the { status: "disabled" } body with status 200 and that no COMMAND_RATE_LIMIT_EVENT_TYPE audit row was written (assert the audit_events row count for that targetKey is unchanged).
  • A new test asserts that same request does not invoke planNextWork (spy/mock the module and assert zero calls).
  • A regression test asserts that for a repo with advisoryAiRouting.chatQa === true and commandRateLimitPolicy: "hold", the audit row IS still recorded and the answer path still runs.
  • A regression test asserts the existing rate_limited response is unchanged for an enabled repo over its maxPerWindow.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example adding the short-circuit without the assertion that no audit row is written — does not resolve this issue.

Test Coverage Requirements

src/api/** is inside Codecov's src/** include; the 99% branch-counted patch gate applies. Both arms of the new isRepoChatQaEnabled condition must be covered, and the existing policy !== "off" and over-maxPerWindow branches must stay covered on the enabled path. The "no audit row written" assertion is the named regression test for this fix.

Expected Outcome

Asking a question on a repo where chat Q&A is disabled costs nothing: no shared rate-limit slot, no audit row, no grounding-bundle build. A maintainer's real @loopover chat budget on a PR can no longer be drained by requests that were never going to be answered.

Links & Resources

src/api/routes.ts:3559-3625, :186, :1853; src/api/maintainer-chat-qa.ts:4-20; src/services/ai-chat-qa.ts:85-87; test/unit/maintainer-chat-qa-helpers.test.ts.

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions