Repository navigation
feat(proxy): add native ROI calculator for gateway spend vs merged PRs - #43669
Conversation
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
|
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
|
bugbot run |
|
@veria-ai |
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
|
bugbot run |
|
@veria-ai |
|
bugbot run |
|
@veria-ai |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit deb9d2d. Configure here.
|
bugbot run |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
@veria-ai |
|
bugbot run |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
…route sweep GET /roi-calculator/repositories (#43669) lists repositories from the configured GitHub API, api.github.com by default, so the S2 sweep's GET of every route made the owned proxy reach an external host and failed the egress check in 31 integration-security tests. It joins /get/latest_release_info in the deny list
…fixtures (#43958) * test(ci): repair stale request fakes, spend-log golden, auto-router labels, and Interactions spec lookups Request fakes now carry the scope a real Starlette request has, the GCS pub/sub spend-log golden gains the agent identity keys from #43722, the auto-router session tests follow the baseline_models contract from #43348, and the Interactions spec checks resolve the create body and resource paths from the live spec instead of hardcoded names * test(ci): move retired OpenAI text-completion fixtures to live vehicles OpenAI still serves native /v1/completions on the gpt-5.4 family, so the single-prompt cases move to text-completion-openai/gpt-5.4-nano. Multi-prompt batches and echo with logprobs now 500 on every OpenAI model, so those cases keep the same text-completion-openai transport pointed at Fireworks, which documents both. The optional-params test asserts the request body actually sent instead of a success callback whose assertions were swallowed * test(ci): use a serverless Fireworks model for the text-completion batch and echo cases gpt-oss-20b is on-demand only on Fireworks, so the CI key got 404 model not deployed; glm-5p3-flash is listed as serverless * test(ci): skip the ROI calculator repository listing in the security route sweep GET /roi-calculator/repositories (#43669) lists repositories from the configured GitHub API, api.github.com by default, so the S2 sweep's GET of every route made the owned proxy reach an external host and failed the egress check in 31 integration-security tests. It joins /get/latest_release_info in the deny list
TLDR
Problem this solves:
How it solves it:
Intentional product change: the calculator runs inside the gateway, using a GitHub token instead of the standalone GitHub App. Full gateway admins configure it; read-only admins view results. Team admins and regular users cannot access the calculator
Scope: ROI calculator only. Dependency manifests, lockfiles, Google API tests, and shared encryption behavior match main. Includes main through
50f5cc9bbbUser Flow
Before: the initial native implementation exposes all settings together and requires manual refreshes
/roi-calculator/as a gateway adminAfter: guided setup leads to the first report and scheduled updates
/roi-calculator/as a gateway adminParity with the standalone calculator
Intentional differences:
Screenshot walkthrough
Screenshots use an isolated local gateway, two real public PRs from
BerriAI/litellm-admin-agent, and paid estimator calls.engineer@example.comis a synthetic QA identity used only to demonstrate matchingSetup: GitHub, repositories, estimator, first backfill
Overview, pull requests and estimate reasoning
People: before matching, matching dialog, matched result
Settings, advanced options, restart confirmation and sample preview
Matching calculator icons in the sidebar and page heading
Screenshots / Proof of Fix
Shared setup: local gateway on port 4036, dashboard on port 3036, isolated PostgreSQL database, real public GitHub PRs,
roi-estimatorrouted to paidgpt-6-luna. Authentication below uses the local admin key through an environment variable, never a committed secretBefore (1afcc91, initial PR implementation)
/roi-calculator/with no saved configuration: all setup fields appear together, shown aboveAfter (5d642e6)
curl -X POST http://localhost:4036/roi-calculator/connections/test -H "Authorization: Bearer $ROI_ADMIN_KEY": HTTP 200curl -X POST http://localhost:4036/roi-calculator/sync -H "Authorization: Bearer $ROI_ADMIN_KEY", then GET the same URL:{"phase":"complete","total":2,"estimated":2,"reused":2,"needs_attention":0}BerriAI/roi-qa-missing-repositoryalongside the real public repository in the isolated QA configuration and start analysis: two existing PR estimates remain, the unavailable repository is named, and cost-per-hour is absentBerriAI/litellm-roi-calculatorrepository alongside the unavailable QA repository and analyze: an error explains that no report was published, and both previous estimates remain; restore the original repositories/roi-calculator/identity-mapwith{"github_login":"invalid.name","email":"engineer@example.com"}: HTTP 422, with settings unchangedAdditional regression checks: 77 ROI backend/database tests and 15 ROI UI tests passed using the unchanged dependencies from main. Real PostgreSQL coverage verifies scope-preserving cache cleanup, exclusive sync ownership, writer routing with a separate reader, the scheduled interval gate, expired-lease fencing, and UTC timestamp recovery. A repeated-startup regression also verifies the scheduler retains exactly one ROI job; it fails with the original duplicate-job error before the fix. Full
make checkpassesValidation on
5d642e6211: all required CI checks pass, Codecov patch coverage passes at 84.13% against 82.51%, Greptile is 5/5 with no outstanding findings, and Veria reports no security issues. All reported review findings have tested fixesOutage regressions cover unavailable repositories, empty partial results, estimator failures, profile lookup failures versus confirmed email removal, and recovery. Refreshed identities persist across restarts; unchanged identities skip database writes. Live PostgreSQL row versions confirm no cache writes during an unchanged rerun
Bugbot found no issues on
deb9d2d5e5; its final rerun is unavailable because the team spending limit was reached. The only subsequent change skips unchanged cache writes and is covered by regression tests and live database checksValidation limitation: Python CodeQL fails because
Security/CWE-117/LogInjection.qlexceeds its 2 GiB result-set limit on both this PR and main. This is an incomplete scan, not a reported security finding; scan configuration and protections are unchanged. Required checks pass on5d642e6211OSV baseline: a fresh scan of main at
264b09ac8dreproduces the same five findings as this PR, affecting the existing PyJWT, urllib3 and Next.js versions. All scanned lockfiles and scan configuration are identical to main; dependency upgrades remain outside this ROI-only changeUnrelated CI baseline: the unchanged Google API compliance tests fail against upstream schema changes. Those tests and dependencies remain identical to main
Pre-Submission checklist
Caveats
Medium
Low
Type
New feature, bug fixes and regression tests