feat(usage): add chunks_stored retention meter

[cbcoutinho]

Why

We are moving the price basis off source bytes (a 31x-spread unit across the fleet) onto chunks. Auditing the ingest path against that model found the data plane can bill ingestion and OCR, but cannot bill retention at all:

• Emitted metrics are tokens_embedded, pages_embedded, pages_ocr, bytes_ingested, bytes_stored — there is no chunk counter on the billable path.
• bytes_stored is emitted per document per indexing pass, so it is an ingest flow. Summing it over a period counts a re-indexed document again on every pass — it measures churn, not what is retained.

Retention is the term that compounds: it is the only cost that keeps accruing after the ingest work is done.

What

Records the retained chunk count once per UTC day, split by index mode, as a chunks_stored usage event. A month of readings sums to chunk-days — the integral of a changing stock — so a corpus loaded or purged mid-month is charged pro rata, and the rollup's existing daily-sum semantics are unchanged.

The measurement already existed and was already mode-aware, so this is mostly wiring:

• Reuses count_indexed() and count_hybrid_chunks() unchanged (the latter already filters on the index_mode payload key).
• Keyword chunks are derived as total - hybrid — one fewer Qdrant round-trip, and the parts always sum to the total.
• Modelled on the existing vector_density_snapshot_task slow-cadence pattern; spawned at both consumer paths, since omitting the multi-user one would silently un-meter those tenants.

Idempotency is the load-bearing detail

Event ids are derived from (metric, mode, UTC date). usage_events inserts are already ON CONFLICT (event_id) DO NOTHING, so a pod that restarts five times in a day still contributes exactly one reading per mode. Without this, restarts would silently multiply the billed chunk-days. Verified against a real SQLite usage_events table as well as in unit tests: three identical runs produce 2 rows totalling 1000 chunk-days, not 3000.

Counts are exact rather than the gauges' exact=False shortcut — this runs once a day and the figure is billable.

Cross-repo sequencing (no merge-order requirement)

chunks_stored is a new metric name, so per migration 007 the control-plane rollup ignores it until the CP catalog and Stripe meter learn it. The mode split rides in the event metadata, and the rollup currently groups by (day, metric) only, so both modes bill as one line until the CP also groups on index_mode.

This lands correct and inert: it changes no existing metric, and is gated on USAGE_METERING_ENABLED. metadata is already JSONB on Postgres — explicitly "so the CP can slice on dimensions later" — so the CP side is a rollup query change, not a migration.

Test coverage

15 new unit tests covering the billing properties: both modes recorded, keyword derived (and never negative if an upsert lands between the two counts), exact counts, empty corpus records nothing, metering-disabled no-ops, Qdrant failure does not raise, the row is stamped at UTC midnight of the day it describes, event ids stable across restarts but distinct across days, plus the task loop's immediate-first-reading and its non-zero sleep floor at the day boundary.

Deviation from plan, flagged deliberately: the plan called for a Pact contract test. On inspection that is the wrong tool here — every existing test in tests/contract/ is an HTTP boundary, but the control plane consumes usage_events by reading the app DB directly. This is a schema contract, not a Pact-able HTTP one. The metric name and metadata shape are asserted in unit tests instead, and the CP-catalog sync requirement is documented at the metric definition. Happy to add a Pact test if you disagree.

Integration verification against a running stack (mixed-mode corpus, then confirming a re-index does not inflate the stock) is still outstanding.

Deck: board 8 #1001. Depends on #1000 for mode-differentiated billing.

---

This PR was generated with the help of AI, and reviewed by a Human

[claude]

Claude finished @cbcoutinho's task in 3m 5s —— View job

---

Review complete ✅

• Gather context (PR body, comments, changed files)
• Read diff against origin/master
• Review metrics_publisher.py changes (core logic)
• Review app.py / config.py wiring
• Review test coverage in test_metrics_publisher.py
• Review docs changes

This is a clean, well-scoped PR — three prior review rounds already tightened the empty-corpus fast path, the total→total_chunks rename, the docs catalog gap, and the zero-interval floor. I traced the remaining logic end-to-end (against nextcloud_mcp_server/vector/processor.py's existing record_indexing_usage, nextcloud_mcp_server/usage/store.py's ON CONFLICT idempotency contract, and the pre-existing vector_density_snapshot_task pattern it mirrors) and didn't find anything blocking.

Correctness

• record_storage_stock (nextcloud_mcp_server/vector/metrics_publisher.py:471) correctly unpacks count_indexed's (documents, chunks) tuple, reuses count_hybrid_chunks unchanged, and derives keyword as total_chunks - hybrid — verified both helpers are untouched and their filters (is_placeholder, chunk_index, index_mode) are exactly what's claimed.
• The idempotency story checks out: _stock_event_id (metrics_publisher.py:452) hashes (metric, mode, date) via uuid5, matching the existing pattern in placeholder.py/dead_letter.py, and UsageEventStore.record_usage_events inserts with ON CONFLICT (event_id) DO NOTHING (usage/store.py:53) — so restarts/replicas genuinely can't inflate billed chunk-days.
• _next_reading_delay (metrics_publisher.py:565) is correctly pure and the floor-after-cap ordering (max(1.0, min(interval, until_midnight))) handles both the zero-interval and midnight-boundary cases the tests assert.
• The metadata["index_mode"] key matches the convention already used by record_indexing_usage in vector/processor.py:524, so the CP rollup can slice both metrics the same way once its catalog learns the new metric.
• App wiring (app.py:2264, app.py:2508) correctly spawns usage_stock_task on both the single-user and multi-user consumer paths, each nested inside the existing vector_sync_enabled block — consistent with how vector_density_snapshot_task is already scoped, so no double-spawn risk.

Known, disclosed tradeoff (not a new finding) — the keyword = max(0, total_chunks - hybrid) clamp only prevents a negative keyword count; if a concurrent upsert lands between the two Qdrant round-trips such that hybrid alone exceeds the total_chunks read moments earlier, the day's reading can overbill by that race window. This is explicitly called out in the code comment and in round-3 of this PR's own review thread, bounded in magnitude, and self-corrects the next day — no action needed, just confirming it's understood rather than overlooked.

Test coverage — thorough: both modes recorded + keyword derivation, exact (not approximate) counts, empty-corpus short-circuit (asserts count_hybrid_chunks isn't even awaited), metering-disabled no-op, Qdrant-failure swallowed, UTC-midnight stamping, event-id stability across restarts and distinctness across days, single-mode tenants not writing a zero row, plus a dedicated pure-function suite for _next_reading_delay including a full 1440-minute sweep of the day. One very minor nit: the PR description says "10 new unit tests" but the diff actually adds 15 (9 in TestRecordStorageStock + 1 in TestUsageStockTask + 5 in TestNextReadingDelay) — almost certainly just a stale count from before round 1 replaced one test with four; purely cosmetic, not worth a churn commit.

Security / performance — no concerns. Counts are exact=True deliberately (billing figure, daily cadence, documented tradeoff vs. the gauges' exact=False), the empty-corpus guard now runs before the extra Qdrant round-trip, and the whole snapshot path is wrapped in the same never-raise contract as the sibling gauge publishers.

Process note: I wasn't able to execute ruff/ty/pytest myself in this sandbox (tool invocations for uv run ... required approval that wasn't available), so I relied on static reading plus the PR thread's own reported results (3387 unit tests passing, ruff/ty green, SonarCloud Quality Gate passed with 0 new issues) — those are consistent with what I read in the diff.

No blocking issues found. Nice attention to the idempotency and race-window details in the write-up.

[cbcoutinho]

Round-1 findings addressed in 6779e0b1:

• Stale function name in comment: correct, thanks — it's record_indexing_usage in vector/processor.py:445, no leading underscore. Fixed, and added the file reference so the cross-reference is unambiguous.
• test_sleep_is_never_zero patching the shared anyio module: fixed properly rather than narrowed. Extracted the delay computation out of the task loop into a pure _next_reading_delay(now, interval), so the boundary behaviour is now tested directly with no patching of the clock or the sleep primitive. Replaced the single test with four: midnight alignment, the 1s floor at 23:59:59.5, the interval cap, and a sweep over every minute of a day asserting no clock position yields a zero-length sleep.

You were right that the broad patch was only there because the logic was inline — extracting it removed the need entirely and tests the actual invariant better.

3386 unit tests pass; ruff check, ruff format --check and ty check green.

On the sandbox note: uv run ruff check && uv run ty check -- nextcloud_mcp_server && uv run pytest -m unit -n auto -q was run locally and is green; CI covers it too.

[cbcoutinho]

Round-2 findings addressed in 0d87e4be:

• total → total_chunks: renamed, plus a one-line comment at the call site noting the discarded element is the document count. Agreed — the binding shouldn't require reading count_indexed's docstring to interpret.
• Docs catalog gap: added a Usage metering (billing) section to docs/observability.md. Went slightly wider than the finding: that file listed only Prometheus series, and chunks_stored is a usage_events row in the app DB, so dropping it into the existing metric lists would have sent people looking for it in Grafana. The new section says that up front, documents the whole usage catalog (not just the new metric), and splits it into ingest flows (per document per pass — a period sum measures churn) versus the retention stock. It also records that adding a metric there is a cross-repo change, since the CP rollup silently ignores metrics its catalog doesn't know.
• Integration verification: agreed, and tracked on Deck chore(deps): update anthropics/claude-code-action action to v1.0.162 #1001 with explicit acceptance criteria — mixed-mode corpus, the two rows summing to nc_get_vector_sync_status's chunk count, re-index not increasing chunks_stored, and purge decreasing it. That third one is exactly the property mocked Qdrant can't prove and the reason the meter exists, so it shouldn't merge unverified indefinitely. Happy to land it here instead of as a fast-follow if you'd prefer — say the word.
• exact=True performance: no change, as you suggested. Noted on the Deck card to watch on the largest tenants; the daily cadence bounds the cost but it isn't free.

42 tests in the touched file pass (3386 unit total); ruff check and ty check green.

[cbcoutinho]

Round-3 findings addressed in d500be75:

• Wasted round-trip on an empty corpus: reordered — the total_chunks <= 0 guard now runs before count_hybrid_chunks, so a never-indexed tenant costs one Qdrant call instead of three. test_empty_corpus_records_nothing now asserts count_hybrid_chunks isn't awaited, so the saving can't silently regress.
• Clamp can overbill under a concurrent upsert: documented at the clamp, as you asked — including why it's preferred to the alternative. Getting an exact figure would mean holding a consistent read across two counts on a collection being written to continuously; the overstatement is bounded by the window between the two calls and re-read from scratch the next day. Deliberate, now explicit.
• No floor on usage_stock_snapshot_interval: fixed rather than deferred. You're right that it mirrors vector_density_snapshot_interval, but the foot-gun is in new code so I'd rather not propagate it. Moving the floor after the cap — max(1.0, min(interval, until_midnight)) — covers a zero interval and the midnight boundary with one guard, and reads more simply than the nested form. Added test_floors_a_zero_interval.

3387 unit tests pass; ruff check, ruff format --check, ty check green.

Remaining known gap is the live-stack verification tracked on Deck #1001 — specifically that a re-index must not increase chunks_stored, which is the one property mocked Qdrant can't establish and the whole reason this meter exists rather than reusing bytes_stored. Happy to land that here rather than as a fast-follow if you'd rather not merge it unverified.

[cbcoutinho]

Round-4 findings addressed in a48e5cdb:

• Zero-value rows for single-mode tenants: fixed — each mode is now guarded on count > 0, matching the if bytes_ingested > 0 pattern in record_indexing_usage. Good catch: a 100%-hybrid tenant would have accrued a permanent daily no-op keyword row, and since absence and an explicit zero are identical to a SUM, nothing is lost by skipping it. Added test_single_mode_tenant_writes_no_zero_row, and updated test_keyword_never_negative (the clamped 0 is now skipped rather than written).
• UUID namespace convention: switched to uuid.NAMESPACE_DNS with a distinguishing prefix, matching placeholder.py and dead_letter.py. You're right that there's no collision risk across the two id spaces — but a lone bespoke namespace constant is exactly the kind of thing that makes a future reader stop and wonder what's special about it, so consistency wins.
• Pact-test process question: agreed that's yours to weigh, not a code matter. My position for the record: tests/contract/ is entirely HTTP boundaries today, and the CP reads usage_events directly, so a Pact test would be modelling a boundary that doesn't exist. Happy to be overruled.

3388 unit tests pass; ruff/format/ty green.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud