compact: attach summarizer cost accounting to /v1/compact responses - #81
Conversation
The seal is a real billable LLM leg but the handler dropped its cost on the floor: a metering proxy in front recorded a $0 row for every compaction. Implement the additive wire contract: every 2xx /v1/compact response that follows host.execute_async — the sealed success AND the compacted:false seal-failure — now also carries - "usage": OpenAI-shape token counts of the summarizer call, and - "x_router": the same block _build_x_router already emits on chat responses (cost_usd via _executed_cost_usd, cost_basis, provider, ...). Both keys are additive and optional; "messages"/"compacted" stay byte-identical (SZC's compact_splice reads only "messages"). Early returns that precede execution (nothing worth sealing) made no call and carry neither key. No session fold (a CompactRequest names no session) and no new logging/persistence of the summary. The usage block builder existed twice (unary chat body, streaming final chunk) — extracted as _openai_usage and shared by all three surfaces. Tests: seal success carries the summarizer leg's tokens + reported cost; compacted:false after execution carries them too; pre-execution early return does not; the splice is byte-identical around the new keys; and the accounting is invariant under conversation size (it is the seal leg's, not the conversation's).
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesCompact accounting
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant CompactEndpoint
participant HostExecuteAsync
participant CostedResponse
Client->>CompactEndpoint: submit messages to /v1/compact
CompactEndpoint->>HostExecuteAsync: execute sealing call
HostExecuteAsync-->>CostedResponse: return summarizer response
CostedResponse-->>Client: return compacted response with usage and x_router
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shim.py (1)
914-914: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer list unpacking over concatenation.
Using list unpacking is generally more idiomatic and slightly more performant in Python than using list concatenation with
+.
shim.py#L914-L914: replacefrozen + [sealed] + recentwith[*frozen, sealed, *recent]tests/test_compact.py#L130-L130: replace[sys0] + convowith[sys0, *convo]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shim.py` at line 914, Replace list concatenation with list unpacking in the compact-message construction around _costed in shim.py (anchor site shim.py:914-914), using frozen, sealed, and recent in the same order. Apply the same style change in tests/test_compact.py:130-130, replacing the [sys0] plus convo construction with equivalent unpacking.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shim.py`:
- Around line 1557-1558: Update the tokens_cached condition in the response
usage handling to check for None explicitly, so a value of 0 still populates
usage["prompt_tokens_details"]; keep the existing behavior for missing values
and align it with the adjacent token-field checks.
---
Nitpick comments:
In `@shim.py`:
- Line 914: Replace list concatenation with list unpacking in the
compact-message construction around _costed in shim.py (anchor site
shim.py:914-914), using frozen, sealed, and recent in the same order. Apply the
same style change in tests/test_compact.py:130-130, replacing the [sys0] plus
convo construction with equivalent unpacking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3ffb9ea0-2d1c-4b1f-bd15-3e01d1d5f65b
📒 Files selected for processing (3)
shim.pytests/test_antseed_node.pytests/test_compact.py
CodeRabbit (PR #81): the truthiness guard dropped an explicit tokens_cached: 0 (caching evaluated, no hits) — inherited byte-exact from the two pre-refactor copies, but inconsistent with x_router, which passes the 0 through verbatim. Now `is not None`, so both surfaces agree; absent stays absent. Pinned by a direct unit test.
1 participant
Why
POST /v1/compactperforms a real, billable LLM summarization viahost.execute_async— but the handler returned only{messages, compacted}, discarding the cost the router itself had already computed. Every seal was invisible to metering proxies downstream: paid to the provider, recorded as $0 (same failure class as the rate-card margin incident).What
usage(OpenAI-shape token counts of the summarizer call) andx_router(the same block chat responses emit, built by the existing_build_x_router), on every response that follows the billable call — including thecompacted:falsepartial-failure return. Pre-execution early-returns carry neither key.compact_splicereads onlymessages) are unaffected — pinned byte-exact bytest_compact_splice_untouched_by_accounting._openai_usageextracted as a shared helper (was duplicated in the chunk and response builders).Tests
tests/test_compact.py: 11 tests — success carries the seal leg's cost (cost_basis == "reported"),compacted:falsestill costed, early-returns clean, tokenless provider omitsusagebut keepsx_router, splice bytes untouched. Full suite: 498 passed, 2 skipped (skips = Ollama env-gated).Rollout
Compatible in any deploy order: the metering proxy reads these keys from 0.2.18 on; older proxies ignore them.
Summary by CodeRabbit
New Features
Tests