Skip to content

fix(sdk): populate TierCounts in built-in store Stats implementations#481

Merged
peteski22 merged 1 commit into
mainfrom
fix/store-stats-tier-counts
Jun 26, 2026
Merged

fix(sdk): populate TierCounts in built-in store Stats implementations#481
peteski22 merged 1 commit into
mainfrom
fix/store-stats-tier-counts

Conversation

@peteski22

@peteski22 peteski22 commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

StoreStats.TierCounts (Go) / tier_counts (Python) was returned nil by the built-in SqliteStore and InMemoryStore in both SDKs. The Client compensated by setting {Local: TotalCount} after reading store stats, so callers going through the client were unaffected — but anyone using a Store directly got incomplete stats, and the built-ins diverged from the PostgreSQL adapter (added in #462) which already populates it.

This sets TierCounts to {Local: total} in all four built-in Stats methods, matching the PostgreSQL reference exactly (including {Local: 0} for an empty store).

Changes

  • sdk/go/store_sqlite.go, sdk/go/store_memory.go — populate TierCounts: {Local: total}
  • sdk/python/src/cq/store.py, sdk/python/src/cq/stores/memory.py — populate tier_counts={Tier.LOCAL: total}
  • sdk/go/storetest/storetest.go, sdk/python/tests/conformance.py — assert tier counts for populated and empty stores; these suites run against every store (sqlite, memory, postgres), so the contract is now enforced uniformly

Testing

  • Go: make -C sdk/go test and make -C sdk/go lint pass
  • Python: make -C sdk/python test (461 passed) and ruff lint/format pass
  • Test-first: added the conformance assertions, confirmed both suites failed (TierCounts = map[] / empty tier_counts), then implemented to green

Fixes #465

Summary by CodeRabbit

  • Bug Fixes

    • Store statistics now include tier breakdowns, so the local item count is shown consistently across in-memory and SQLite-backed stores.
    • Empty stores now report a zero count for the local tier instead of leaving the field unset.
  • Tests

    • Expanded stats checks to verify tier counts alongside existing totals and distribution values.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@peteski22, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 30 minutes and 16 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 credits.

🚦 How do rate 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 see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e867524b-b948-436a-be68-4897e21df885

📥 Commits

Reviewing files that changed from the base of the PR and between d07c71b and f9bd376.

📒 Files selected for processing (6)
  • sdk/go/store_memory.go
  • sdk/go/store_sqlite.go
  • sdk/go/storetest/storetest.go
  • sdk/python/src/cq/store.py
  • sdk/python/src/cq/stores/memory.py
  • sdk/python/tests/conformance.py

Walkthrough

Go and Python store stats now return tier counts for in-memory and SQLite stores, and the conformance suites assert the new field for empty and populated stores.

Changes

Store stats tier counts

Layer / File(s) Summary
Go stats implementations
sdk/go/store_memory.go, sdk/go/store_sqlite.go
Stats now returns TierCounts with Local set from the current stored unit count in both Go stores.
Go conformance checks
sdk/go/storetest/storetest.go
The Go conformance suite imports maps, compares TierCounts, and adds an empty-store assertion for cq.Local.
Python stats implementations
sdk/python/src/cq/stores/memory.py, sdk/python/src/cq/store.py
stats() now includes tier_counts with Tier.LOCAL set from the current stored unit count in both Python stores.
Python conformance checks
sdk/python/tests/conformance.py
The Python conformance suite imports Tier and asserts tier_counts for empty and populated stores.

Possibly related PRs

  • mozilla-ai/cq#398: Updates the StoreStats/tier_counts response path and remote decoding, which overlaps with the same stats field now being populated by local stores.
  • mozilla-ai/cq#446: Changes the StoreStats schema to require tier_counts, matching the field added in these store stats() implementations.

Suggested labels

sdk, sdk-go, sdk-python

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states the main change: populating TierCounts in built-in store Stats implementations.
Linked Issues check ✅ Passed The PR updates the specified Go and Python sqlite and memory stores to populate TierCounts, matching issue #465.
Out of Scope Changes check ✅ Passed The changes stay focused on TierCounts population and related conformance tests, with no obvious unrelated additions.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/store-stats-tier-counts

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.

The StoreStats TierCounts field was left nil by the built-in SqliteStore
and InMemoryStore in both SDKs. The Client compensated by setting
{Local: TotalCount} after the fact, but callers using a Store directly
got incomplete stats, and behavior diverged from the PostgreSQL adapter
which already populates it.

Populate TierCounts as {Local: total} in all four built-in Stats methods
(Go and Python, sqlite and memory), matching the PostgreSQL adapter. Add
the assertion to the shared store conformance suites so every store,
including postgres, is held to it going forward.

Fixes #465

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a stats contract inconsistency across built-in Store implementations in both Go and Python SDKs by ensuring tier breakdowns are always populated (matching the PostgreSQL adapter behavior), and enforces this via shared conformance suites.

Changes:

  • Populate TierCounts / tier_counts in built-in SQLite and in-memory stores as {Local: total} (including {Local: 0} for empty stores).
  • Extend Go and Python store conformance suites to assert tier counts for both empty and populated stores.
  • Align behavior across built-ins and the PostgreSQL adapter so direct Store consumers get complete stats without relying on Client-side compensation.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
sdk/go/store_sqlite.go Populate StoreStats.TierCounts with {Local: totalCount} in SQLite store stats.
sdk/go/store_memory.go Populate StoreStats.TierCounts with {Local: len(...)} in in-memory store stats.
sdk/go/storetest/storetest.go Add conformance assertions for tier counts (populated + empty store).
sdk/python/src/cq/store.py Populate StoreStats.tier_counts with {Tier.LOCAL: total} in SQLite store stats.
sdk/python/src/cq/stores/memory.py Populate StoreStats.tier_counts with {Tier.LOCAL: len(units)} in in-memory store stats.
sdk/python/tests/conformance.py Add conformance assertions for tier counts (populated + empty store).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@peteski22 peteski22 merged commit 76b3912 into main Jun 26, 2026
14 checks passed
@peteski22 peteski22 deleted the fix/store-stats-tier-counts branch June 26, 2026 17:04
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.

Store.Stats should populate TierCounts in all store implementations

2 participants