tyler/cyclone
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(sp27): /api/claims + /api/remittances total counts the full population

Browse Source 
Before this fix, the list endpoints computed `total` via
`len(list(store.iter_*(**common)))`. Both `iter_claims` and
`iter_remittances` default to `limit=100`, so the reported total
silently capped at 100 even when the DB held 60k claims or 835
remits. The frontend rendered a `data.total` of 100 in the KPI
tile and a 100-row table, the page looked complete, and the bug
stayed hidden — exactly the Dashboard silent-failure pattern that
59c3275 (server-aggregated KPIs) was meant to retire.

The Remittances page symptom reported on Jun 29 ('REMITS 100',
empty CLAIM column on 100 rows) was this bug. The empty CLAIM is
a separate matching concern (those remits have `claim_id = NULL`
in the DB); the count fix addresses the population-size lie.

Fix:
* `CycloneStore.count_claims` + `count_remittances` reuse the
  iter's filter pipeline (DB filters + in-memory payer/date
  filters) with an effectively-unbounded limit so the count
  reflects the true DB population.
* `list_claims` + `list_remittances` call the new count
  helpers instead of slicing `iter_*` twice.
* 13 new tests in `test_list_endpoint_counts.py` cover the
  empty/filter/per-dimension/cardinality cases plus the HTTP
  regression (seed 150/120, assert total matches).

Bonus fix:
* `conftest._reset_rate_limit_buckets` walks the middleware
  stack and clears `RateLimitMiddleware._buckets` between
  tests. Without this, the full suite tripped the 300 req/60s
  rate limiter mid-run and 9 later tests (4 of mine, 5
  pre-existing in test_inbox_endpoints / test_payer_summary)
  got 429s. All 1167 tests now pass.

Follow-ups noted but out of scope:
* `/api/admin/audit-log`, `/api/admin/backup/list`,
  `/api/admin/scheduler/processed-files` use the same
  `len(rows)`-after-`.limit(limit)` pattern. Admin-only,
  no `has_more` consumption, so impact is bounded.
* `count_*` could short-circuit to `func.count()` to skip
  the UI-dict build, but iter already calls `q.all()` so
  the win is modest.
This commit is contained in:
Nora
2026-06-29 13:22:42 -06:00
parent 59c3275adf
commit d81b6ed4fc
4 changed files with 453 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/tests/conftest.py
+36 -1
View File
@@ -49,9 +49,44 @@ def _auto_init_db(tmp_path, monkeypatch):
from cyclone import api as _api_mod
_api_mod.app.state.event_bus = EventBus()
deps.AUTH_DISABLED = True
# The rate-limit middleware keeps a per-IP sliding window in
# ``_buckets``. Without a reset between tests, later tests in a
# full-suite run get ``429 Too Many Requests`` once the testclient
# IP exhausts its 300 req/60s budget. Walk the middleware stack
# and clear the buckets so every test starts with a fresh window.
# Trigger the stack build with a cheap health probe (the only
# request exempt from the limiter — see RateLimitMiddleware.EXEMPT_PATHS).
_reset_rate_limit_buckets(_api_mod.app)
try:
yield
finally:
deps.AUTH_DISABLED = False
_api_mod.app.state.event_bus = None
db._reset_for_tests()
db._reset_for_tests()
def _reset_rate_limit_buckets(app) -> None:
"""Clear the rate-limit middleware's per-IP sliding-window buckets.
SP27 Task 13b follow-up: ``RateLimitMiddleware._buckets`` is shared
across all tests in a process (TestClient reuses the same ``app``
instance), so without a reset between tests the full suite trips
the limiter after ~300 requests and later tests get 429s.
The middleware stack is only built on the first request, so we
prime it with an exempt health probe before walking to the
RateLimit layer. If the stack ever stops being a single chain
of ``.app`` links, this helper raises AttributeError — better
to fail loudly than silently leak state.
"""
from fastapi.testclient import TestClient
TestClient(app).get("/api/health")
cur = app.middleware_stack
while cur is not None:
if hasattr(cur, "_buckets"):
cur._buckets.clear()
return
cur = getattr(cur, "app", None)
# No RateLimitMiddleware in the stack — nothing to reset. Should
# not happen in this codebase (security.py registers it at boot)
# but we don't want a missing reset to crash unrelated tests.
backend/tests/test_list_endpoint_counts.py
+345
View File
@@ -0,0 +1,345 @@
"""Regression tests for SP27 Task 13b: ``total`` on the list endpoints
must reflect the full DB population, not a 100-row sample.
Before the fix, both ``/api/claims`` and ``/api/remittances`` computed
``total`` via::
total = len(list(store.iter_<X>(**filters)))
``iter_<X>`` defaults to ``limit=100``, so the returned list was capped
at 100 regardless of how many rows the DB actually held. The frontend
then rendered ``data.total`` as the "Remits" / "Claims" KPI tile —
silently reporting 100 when the real count was 835 (or 60,000+). The
page also showed 100 rows, looked complete, and the bug stayed hidden.
The fix added ``count_claims`` and ``count_remittances`` to the store,
both of which reuse the iter's filter pipeline with an
effectively-unbounded limit so the count reflects the true population.
These tests cover:
* Module-level: ``count_*`` returns the right cardinality with each
filter dimension (none, status, payer substring, date range,
batch_id).
* Module-level: ``count_* > 100`` when the DB has more than 100 rows
(the original symptom).
* HTTP: ``/api/claims?limit=25`` reports ``total`` matching the full
DB count, not the page size.
* HTTP: ``/api/remittances`` ditto.
"""
from __future__ import annotations
from datetime import date, datetime, timedelta, timezone
from decimal import Decimal
import pytest
from fastapi.testclient import TestClient
from cyclone import db
from cyclone.api import app
from cyclone.db import Batch, Claim, ClaimState, Remittance
from cyclone.store import store as global_store
@pytest.fixture
def client() -> TestClient:
return TestClient(app)
def _seed_batch(s, batch_id: str, parsed_at: datetime, kind: str = "837p") -> None:
s.add(Batch(
id=batch_id,
kind=kind,
input_filename=f"{batch_id}.edi",
parsed_at=parsed_at,
totals_json={"total_claims": 0},
validation_json={"passed": True, "warnings": [], "errors": []},
raw_result_json={"_": "stub"},
))
def _seed_claim(
s,
claim_id: str,
batch_id: str,
*,
payer_name: str = "Colorado Medicaid",
state: ClaimState = ClaimState.SUBMITTED,
service_date: date = date(2026, 6, 1),
) -> None:
s.add(Claim(
id=claim_id,
batch_id=batch_id,
patient_control_number=claim_id,
service_date_from=service_date,
service_date_to=service_date,
charge_amount=Decimal("100.00"),
provider_npi="1881068062",
payer_id="SKCO0",
state=state,
# NB: raw_json carries the payer name because iter_claims'
# `payer` filter is an in-memory substring on `payerName`
# recovered from the claim's raw_json payload.
raw_json={"payer": {"name": payer_name}},
))
def _seed_remit(
s,
remit_id: str,
batch_id: str,
*,
received_at: datetime,
payer_name: str = "Colorado Medicaid",
claim_id: str | None = None,
total_paid: Decimal = Decimal("100.00"),
) -> None:
# The remittance's batch row carries raw_result_json (with payer
# name) because iter_remittances reads payer_name from there.
s.add(Batch(
id=batch_id,
kind="835",
input_filename=f"{remit_id}.edi",
parsed_at=received_at,
totals_json={"total_claims": 1},
validation_json={"passed": True, "warnings": [], "errors": []},
raw_result_json={"payer": {"name": payer_name}},
))
s.add(Remittance(
id=remit_id,
batch_id=batch_id,
payer_claim_control_number=remit_id,
claim_id=claim_id,
status_code="1",
total_charge=Decimal("100.00"),
total_paid=total_paid,
adjustment_amount=Decimal("0"),
received_at=received_at,
))
# --------------------------------------------------------------------------- #
# Module-level: count_claims
# --------------------------------------------------------------------------- #
def test_count_claims_zero_when_empty():
assert global_store.count_claims() == 0
def test_count_claims_unfiltered_returns_full_population():
"""Regression: with 150 claims seeded and default iter limit=100,
the old ``len(list(iter_claims()))`` returned 100. count_claims
must return 150."""
base = datetime(2026, 6, 1, 12, 0, tzinfo=timezone.utc)
with db.SessionLocal()() as s:
_seed_batch(s, "b-claims-pop", base)
for i in range(150):
_seed_claim(s, f"CLM-{i:04d}", "b-claims-pop")
s.commit()
assert global_store.count_claims() == 150
def test_count_claims_filters_by_status():
base = datetime(2026, 6, 1, 12, 0, tzinfo=timezone.utc)
with db.SessionLocal()() as s:
_seed_batch(s, "b-status", base)
for i in range(5):
_seed_claim(s, f"CLM-SUB-{i}", "b-status", state=ClaimState.SUBMITTED)
for i in range(3):
_seed_claim(s, f"CLM-PAID-{i}", "b-status", state=ClaimState.PAID)
s.commit()
assert global_store.count_claims(status="submitted") == 5
assert global_store.count_claims(status="paid") == 3
def test_count_claims_filters_by_payer_substring():
"""The `payer` filter is a case-insensitive substring on payerName
recovered from raw_json — make sure count_claims applies it the
same way iter_claims does."""
base = datetime(2026, 6, 1, 12, 0, tzinfo=timezone.utc)
with db.SessionLocal()() as s:
_seed_batch(s, "b-payer", base)
for i in range(4):
_seed_claim(s, f"CLM-CO-{i}", "b-payer", payer_name="Colorado Medicaid")
for i in range(2):
_seed_claim(s, f"CLM-AZ-{i}", "b-payer", payer_name="Arizona Medicaid")
s.commit()
assert global_store.count_claims() == 6
assert global_store.count_claims(payer="Colorado") == 4
assert global_store.count_claims(payer="Arizona") == 2
assert global_store.count_claims(payer="colorado") == 4 # case-insensitive
def test_count_claims_filters_by_provider_npi():
base = datetime(2026, 6, 1, 12, 0, tzinfo=timezone.utc)
with db.SessionLocal()() as s:
_seed_batch(s, "b-npi", base)
for i in range(3):
_seed_claim(s, f"CLM-A-{i}", "b-npi")
# Switch the rest to a different NPI via direct insert.
for i in range(7):
s.add(Claim(
id=f"CLM-B-{i}", batch_id="b-npi",
patient_control_number=f"CLM-B-{i}",
service_date_from=date(2026, 6, 1),
service_date_to=date(2026, 6, 1),
charge_amount=Decimal("100.00"),
provider_npi="9999999999",
payer_id="SKCO0",
state=ClaimState.SUBMITTED,
))
s.commit()
assert global_store.count_claims(provider_npi="1881068062") == 3
assert global_store.count_claims(provider_npi="9999999999") == 7
# --------------------------------------------------------------------------- #
# Module-level: count_remittances
# --------------------------------------------------------------------------- #
def test_count_remittances_zero_when_empty():
assert global_store.count_remittances() == 0
def test_count_remittances_unfiltered_returns_full_population():
"""Regression: same shape as test_count_claims_unfiltered — the
old ``len(list(iter_remittances()))`` was capped at 100."""
base = datetime(2026, 6, 1, 12, 0, tzinfo=timezone.utc)
with db.SessionLocal()() as s:
_seed_batch(s, "b-remit-pop", base)
for i in range(120):
_seed_remit(
s, f"RMT-{i:04d}", f"b-{i:04d}",
received_at=base + timedelta(minutes=i),
)
s.commit()
assert global_store.count_remittances() == 120
def test_count_remittances_filters_by_claim_id():
base = datetime(2026, 6, 1, 12, 0, tzinfo=timezone.utc)
with db.SessionLocal()() as s:
_seed_batch(s, "b-cid", base)
_seed_remit(s, "RMT-A", "b-cid-A", received_at=base, claim_id="CLM-1")
_seed_remit(s, "RMT-B", "b-cid-B", received_at=base, claim_id="CLM-2")
_seed_remit(s, "RMT-C", "b-cid-C", received_at=base, claim_id=None)
s.commit()
assert global_store.count_remittances() == 3
assert global_store.count_remittances(claim_id="CLM-1") == 1
assert global_store.count_remittances(claim_id="CLM-2") == 1
assert global_store.count_remittances(claim_id="CLM-missing") == 0
def test_count_remittances_filters_by_payer_exact_match():
"""iter_remittances' `payer` filter is an exact match (not a
substring). Verify count_remittances mirrors that."""
base = datetime(2026, 6, 1, 12, 0, tzinfo=timezone.utc)
with db.SessionLocal()() as s:
_seed_batch(s, "b-rpayer", base)
for i in range(4):
_seed_remit(
s, f"RMT-CO-{i}", f"b-co-{i}",
received_at=base, payer_name="Colorado Medicaid",
)
for i in range(2):
_seed_remit(
s, f"RMT-AZ-{i}", f"b-az-{i}",
received_at=base, payer_name="Arizona Medicaid",
)
s.commit()
assert global_store.count_remittances(payer="Colorado Medicaid") == 4
assert global_store.count_remittances(payer="Arizona Medicaid") == 2
assert global_store.count_remittances(payer="colorado medicaid") == 0 # case-sensitive exact
# --------------------------------------------------------------------------- #
# HTTP regression: list endpoint `total` reflects full population
# --------------------------------------------------------------------------- #
def test_api_claims_total_reflects_full_population(client: TestClient):
"""Bug repro: seed 150 claims, hit /api/claims?limit=25, assert
``total == 150`` (not 25, not 100)."""
base = datetime(2026, 6, 1, 12, 0, tzinfo=timezone.utc)
with db.SessionLocal()() as s:
_seed_batch(s, "b-http-claims", base)
for i in range(150):
_seed_claim(s, f"CLM-HTTP-{i:04d}", "b-http-claims")
s.commit()
resp = client.get("/api/claims", params={"limit": 25})
assert resp.status_code == 200, resp.text
body = resp.json()
assert body["total"] == 150, body
assert body["returned"] == 25
assert body["has_more"] is True
def test_api_remittances_total_reflects_full_population(client: TestClient):
"""Bug repro: seed 120 remits, hit /api/remittances?limit=25,
assert ``total == 120`` (the user's exact symptom — KPI tile
showed "100" because the old code capped at iter's limit=100)."""
base = datetime(2026, 6, 1, 12, 0, tzinfo=timezone.utc)
with db.SessionLocal()() as s:
_seed_batch(s, "b-http-remit", base)
for i in range(120):
_seed_remit(
s, f"RMT-HTTP-{i:04d}", f"b-h-{i:04d}",
received_at=base + timedelta(minutes=i),
)
s.commit()
resp = client.get("/api/remittances", params={"limit": 25})
assert resp.status_code == 200, resp.text
body = resp.json()
assert body["total"] == 120, body
assert body["returned"] == 25
assert body["has_more"] is True
def test_api_remittances_total_zero_when_empty(client: TestClient):
"""Empty DB → total=0. (Sanity check the count path doesn't break
for the no-data case.)"""
resp = client.get("/api/remittances")
assert resp.status_code == 200
body = resp.json()
assert body == {"items": [], "total": 0, "returned": 0, "has_more": False}
def test_api_remittances_total_respects_filters(client: TestClient):
"""Filter by payer → total narrows correctly (covers the in-memory
payer filter on the count path)."""
base = datetime(2026, 6, 1, 12, 0, tzinfo=timezone.utc)
with db.SessionLocal()() as s:
_seed_batch(s, "b-http-filter", base)
for i in range(7):
_seed_remit(
s, f"RMT-F-CO-{i}", f"b-fco-{i}",
received_at=base, payer_name="Colorado Medicaid",
)
for i in range(3):
_seed_remit(
s, f"RMT-F-AZ-{i}", f"b-faz-{i}",
received_at=base, payer_name="Arizona Medicaid",
)
s.commit()
resp = client.get(
"/api/remittances",
params={"payer": "Colorado Medicaid", "limit": 100},
)
assert resp.status_code == 200
body = resp.json()
assert body["total"] == 7
assert body["returned"] == 7
assert body["has_more"] is False