refactor: consolidate rate-limit factories, fix three latent bugs

[lfiaschi]

Summary

Outcome of a deep codebase review focused on high-leverage, low-risk cleanups. Four issues, each landed with regression tests. +371 / -123 LOC, 945 server tests pass (up from 933), ruff and mypy clean.

Findings & fixes

1. Rate-limit factory duplication — DRY (code-health, M impact / S effort)

Nine near-identical _enforce_*_rate_limit functions across three route modules each did the same hasattr(state, ...) -> RateLimiter -> stash on state dance:

server/src/decision_hub/api/registry_routes.py:86-167   (7 copies)
server/src/decision_hub/api/search_routes.py:32-41      (1 copy)
server/src/decision_hub/api/auth_routes.py:29-38        (1 copy)

Fix: Centralise the lazy-init dance in rate_limit.get_or_create_limiter(app_state, attr_name, *, max_requests, window_seconds) -> RateLimiter. Drive the seven registry_routes enforcers from a _RATE_LIMIT_CONFIGS table via _make_enforcer() — each generated function keeps a unique __name__ so FastAPI's dependency cache still treats them as distinct. The other two enforcers (search, auth) collapse to three lines apiece.

2. RateLimiter stale-IP purge rarely fires — bug + perf (M / S)

server/src/decision_hub/api/rate_limit.py:57 previously read:

total = sum(len(v) for v in self._requests.values())
if total % 100 == 0:
    self._purge_stale(cutoff)

Because each call prunes expired timestamps for its own key first, the sum stays small (typically ~1 per active IP after the per-call prune). The modulo therefore only hits zero by coincidence; under a single hot IP it never fires. Memory grows unbounded.

Fix: Per-instance call counter (_PURGE_EVERY_N_CALLS = 1024). Purge runs deterministically every N requests regardless of traffic shape.

3. Non-UTF-8 source files surface as HTTP 500 — bug (M / S)

server/src/decision_hub/domain/publish.py:124-128 called .decode() (strict UTF-8) on every scannable file. A .py entry containing arbitrary bytes therefore raised UnicodeDecodeError, which /v1/publish doesn't catch — its except clause only handles (ValueError, zipfile.BadZipFile). The user gets a 500.

Fix:

• Source files: decode with errors="replace" so the scanner still sees a string, and append to unscanned_files so the gauntlet's partial-coverage check downgrades the grade.
• SKILL.md and lockfiles: keep strict UTF-8 (they drive identity and dependency resolution); raise a clean ValueError (→ 422) when malformed.

4. seo_routes.sitemap_xml cached a Starlette Response object — bug (M / S)

server/src/decision_hub/api/seo_routes.py stashed the constructed Response in the TTL cache and returned the cached instance directly on subsequent hits. Response objects carry mutable per-request state (header finalisation, charset, etc.) and the Starlette docs are explicit they're not meant to be shared.

Fix: Split rendering into _build_sitemap_xml(conn) -> str and cache only the XML body. The route builds a fresh Response on every hit.

Issues considered but deferred

• server/src/decision_hub/infra/database.py is 3,465 lines — splitting by domain table grouping would be valuable but is out of scope (architectural M/L).
• server/src/decision_hub/api/registry_routes.py is 1,352 lines and mixes registry, eval runs, visibility, and access endpoints — same reasoning.
• _run_to_response / _tracker_to_response converters duplicate ISO-format date logic — low value, skipped.

Test plan

• uv run --package decision-hub-server --extra dev pytest server/tests -m "not slow" — 945 passed, 34 deselected
• uv run --package dhub-core --extra dev pytest shared/tests — 98 passed
• uvx ruff check . — clean
• uvx ruff format --check . — clean
• uvx mypy server/src/decision_hub/api/rate_limit.py server/src/decision_hub/api/registry_routes.py server/src/decision_hub/api/seo_routes.py server/src/decision_hub/domain/publish.py — clean
• New tests:
  • test_rate_limit.py::TestStaleIpPurge::test_idle_ips_eventually_evicted — idle IPs dropped after the window
  • test_rate_limit.py::TestStaleIpPurge::test_purge_cadence_does_not_depend_on_per_ip_rate — single hot IP still triggers purge
  • test_rate_limit.py::TestGetOrCreateLimiter — lazy init / identity / distinctness
  • test_publish.py::TestNonUtf8Handling::test_binary_source_file_is_replaced_and_flagged
  • test_publish.py::TestNonUtf8Handling::test_non_utf8_skill_md_raises_value_error
  • test_publish.py::TestNonUtf8Handling::test_non_utf8_lockfile_raises_value_error
  • test_seo_routes.py::TestSitemapCaching::test_cache_stores_xml_string_not_response_object
  • test_seo_routes.py::TestSitemapCaching::test_cache_hit_returns_fresh_response_with_same_body
  • test_seo_routes.py::TestSitemapCaching::test_ttl_zero_disables_cache

https://claude.ai/code/session_016diHyWN9knNJAKe5VwmezW

---

Generated by Claude Code

[lfiaschi]

Closing as part of the 2026-08-11 open-PR consolidation (see #451). Overlaps the retained merge queue (#449, #448, #447, #446, #405, #438, #443, #375). Branch preserved.