Repository navigation
refactor: clean up fresh tech debt from 2026-09-30 - #43993
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@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✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
bugbot run |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
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 ea2d141. Configure here.
TLDR
Problem this solves:
How it solves it:
User Flow
Before: a proxy admin ingests agent traces and checks the ROI calculator, and everything answers
{}After: the same admin sees exactly the same answers, since only unused code and comments are gone
{}Findings (run of 2026-10-01, window 2026-09-30)
6b9766fa0cfeat(proxy): add native ROI calculator for gateway spend vs merged PRs (feat(proxy): add native ROI calculator for gateway spend vs merged PRs #43669).current_cache_contextinroi_calculator/pull_cache.pywas a one-line wrapper overestimator.cache_contextwith no caller inlitellm/,enterprise/, ortests/. Deleted along with the import it alone used6b9766fa0c(feat(proxy): add native ROI calculator for gateway spend vs merged PRs #43669).pull_cache_keyinroi_calculator/estimator.pyhad no caller anywhere; the sync path keys pulls withpull_cache.cache_keyinstead. Deleted, every helper it used still has other callers268eb4d6e6feat(tracing): add OTLP trace ingestion and reads (feat(tracing): port OTLP ingestion to current trace foundation #43915). Four ASCII section banners (# ---- decode,# ---- normalize,# ---- write,# ---- read) intracing/decode.pyandtracing/receiver.py. Deleted268eb4d6e6(feat(tracing): port OTLP ingestion to current trace foundation #43915). The# Agent tracing / ClickHousesection header inconstants.pyrestated the constant names below it. Deleted268eb4d6e6(feat(tracing): port OTLP ingestion to current trace foundation #43915), made stale by6fd9334751feat(lens): analyze agent activity with a separate worker (feat(lens): analyze agent activity with a separate worker #43889). The# agent tracing: OTLP ingest + reads (scoped to the caller's team in the handler)comment inLiteLLMRoutesnow sits above the unrelated/engineroutes. Deleted rather than moved: the team scoping it mentions is documented in thetracing_endpoints.pyhandler docstring and enforced there, so the route list does not need it. The list itself is unchangedDeliberately left alone
9dda4d895f(fix(cost_calculator): bill ultrafast prompts above 272k at the ultrafast long-context rates #43764) raised thereportUnknownArgumentTypeceiling inbasedpyright-code-budget.jsonfrom 44358 to 44802. That is real debt, but repo AGENTS.md forbids budget edits on PR branches and lowering it needs about 444 typing fixes, so it is reported instead# noqa: F401re-export ofDualCacheincaching/caching.py(fix(caching): write the response-cache SET to Redis at once instead of on the post-call batch #43973) is justified: theX as Xalias tripsPLC0414and adding__all__would change star-import behaviorgetattr(..., "callback_name", None)inutils.py(fix(proxy): register a UI-configured arize callback next to otel under OTel v2 #43906) andgetattr(..., "call_type", None)in the Gray Swan guardrail (fix(grayswan): send request conversation and tool calls to post-call monitor #43770) need a shared protocol or a runtime import to type, wider than a same-day cleanupMapping[str, Any]row and payload types inclickhouse_spend_logger.pyandtracing/store.py(feat(tracing): port OTLP ingestion to current trace foundation #43915, feat(tracing): store spend in ClickHouse automatically #43928) need Pydantic row models at the ClickHouse boundary, also too wide here# noqa: BLE001,# pyright: ignore[reportPrivateUsage], and# rebind-oksuppressions in the agents, MCP auth, Lens, and ROI calculator code each name their rule and carry a reasonScreenshots / Proof of Fix
Parity A/B, since this PR must not change behavior: the same 14 requests sent to a live proxy at the merge base and at the tip, each leg booted from its own worktree (
litellm.__file__asserted inside it) with--num_workers 2, its own Postgres database and its own ClickHouse database,general_settings.tracing.store: clickhouse, realopenai/gpt-4o-minicalls, and real GitHub. Both legs loggedAgent tracing enabled (store=clickhouse)on both workers before traffic. The trace payload istests/test_litellm/tracing/fixtures/langsmith_deep_agent_export.json, a real LangSmith OTLP export. The ROI leg connects GitHub, selectsBerriAI/litellm-docs, picksgpt-4o-minias estimator, starts a sync withPOST /roi-calculator/sync, polls it to completion, and reads the report, which drivesestimator.pyandpull_cache.pyend to end. Volatile fields (completion id and text, timestamps, durations) are masked and each body is cut at 400 charactersBefore (0980f75)
Every request answers as expected: chat 200, OTLP ingest 200
{}, the trace lists with 6 spans, ROI settings save, the sync returns 202 and finishesphase=completewithdone=8/8, estimated=8, needs_attention=0, error=null, the report returns 200, and/enginereturns{"engines":[],"workers":[],"tracing_enabled":true}Raw commands and masked output, base
After (ea2d141)
The same 14 requests give the same statuses and bodies: the sync finishes
phase=completewithdone=8/8, estimated=8, needs_attention=0, error=null, and both legs write the same 8roi_calculator_pull_<cache_key>rows to Postgres, sopull_cache.cache_keyis unchangedRaw commands and masked output, head
Both legs ran on freshly created Postgres and ClickHouse databases. Diffing the two transcripts leaves only the leg header and the proxy port, with every status, trace id, span count, sync counter, report count, and
/enginebody identical. Both databases end with the same 8roi_calculator_pull_<cache_key>rows, so the survivingpull_cache.cache_keyproduced identical keys at base and tip. Comparing the full storedroi_calculator_reportvalues, the only differences aresynced_atand one PR estimated at 3.0 hours on base and 2.0 on head, which isgpt-4o-minioutput, not coderoi_calculator_pull rows, identical on both legs
Live PR risk
Ran /live-pr-risk and found no regressions/backward incompatible risks. The only removed symbols are
pull_cache_keyandcurrent_cache_context; a sweep oflitellm/,enterprise/,tests/,ui/,litellm-docs, and the sibling repos finds no reference, string or otherwise, and neither module defines__all__. Every other hunk is a comment. The live paths that reach the edited modules (OTLP ingest, trace list, trace and span reads, ROI settings, a full ROI sync, and the ROI report) were driven in the A/B above with matching resultsAudit
Ran /audit at base
0980f756bdand headea2d1410e5. Surface inventory: the four filesconstants.py,_types.py,tracing/decode.py, andtracing/receiver.pyparse to an identical AST at base and head (ast.dumpequality), so they have no runtime surface. Inestimator.pyandpull_cache.pyevery surviving top-level node is AST-identical and the only removed nodes arepull_cache_key,current_cache_context, and the import only the latter used, so the inventory is the ROI sync path that imports both modules plus any consumer of the removed names, of which there is nonetests/integration/database/test_roi_sync_store.py::test_roi_cache_survives_scope_changes_and_uses_writergpt-4o-miniHead runs collected and passed the same node id with seed 4106601, no skips or retries. No new integration test was added: no surviving line changed, and the cleanup run forbids speculative tests
Caveats (if any)
Low
proxy-behavioris red only because Codecov could not verify its CLI signature after 929 tests passed; the merge base0980f756bdfails the same step (job 110240402606) and it is not a required checkoauthwaits on thee2e-changedenvironment approval, which applies only to PRs touchingtests/e2e/; this PR does not and it is not a required checkpull_cache_keydrops the only key that covered diff evidence (files, commits, line counts)pull_cache.cache_keykeys on head_sha, title, body, and login, and is unchangedType
Refactoring
Pre-Submission checklist
tests/unit/proxy/roi_calculator,tests/unit/proxy/management_endpoints/test_roi_calculator_endpoints.py,tests/test_litellm/tracing,tests/test_litellm/proxy/test_tracing_endpoints.pyFinal Attestation
Link to Devin session: https://app.devin.ai/sessions/5de075df67d943c4aa29c8af73840e74
Open in Devin Desktop: https://app.devin.ai/desktop/session/5de075df67d943c4aa29c8af73840e74?variant=devin
Note
Low Risk
Comment-only edits plus deletion of symbols with no callers in the repo; OTLP ingest and ROI endpoints were A/B verified unchanged.
Overview
Removes 43 lines of unused code and comment noise from recent agent-tracing and ROI calculator work, with no intended behavior change.
ROI calculator: Deletes
pull_cache_keyinestimator.pyandcurrent_cache_contextinpull_cache.py(plus itscache_contextimport). Sync still keys pulls viapull_cache.cache_key;cache_contextremains in use on the sync path.Tracing / proxy: Strips section-banner comments in
tracing/decode.pyandtracing/receiver.py, the redundant ClickHouse header inconstants.py, and a stale OTLP route comment in_types.pythat no longer matched the/engineroutes below it.Reviewed by Cursor Bugbot for commit ea2d141. Bugbot is set up for automated code reviews on this repo. Configure here.