diff --git a/nextcloud_mcp_server/vector/processor.py b/nextcloud_mcp_server/vector/processor.py index 5e92037c..8b7d6851 100644 --- a/nextcloud_mcp_server/vector/processor.py +++ b/nextcloud_mcp_server/vector/processor.py @@ -122,23 +122,30 @@ async def record_indexing_usage( chunk_count: int, token_count: int, total_chars: int, + page_count: int | None, ) -> None: - """Record the two billable usage events for one embedded document. + """Record the billable usage events for one embedded document. - ``pages_embedded`` is the buyer-facing "pages indexed" dimension; - ``tokens_embedded`` is the embedding request's token count — the same metric - search records, so the meter bills embedding tokens whether they were - incurred indexing a document or embedding a query (Deck #67). + Two metered dimensions (Deck #67), recorded independently: - TODO(#282): ``pages_embedded`` currently carries the raw chunk count - (``len(chunk_texts)``) as an interim value. The real normalized "pages - indexed" count — real pages for paginated types (PDF/DOCX/PPT), a fixed - chars/tokens-per-page constant otherwise — is deferred to instrumentation - card #282; this code (card #284) only lands the metric name/contract. + - ``tokens_embedded`` — the embedding request's token count, recorded for + *every* embedded document. The same metric search records, so the meter + bills embedding tokens whether they were incurred indexing a document or + embedding a query. + - ``pages_embedded`` — a charge for **parsing** (PDF page extraction / OCR), + not a normalized content size. ``page_count`` is the real number of pages + the document processor parsed. Text content (notes, deck cards, news + items) is never parsed, carries no ``page_count``, and accrues **no** + ``pages_embedded`` row — only ``tokens_embedded``. There is deliberately + no chars/tokens-per-page constant: pages map 1:1 to parsed document pages + (card #282). Best-effort and flag-gated: a metering failure is logged and never breaks indexing. No-op when metering is disabled or the document produced no chunks (an empty batch embeds nothing and would only write zero-value rows). + ``pages_embedded`` is additionally skipped when ``page_count`` is absent or + zero — gating on the page count itself (not the ``doc_type``) keeps this + correct if a future non-PDF parsed type starts reporting pages. Privacy note: ``user_id`` stays tenant-local — the CP rollup aggregates GROUP BY (day, metric) into ``usage_daily`` (no metadata column), so nothing @@ -159,24 +166,27 @@ async def record_indexing_usage( store = await UsageEventStore.shared() # enabled=True: the guard above already confirmed the flag, so the store # skips a second uncached Settings build per record (ADR-024). - # record_usage_event swallows its own write failures, so the two records - # are independent; if pages_embedded somehow raised mid-way, - # tokens_embedded would be skipped, leaving an unmatched pages_embedded - # row — acceptable under the (day, metric) SUM-aggregation billing model. - await store.record_usage_event( - # TODO(#282): value is the interim chunk count; switch to normalized - # real-page count when the per-page constant lands. - metric="pages_embedded", - value=chunk_count, - metadata=metadata, - enabled=True, - ) + # record_usage_event swallows its own write failures, so the records are + # independent; one raising never blocks the other — acceptable under the + # (day, metric) SUM-aggregation billing model. + + # tokens_embedded: recorded for every embedded document. await store.record_usage_event( metric="tokens_embedded", value=token_count, metadata=metadata, enabled=True, ) + # pages_embedded: parsed pages only. Text content has no page_count, so + # skip it rather than write a zero-value row that would misrepresent a + # no-parse document as billable parsing work. + if page_count: + await store.record_usage_event( + metric="pages_embedded", + value=page_count, + metadata=metadata, + enabled=True, + ) except Exception: # Reached only when shared()/store construction itself raises # (record_usage_event swallows its own write failures). Metering is on, @@ -912,11 +922,18 @@ async def _index_document( # Export token consumption to Prometheus (always-on, independent of # the billing flag) so Grafana sees indexing token cost. record_embedding_tokens(provider, "index", embed_tokens) - # Usage metering (Deck #67): record the chunk volume + - # embedding-token count for this document. Best-effort and - # flag-gated; placed after the embedding succeeds so it can never - # affect the indexing path. See record_indexing_usage for the + # Usage metering (Deck #67): record the embedding-token count (all + # docs) and, for parsed files, the real parsed-page count. Best- + # effort and flag-gated; placed after the embedding succeeds so it + # can never affect the indexing path. ``page_count`` is set by the + # document processors for PDFs and absent for text types, so text + # content meters tokens only. See record_indexing_usage for the # metric/privacy details. + # + # Narrow defensively: file_metadata values are loosely typed, so a + # malformed page_count meters as "no pages" rather than erroring on + # the indexing path. + raw_page_count = file_metadata.get("page_count") await record_indexing_usage( enabled=settings.usage_metering_enabled, provider=provider, @@ -926,6 +943,9 @@ async def _index_document( chunk_count=len(chunk_texts), token_count=embed_tokens, total_chars=total_chars, + page_count=( + raw_page_count if isinstance(raw_page_count, int) else None + ), ) async def generate_sparse_embeddings(): diff --git a/tests/unit/test_processor_metering.py b/tests/unit/test_processor_metering.py index 26838c29..a5449e4b 100644 --- a/tests/unit/test_processor_metering.py +++ b/tests/unit/test_processor_metering.py @@ -1,9 +1,12 @@ """Unit tests for the indexing-path usage-metering helper (Deck #67). -``record_indexing_usage`` records the two billable events (``pages_embedded`` + -``tokens_embedded``) after a document's chunks are embedded. These cover the -value mapping, the flag/zero-chunk no-ops, and the best-effort failure path -without standing up the full document pipeline. +``record_indexing_usage`` records the billable events after a document's chunks +are embedded: ``tokens_embedded`` for every document, and ``pages_embedded`` +only for parsed files (real ``page_count``). Text content (no ``page_count``) +meters tokens only — ``pages_embedded`` is a charge for parsing, not content +size (card #282). These cover the value mapping, the flag/zero-chunk no-ops, the +text-only path, and the best-effort failure path without standing up the full +document pipeline. """ from unittest.mock import AsyncMock, MagicMock @@ -25,8 +28,8 @@ def store_spy(monkeypatch): @pytest.mark.unit -async def test_records_pages_embedded_and_token_count(store_spy): - """Both events fire: pages_embedded = chunk count, tokens_embedded = tokens.""" +async def test_parsed_file_records_pages_and_tokens(store_spy): + """A parsed PDF fires both events: pages_embedded = real page count.""" await processor.record_indexing_usage( enabled=True, provider="mistral", @@ -36,11 +39,13 @@ async def test_records_pages_embedded_and_token_count(store_spy): chunk_count=110, token_count=4242, total_chars=170826, + page_count=12, ) calls = store_spy.record_usage_event.await_args_list by_metric = {c.kwargs["metric"]: c.kwargs["value"] for c in calls} - assert by_metric == {"pages_embedded": 110, "tokens_embedded": 4242} + # pages_embedded is the real parsed-page count, NOT the chunk count. + assert by_metric == {"pages_embedded": 12, "tokens_embedded": 4242} for c in calls: # Hot-path fast-gate + tenant-local attribution metadata. assert c.kwargs["enabled"] is True @@ -50,6 +55,47 @@ async def test_records_pages_embedded_and_token_count(store_spy): assert c.kwargs["metadata"]["doc_type"] == "file" +@pytest.mark.unit +async def test_text_doc_records_tokens_only(store_spy): + """Unparsed text content (no page_count) meters tokens, never pages.""" + await processor.record_indexing_usage( + enabled=True, + provider="mistral", + model="mistral-embed", + doc_type="note", + user_id="alice", + chunk_count=4, + token_count=512, + total_chars=7000, + page_count=None, + ) + + calls = store_spy.record_usage_event.await_args_list + by_metric = {c.kwargs["metric"]: c.kwargs["value"] for c in calls} + assert by_metric == {"tokens_embedded": 512} + assert "pages_embedded" not in by_metric + + +@pytest.mark.unit +async def test_zero_pages_skips_pages(store_spy): + """page_count=0 (e.g. an empty/corrupt PDF) records tokens but no pages.""" + await processor.record_indexing_usage( + enabled=True, + provider="mistral", + model="mistral-embed", + doc_type="file", + user_id="alice", + chunk_count=4, + token_count=99, + total_chars=1000, + page_count=0, + ) + + calls = store_spy.record_usage_event.await_args_list + by_metric = {c.kwargs["metric"]: c.kwargs["value"] for c in calls} + assert by_metric == {"tokens_embedded": 99} + + @pytest.mark.unit async def test_disabled_is_noop(store_spy): """Flag off → no store access, no events.""" @@ -62,6 +108,7 @@ async def test_disabled_is_noop(store_spy): chunk_count=10, token_count=20, total_chars=5, + page_count=3, ) store_spy.record_usage_event.assert_not_awaited() @@ -78,6 +125,7 @@ async def test_zero_chunks_is_noop(store_spy): chunk_count=0, token_count=0, total_chars=0, + page_count=3, ) store_spy.record_usage_event.assert_not_awaited() @@ -101,4 +149,5 @@ async def test_store_failure_is_swallowed(monkeypatch): chunk_count=3, token_count=7, total_chars=9, + page_count=2, )