feat: add granular llm cost limits - #4258
Conversation
There was a problem hiding this comment.
Thanks for the thorough implementation!
I think the A/B split from your plan is mostly right, but I don't think this is ready to merge yet because the new scoped limits do not participate in the existing reset schedule.
Blocking:
- user and virtual_key token-cost limits are now updated from interactions, but cleanup still only finds organization-scoped limits via
findLimitsNeedingCleanup(organizationId, cutoffTime). That means user/virtual-key limits will accumulate indefinitely and never reset on the configured organization cleanup interval. Since these new scopes are part of this PR, please update cleanup/reset behavior and add coverage for at least user and virtual_key limits resetting. - team/agent limits likely need the same treatment if the intended contract is that all token-cost limits follow the org cleanup interval.
- Agent limits being API/MCP-only and not manageable in the UI - do you have a proposed plan here? 🙂
Other notes / answers to some of your questions from the GitHub issue:
- The question about team-scoped limits applying to all teams attached to an agent versus only teams that include the requesting user looks like a real product/semantics question, but it is existing behavior rather than something this PR introduced. I would not block this PR on it once reset behavior is fixed.
- I agree that all-model limits do not need preinitialized zero-usage rows for every configured model. First-use initialization is simpler and handles future provider/model changes better.
- A uniqueness constraint for near-duplicate limits is worth discussing, but I would keep it out of this PR, so as to keep size of the PR manageable for review
|
Hi @joeyorlando, thanks for the prompt and detailed reply!
That's exactly what I thought. I felt a bit cautios about this gap though, given it's ~5 months old, decided to raise a question in my implementation plan and confirm before rushing to fix it. Will update the PR shortly.
I didn't find any strong reason not to allow editing agent limits from the common limits UI. I will simply add it to the UI :)
Thanks for answering those! I originally planned to file separate issues/PRs. If this was not a test task I would also implement and ship A and B parts of my original implementation plan in 2 separate PRs TBH :) |
|
Also, technically it is possible currently to create a usage limit for any agent type, including mcp_gateway and llm_proxy. We might want to come up with a sorted out semantics of what agent types do we allow to limit and how would it look like from the UX perspective. I would leave it out of scope of this PR though. |
right, so the |
…r-llm-cost-limits
…r-llm-cost-limits
c9218c8 to
8a25eb8
Compare
|
Implemented scheduled usage limits reset for limits scoped to any entity type. Triggered on interaction request or on list limits request. Had a chance to re-iterate on the implementation, re-implemented the check-and-reset routine in a constant number of db queries. Added Agents and LLM Proxy limits to the Costs and Limits UI page. Decided to keep these agent types separate in the limits UI to maintain consistency with the rest of the UX where these are separate as well. Tested thoroughly in local env. Good to go IMO. @joeyorlando FYI ^ |
| | `entity_id` | `string` | Yes | The ID of the entity (organization, team, or agent). | | ||
| | `entity_type` | `"organization" \| "team" \| "agent" \| "user" \| "virtual_key"` | Yes | The type of entity to apply the limit to. | | ||
| | `entity_id` | `string` | Yes | The ID of the entity (organization, team, agent, user, or virtual_key). | | ||
| | `limit_type` | `"token_cost" \| "mcp_server_calls" \| "tool_calls"` | Yes | The type of limit to apply. | |
There was a problem hiding this comment.
as a stretch-goal I would recommend removing mcp_server_calls and tool_calls from limit_type entirely - and not just for this MCP tool call arg, but entirely. I believe this was added quite some time ago, with the thought that we would soon add support for those concepts, but this is yet to be the case.
In fact, with that said, limit_type could likely be removed entirely (more args/descriptions leaves more room for models to hallucinate / make poor tool-call decisions when calling these tools)
(if we add MCP tool call limits those we'd likely rethink that implementation entirely)
There was a problem hiding this comment.
Great! Though this would be a breaking change on the db migration side. For the duration of the update in the customer cluster, old pods would throw on all (most?) queries to the limits table if such triggered by any requests.
So this looks like a 2-release job.
We could start with the stage 1 of deprecating this column: remove it from the sources entirely while keeping the column in the database in place, maybe making it a nullable with default token_cost. Then in some later PR aimed for a subsequent release do a stage 2 and finally cleanup the db column.
I still would be more comfortable creating a dedicated issue for that and tracking/commiting this work separately.
@joeyorlando WDYT? Let's create another issue for that or would you still insist on removing the limitType from the application side in the scope of this issue and PR?
There was a problem hiding this comment.
When creating/editing a limit, the dropdowns to select which entity it applies to are rather generic.
We already have some "more detailed/polished" dropdowns for things like users, models:
I would propose extracting those dropdowns out into their own reusable components (if they aren't already - to avoid duplication) and reusing those, and reusing similar other entity type's dropdowns (ex. agents, virtual keys, etc).
lastly, they're some tsc errors in the last CI job.
| @@ -25,9 +26,9 @@ Archestra stores both raw spend and savings. Savings can come from: | |||
|
|
|||
| ## Usage Limits | |||
There was a problem hiding this comment.
what happens if you were to have several limits defined:
- organization
- team
- virtual key
- user
And let's say you have that user, who belongs to that team, using that virtual key - how would the multiple limits apply? Would it be the case that if any of those limits are surpassed it would "short-circuit" the request? Do we have any test case(s) for this "multi limit case"? What would the end user see, would they be able to discern from the error message they'd get back exactly what limit caused the request to fail? What would show up in observability data?
There was a problem hiding this comment.
let's say you have that user, who belongs to that team
Just for clarity, earlier we agreed not to change the current semantics, which is team limits only apply to agent requrests when the agent is assigned to a team, not to a user request when the user is a member of a team
There was a problem hiding this comment.
Would it be the case that if any of those limits are surpassed it would "short-circuit" the request
Yes. Any exceeded limit applicable to a request will block the request
There was a problem hiding this comment.
Do we have any test case(s) for this "multi limit case"?
Yes, there is a thorough test coverage for various exceeded limits combinations scenarios under platform/backend/src/models/limit.test.ts
There was a problem hiding this comment.
let's say you have that user, who belongs to that team, using that virtual key - how would the multiple limits apply?
In terms of remaining usage quota checks for any given request, the limits are checked in order from the most granular to the least granular. Specifically:
- Virtual key limits
- User limits
- Agent limits (LLM Proxy falls here)
- Team limits
- Organization limits
What would the end user see, would they be able to discern from the error message they'd get back exactly what limit caused the request to fail?
Totally, whichever limit in order blocks first will define the error message that will float up to a client.
I attach screenshots of an example below with a few kinds of limits exhausted, and the request hits the most granular one
There was a problem hiding this comment.
What would show up in observability data?
API server logs affected limits and entities for blocked requests
@backend:dev: [19:25:04 UTC] INFO: [LimitValidation] Checking limit cadebafc-41ee-45f7-96b2-9e69df545bb5 for team 72869595-0589-4eea-a827-4e774271c629
@backend:dev: [19:25:04 UTC] INFO: [LimitValidation] LIMIT EXCEEDED for team 72869595-0589-4eea-a827-4e774271c629: 1.990595 cost_dollars >= 1
@backend:dev: [19:25:04 UTC] INFO: [LimitValidation] BLOCKED by team-level limit for team: 72869595-0589-4eea-a827-4e774271c629
There was a problem hiding this comment.
Interestingly. Just found a bug that is specific to request errors that come from the limits checks. If an interaction is rejected by a limit, the handler immediately returns, so the interaction is not recorded.
I believe it is out of scope of this task, though will fix it right away, because it is fairly trivial to fix.
There was a problem hiding this comment.
Turns out it's not that trivial because another bug was hidden behind this one - logs for errored interactions won't open on the Logs page. The scope of this PR already starting to get a bit blurry, I created a separate issue to dedicate it proper attention #4444
FYI @joeyorlando ^
56a0bf8 to
9f7efdb
Compare
…r-llm-cost-limits
9f7efdb to
a729db6
Compare
Came from bad merge. My bad. I've re-merged the upstream. Tested locally to the best of my current knowledge :) @joeyorlando Could you please approve the CI to re-test? |
|
Strange that CI check fails on the diff that came from main 🤔 Turns out turbo cache config was broken that's why I didn't catch these errors locally. I found remote turbo cache in our CI. That might explain how linting errors passed CI and got into main unnoticed in the first place. Pushed a fix to this branch. Should be fine now when the CI workflows are approved. |
There was a problem hiding this comment.
which files was this not capturing with these glob patterns? I think these patterns are too broad and will pick up node_modules / dist, no?
There was a problem hiding this comment.
which files was this not capturing with these glob patterns?
From the failing job output: file=tests/identity-providers.ee.spec.ts
inputs glob patterns are resolved relative to the root of each pnpm package within the pnpm workspace. E.g. patterns like e2e-tests/**/*.ts will not match any files in the e2e-tests pnpm package. So in fact no files in e2e-tests and shared packages could invalidate turbo cache with the old config.
There was a problem hiding this comment.
will pick up node_modules / dist
😅 Yes, I went to read turbo docs and was extremely surprised turbo does not filter out these by default. Well it sort of can filter out all the gitignored paths, but that requires adding "$TURBO_DEFAULT$ to the patterns.
Added.
62d3afd to
4ccb359
Compare
@joeyorlando Made them tables little bit more responsive
Agents table and the others built on top of this DataTable are still fine as well
|
|
🎉 @joeyorlando 🙏❤️ |
🤖 I have created a release *beep* *boop* --- ## [1.2.38](platform-v1.2.37...platform-v1.2.38) (2026-05-11) ### Features * add granular llm cost limits ([#4258](#4258)) ([4b6f50c](4b6f50c)) ### Bug Fixes * add require-approval to built-in config policy subagent ([#4521](#4521)) ([9942310](9942310)) * allow scroll on alternative-onboarding-dialog ([#4534](#4534)) ([0fc0674](0fc0674)) * change frontend visibility settings for roles ([#4456](#4456)) ([4c94643](4c94643)) * make pool max configurable via ARCHESTRA_DATABASE_POOL_MAX, defa… ([#4537](#4537)) ([85fb79f](85fb79f)) * **model-router:** hide models not linked to mapped API keys ([#4533](#4533)) ([6565c43](6565c43)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: archestra-ci[bot] <222894074+archestra-ci[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>







Closes #3603
/claim #3603
This PR addresses items A and B from the implementation plan.
Remaining considerations are left for further discussions with the team and for future PRs.