Skip to content

feat: get plan bulk with cache#10

Open
tomerqodo wants to merge 4 commits intocursor_full_base_feat_get_plan_bulk_with_cache_pr10from
cursor_full_head_feat_get_plan_bulk_with_cache_pr10
Open

feat: get plan bulk with cache#10
tomerqodo wants to merge 4 commits intocursor_full_base_feat_get_plan_bulk_with_cache_pr10from
cursor_full_head_feat_get_plan_bulk_with_cache_pr10

Conversation

@tomerqodo
Copy link
Copy Markdown

@tomerqodo tomerqodo commented Jan 25, 2026

Benchmark PR from agentic-review-benchmarks#10


Note

Adds a cache-aware bulk plan retrieval to reduce load on the billing API and strengthens validation.

  • New get_plan_bulk_with_cache uses Redis (mget + pipeline setex) with key prefix tenant_plan: and 10-min TTL; logs cache hit/miss, gracefully falls back to API on Redis errors
  • Helper _make_plan_cache_key and SubscriptionPlan.__str__ for clarity/debugging
  • Harden get_plan_bulk: per-tenant validation with error logging; skips invalid entries instead of failing the batch
  • Extensive test coverage: integration tests with testcontainers covering cache hit/miss, invalid/expired cache, Redis failures, and pipeline errors; unit test for invalid plan data handling

Written by Cursor Bugbot for commit 70fe873. Configure here.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

logger.exception(
"get_plan_bulk: failed to validate subscription plan for tenant(%s)", tenant_id
)
results[tenant_id] = None
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Invalid tenants added to results with None instead of skipped

High Severity

When validation fails for a tenant's plan data in get_plan_bulk, the code sets results[tenant_id] = None. However, the return type is dict[str, SubscriptionPlan] (not allowing None), and the corresponding test test_get_plan_bulk_with_invalid_tenant_plan_skipped expects invalid tenants to be excluded from the result (assert "tenant-invalid" not in result). The implementation adds them with None values, causing test failures and type contract violations. This also causes get_plan_bulk_with_cache to cache "null" strings for invalid tenants.

Fix in Cursor Fix in Web


def __str__(self) -> str:
"""Return a human-readable string representation for debugging."""
return f"Plan: {self['plan']}, Expiration: {self['expiration_date']}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

TypedDict __str__ method is dead code that won't execute

Low Severity

The __str__ method defined on SubscriptionPlan TypedDict will never be called. TypedDict is a typing construct where instances are plain dictionaries at runtime, not actual instances of the TypedDict class. Calling str() on a SubscriptionPlan value will use the default dict string representation, not the custom one defined here. The docstring indicates this was intended "for debugging" but it won't work as expected.

Fix in Cursor Fix in Web

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.

3 participants