Skip to content

LCORE-3910: report the usage of the summarization and fold calls - #2867

Open
max-svistunov wants to merge 1 commit into
lightspeed-core:mainfrom
max-svistunov:lcore-3910-1-summarizer-usage
Open

max-svistunov wants to merge 1 commit into
lightspeed-core:mainfrom
max-svistunov:lcore-3910-1-summarizer-usage

Conversation

@max-svistunov

@max-svistunov max-svistunov commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Description

LCORE-3910, the first of two PRs. Compaction summarizes older turns with an LLM call of its own (summarize_chunk()), and folds the stored summaries into one with another (recursively_resummarize()). The provider bills both calls. Both functions read the text of the response and nothing else, so the usage the provider reported was lost there, before any metric, quota or token count could include it.

Both functions now take an optional keyword-only count_call callback and call it with the model and the usage of the call, a TokenCounter with llm_calls=1. They do so as soon as the response arrived and before they look at it. A call that returned no text makes the function raise ValueError, and it is reported all the same, because the provider billed it. reported_usage() reads usage.input_tokens and usage.output_tokens from the response, the fields OpenAIResponseObject.usage has in ogx-client 1.2.5 (the version in uv.lock). A call the provider reported no usage for is reported as one call with zero tokens.

Nothing changes for a running service with this PR alone. Nothing passes a callback yet: the two callers in src/utils/conversation_compaction.py are not touched. Without a callback the functions do what they did, and reported_usage() is not called. The 35 tests the module had pass unchanged.

Why this is a PR of its own. #2797 held LCORE-3910 as one commit of 1.8k lines on top of #2796, and nobody has reviewed it. It is replaced by two PRs. This is the first; src/utils/compaction.py is the same file as in #2797. The second holds the rest as three commits:

  • Metrics: compaction passes a counter as count_call, and the calls are recorded in ls_llm_token_sent_total, ls_llm_token_received_total and ls_llm_calls_total under the endpoint of the request.
  • Quota: /v1/query, /v1/streaming_query and /v1/responses charge each call to the quota of the user.
  • Reported token counts and documentation: /v1/query and /v1/streaming_query include the calls in the input_tokens / output_tokens they report, and the user guide and the design document say who pays for the calls.

This PR does not close LCORE-3910. The second PR does.

Relation to the other open PRs. The branch starts from main and does not depend on #2796 (LCORE-3908). The two PRs have no file in common, so they can be reviewed side by side and merged in either order. The second PR needs both merged first: it is built on top of #2796, because it changes the same lines of the streaming and A2A call sites, and on this PR. #2167 (LCORE-1347) moves src/utils/compaction.py to src/lightspeed_stack/utils/ and rewrites the imports of both files of this PR, in the lines where it adds its own: expect a conflict there in whichever merges second.

Why not extract_token_usage(). utils.responses.extract_token_usage() already turns a usage object into a TokenCounter. It also records the LLM metrics under an endpoint path, and the summarizer does not know which endpoint it runs for. utils/compaction.py is kept pure on purpose: its docstring says it does not touch conversation state, the cache or locks. Metrics and quota stay out of it in the same way. It reports each call, and the caller decides what to do with the usage.

On the ticket text. The ticket lists as its first symptom that ConversationSummary.token_count is a tiktoken count of the summary text and not the usage the model reported. That field is left as it is, here and in the second PR. It is the size of the summary in the context window, and _maybe_persist_fold() adds these sizes up to decide when the summaries are folded. The usage of the call is another quantity, so it is reported next to the field and does not replace it. The two decisions the ticket asks for (quota, reported token counts) are not part of this PR; they come with parts 3 and 4.

Known limits.

  • A call that fails, or is cancelled before its response arrived, reports nothing, whatever the provider bills for it.
  • An exception raised by the callback is not caught. It propagates out of the function, and the summary of that call is not returned. That matters in the second PR, where the callback consumes quota.
  • summarize_chunk() now has six arguments, one more than pylint allows, and gets a too-many-arguments disable.
  • The user guide already says that the summarization tokens are counted against the quota of the user (docs/user_doc/conversation_compaction.md). That stays untrue until the second PR. No documentation changes here.
File Change
src/utils/compaction.py type CallCounter, reported_usage(); summarize_chunk() and recursively_resummarize() take count_call and report their call (+45 / -2)
tests/unit/utils/test_compaction.py class TestCallUsage, 7 cases (+98 / -1)

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library [pyproject.toml + uv.lock]
  • Bump-up dependent library [requirements.*.txt for Konflux]
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement
  • Benchmarks improvement

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

  • Assisted-by: Claude Opus 4.8
  • Generated by: Claude Opus 4.8

Related Tickets & Documents

  • Related Issue # LCORE-3910
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  1. Manual verification. No live run was made for this PR: nothing passes the callback yet, so a running service makes the same calls and gives the same responses as on main. The commit of LCORE-3910: count the summarization calls in quota, token counts and metrics #2797, with the same src/utils/compaction.py, was verified live on 2026-09-28; the run is in its description.

  2. Run the tests of the change:

uv run pytest tests/unit/utils/test_compaction.py -q   # 42 passed: the 35 of main and 7 new

The new cases are in TestCallUsage: reported_usage() with usage and without it (no usage object, counts that are None, no counts), summarize_chunk() reports its call, and each of the two functions reports a call that returned no text. The fold has the no-text case only, because the same line reports in both cases.

Check that they fail without the change in src/. The test module does not import:

ImportError: cannot import name 'reported_usage' from 'utils.compaction'

With the two count_call(model, reported_usage(response)) lines replaced by pass:

FAILED tests/unit/utils/test_compaction.py::TestCallUsage::test_summarize_chunk_reports_its_call
FAILED tests/unit/utils/test_compaction.py::TestCallUsage::test_summarize_chunk_reports_a_call_that_returned_no_text
FAILED tests/unit/utils/test_compaction.py::TestCallUsage::test_fold_reports_a_call_that_returned_no_text
3 failed, 39 passed in 1.49s

With the report moved behind the empty-text check, the no-text case of that function fails.

  1. Run the full suites:
uv run pytest tests/unit -q                                                          # 3714 passed, 1 skipped (main: 3707 passed, 1 skipped)
uv run pytest tests/integration --ignore=tests/integration/container_lifecycle -q   # 326 passed (main: 326 passed)
  1. Linters:
uv run make black ruff docstyle pylint pyright   # all pass

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: lightspeed-core/lightspeed-stack/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 58a69b7c-1620-425e-85db-b37c90f31bc3
📥 Commits

Reviewing files that changed from the base of the PR and between 5737342 and 8c4445a.

📒 Files selected for processing (2)
  • src/utils/compaction.py
  • tests/unit/utils/test_compaction.py
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Compaction summarizes older turns with an LLM call of its own, and
folds the summaries with another. The provider bills both, and their
usage was discarded: summarize_chunk() and recursively_resummarize()
read the text of the response and nothing else.

Both functions now take an optional count_call callback and hand it
the model and the usage the provider reported for the call. They do so
as soon as the response arrived and before they look at it, so a call
that returned no text is reported as well. reported_usage() reads the
usage from the response; a call without reported usage counts as a
call with zero tokens. Without a callback nothing changes.

This commit only reports. Nothing passes a callback yet: the metrics,
the quota and the token counts told to the client follow in commits of
their own. utils.responses.extract_token_usage() is not reused,
because it records the LLM metrics under an endpoint path, and the
summarizer does not know which endpoint it runs for.

Tests: tests/unit/utils/test_compaction.py covers reported_usage() with
and without usage, and that each of the two calls is reported, also
when it returned no text. The existing tests of the module pass
unchanged.
@max-svistunov
max-svistunov force-pushed the lcore-3910-1-summarizer-usage branch from 3078b97 to 8c4445a Compare October 7, 2026 22:50

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant