feat: get plan bulk with cache

[hila-f-qodo]

Benchmark PR from qodo-benchmark#445

---

[devin-ai-integration]

Devin Review found 2 potential issues.

View issues and 2 additional flags in Devin Review.

[devin-ai-integration]

🔴 get_plan_bulk stores None for invalid tenants instead of skipping them

When validation fails for a tenant's subscription plan in get_plan_bulk, the code incorrectly stores None in the results dictionary instead of skipping the invalid entry.

Click to expand

Issue Details

At line 292, when validation fails:

results[tenant_id] = None

This contradicts:

1. The return type annotation dict[str, SubscriptionPlan] which doesn't include None
2. The test test_get_plan_bulk_with_invalid_tenant_plan_skipped which expects assert "tenant-invalid" not in result

Impact

• Callers expecting SubscriptionPlan values will receive None unexpectedly
• The downstream get_plan_bulk_with_cache function will cache None values (serialized as "null") to Redis, causing validation failures on subsequent cache reads
• Type safety is violated as the return type doesn't match the actual returned values

Expected Behavior

Invalid tenant plans should be skipped entirely (not added to results), matching the test expectation and type annotation.

Recommendation: Remove line 292 (results[tenant_id] = None) so that invalid tenant plans are simply not added to the results dictionary, matching the expected behavior from the test.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🔴 get_plan_bulk_with_cache caches None values from get_plan_bulk, causing cache pollution

When get_plan_bulk returns None values for invalid tenants (due to BUG-0001), get_plan_bulk_with_cache caches these None values to Redis, which will fail validation when retrieved later.

Click to expand

Issue Details

At lines 365-367:

for tenant_id, subscription_plan in bulk_plans.items():
    tenant_plans[tenant_id] = subscription_plan  # subscription_plan can be None
    plans_to_cache[tenant_id] = subscription_plan

And at lines 373-377:

for tenant_id, subscription_plan in bulk_plans.items():
    redis_key = cls._make_plan_cache_key(tenant_id)
    json_str = json.dumps(subscription_plan)  # json.dumps(None) = "null"
    pipe.setex(redis_key, cls._PLAN_CACHE_TTL, json_str)

Impact

1. None values are stored in tenant_plans and returned to callers
2. json.dumps(None) produces "null", which gets cached in Redis
3. When this cached "null" is later retrieved and parsed, subscription_adapter.validate_python(plan_dict) will fail because None doesn't match SubscriptionPlan schema
4. This causes the tenant to be treated as a cache miss on every subsequent call, defeating the purpose of caching

Expected Behavior

Only valid SubscriptionPlan values should be cached. None values should be filtered out before caching.

Recommendation: Add a check to filter out None values before storing in tenant_plans and plans_to_cache. For example:

for tenant_id, subscription_plan in bulk_plans.items():
    if subscription_plan is not None:
        tenant_plans[tenant_id] = subscription_plan
        plans_to_cache[tenant_id] = subscription_plan

---

Was this helpful? React with 👍 or 👎 to provide feedback.