feat(focus): add ternary destination for FOCUS export - #39795
Conversation
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
Greptile SummaryAdds a Ternary callback and FOCUS destination that enriches spend records with token-count tags and uploads CSV data to Ternary.
Confidence Score: 4/5The implementation appears functionally safe, but the outstanding repository comment-policy requirement must be satisfied before merging. The empty-content regression test now correctly fails if an HTTP client is acquired, and the prior cadence findings are fixed or withdrawn. However, asproul claimed the comment-policy finding was fixed, while several routine explanatory comments remain in tests/test_litellm/integrations/ternary/test_ternary_logger.py, so that existing rule-linked finding remains unresolved. Files Needing Attention: tests/test_litellm/integrations/ternary/test_ternary_logger.py
|
| Filename | Overview |
|---|---|
| litellm/integrations/focus/destinations/ternary_destination.py | Implements validated, chunked FOCUS CSV delivery through LiteLLM's shared asynchronous HTTP client. |
| litellm/integrations/ternary/ternary_logger.py | Configures day-aligned Ternary exports and enriches FOCUS tags; the existing comment-policy finding remains only partly addressed. |
| tests/test_litellm/integrations/focus/test_ternary_destination.py | Restores explicit regression coverage proving empty content does not initialize an HTTP client. |
| tests/test_litellm/integrations/ternary/test_ternary_logger.py | Covers logger behavior comprehensively, but still contains comments covered by the outstanding repository-policy thread. |
Reviews (3): Last reviewed commit: "feat(focus): add ternary destination for..." | Re-trigger Greptile
| start_time: Final = (now_utc - timedelta(days=1)).replace(hour=0, minute=0, second=0, microsecond=0) | ||
| return FocusTimeWindow(start_time=start_time, end_time=now_utc, frequency=self.frequency) | ||
|
|
||
| # -- enrichment: inject token counts the shared transformer drops -------- |
There was a problem hiding this comment.
Comments Violate Repository Policy
The new files add section labels and explanatory comments beyond essential complex logic. Repository policy requires removing them before merge.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Fixed -- stripped the section labels and explanatory prose per CLAUDE.md, keeping only lint suppressions and concise docstrings
There was a problem hiding this comment.
Stripped the remaining section labels and trimmed the restating inline comments across both test files; kept only concise business-logic rationale and rule suppressions.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
4229140 to
ef1342f
Compare
Add a `ternary` FOCUS export destination (plus a `ternary` callback) so a LiteLLM proxy can push FOCUS-formatted spend to Ternary with `callbacks: ["ternary"]` -- no separate export job - FocusTernaryDestination: a pure sink modeled on the sibling vantage/mavvrik/gcs destinations; POSTs the FOCUS CSV to the Ternary cost-ingestion endpoint, chunking an oversized backfill (csv-module repartition, quoted-newline safe; oversized single row raises rather than silently dropping) with a stable X-Ternary-Upload-Id + 0-based index/total so the receiver stages-then-swaps. connection_id is validated + URL-encoded; base_url must be HTTPS (loopback exempt for local); the upload has a bounded timeout; it aborts on the first failed chunk so nothing partial lands - TernaryLogger: presets the FOCUS pipeline (CSV) and enriches the FOCUS Tags JSON with the per-request token counts the shared transformer drops (Ternary weights allocation by tokens); the enrichment is confined to this path and only adds keys. Cadence is daily (default) or interval (validation loops, an explicit interval-seconds required); the export window is day-aligned and drops days before the window start so a whole-day-replace receiver never truncates - Registration mirrors vantage/mavvrik: ternary callback literal, logging dispatch, scheduler job, and CustomLoggerRegistry entry -- no shared-code changes - Tests: mocked unit tests only (no live calls), 100% line coverage of both new modules
ef1342f to
a60a4d8
Compare
|
@greptileai please re-review — the two open findings (empty-content test and comment policy) are addressed in a60a4d8. |
Description
Adds a
ternaryFOCUS export destination (plus aternarycallback) so a LiteLLM proxy can push FOCUS-formatted spend to Ternary with justcallbacks: ["ternary"]— no separate export job.It follows the existing pure-sink destination pattern (Vantage / Mavvrik / GCS): it adds no FOCUS columns and forwards the FOCUS CSV as-is to a Ternary cost-ingestion endpoint, chunking an oversized backfill with a stable
X-Ternary-Upload-Id+ 0-based index/total so the receiver can stage-then-swap. A thinTernaryLoggerpresets the pipeline (CSV) and enriches the standard FOCUSTagsJSON with the per-request token counts the shared transformer drops (Ternary weights cost allocation by tokens). The enrichment is confined to the ternary code path and only adds keys toTags— the shared transformer is untouched, so other FOCUS exports are unaffected.Self-contained: no changes to existing code paths beyond the same registry/dispatch/scheduler wiring Vantage and Mavvrik already use.
New files:
litellm/integrations/focus/destinations/ternary_destination.pylitellm/integrations/ternary/ternary_logger.pyDocs land in a companion PR against
BerriAI/litellm-docs: BerriAI/litellm-docs#1197.Relevant issues
No linked issue — this mirrors the self-contained pattern of the already-merged FOCUS destinations #29935 (Mavvrik) and #29751 (GCS), which were likewise PR-only.
Type
🆕 New Feature
Testing
Mocked unit tests only — no live LLM/API calls (the HTTP handler is mocked):
tests/test_litellm/integrations/focus/test_ternary_destination.py— URL/auth/upload headers, single-shot vs chunked, csv-safe repartition (quoted newlines), oversized-row error, abort-on-first-chunk-failure,connection_idvalidation + URL-encoding.tests/test_litellm/integrations/ternary/test_ternary_logger.py— config presets, cadence validation, day-aligned window, and per-row token-Tags alignment through the realFocusTransformer.43 tests pass;
ruff check/ruff formatand the repo type-discipline gate are clean.Checklist
ruff+ type-discipline clean; formatting applied