fix(proxy): reject rate_limit_requests_per_minute=0 when limiting is enabled (#2142)

## Description

A `rate_limit_requests_per_minute` of 0 makes the proxy return a 500 on
every rate-limited request instead of failing configuration early.

The token-bucket wait computation divides by the per-minute rate:

```python
def consume_from_bucket(*, available_tokens, requested_tokens, rate_per_minute):
    if available_tokens >= requested_tokens:
        return True, available_tokens - requested_tokens, 0.0
    wait_seconds = (requested_tokens - available_tokens) * (60.0 / rate_per_minute)
    return False, available_tokens, wait_seconds
```

With `rate_limit_requests_per_minute == 0`, the bucket initializes to 0
tokens, so the first request reaches the division and raises
`ZeroDivisionError`. The CLI guards `--rpm` with
`click.IntRange(min=1)`, but `HEADROOM_PROXY_CONFIG_JSON` and
programmatic `ProxyConfig(...)` construction bypass that guard.

## Fix

Validate `rate_limit_requests_per_minute >= 1` in
`ProxyConfig.__post_init__` when `rate_limit_enabled`, mirroring the
existing `retry_max_attempts` validation. Bad enabled configs now fail
fast with a clear message. When rate limiting is disabled, `rpm=0`
remains inert and is not rejected.

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)

## Changes Made

- `headroom/proxy/models.py`: reject `rate_limit_requests_per_minute <
1` when `rate_limit_enabled`.
- `tests/test_proxy_config_rate_limit.py`: cover zero/negative enabled
values, disabled zero, and a valid enabled value.
- `CHANGELOG.md`: add a bug-fix entry.
- Merged current `main` to pick up the repository-wide memory factory
type annotation fix that was breaking the PR lint job.

## Testing

- [x] Unit tests pass (`pytest` focused locally; broader CI passed on
the pre-merge head and fresh CI is running on the main-merged head)
- [x] Linting passes (`ruff check` focused locally)
- [x] Type checking passes for the prior mypy blocker after merging
current `main`
- [x] New tests added for new functionality
- [ ] Manual testing performed

### Test Output

```text
uvx ruff@0.15.17 check headroom/proxy/models.py tests/test_proxy_config_rate_limit.py headroom/memory/factory.py
All checks passed!

uvx ruff@0.15.17 format --check headroom/proxy/models.py tests/test_proxy_config_rate_limit.py headroom/memory/factory.py
3 files already formatted

git diff --check headroomlabs/main...HEAD
# no output

uv run --extra dev python -m pytest tests/test_proxy_config_rate_limit.py -q
4 passed
```

## Real Behavior Proof

- Environment: Windows 11 review worktree, Python 3.13.3.
- Exact command / steps: ran the focused rate-limit config test file and
targeted lint/format checks.
- Observed result: enabled zero and negative rpm raise `ValueError`;
disabled zero is accepted; valid enabled rpm is accepted.
- Not tested: full suite; fresh CI is queued after the main merge.

## Review Readiness

- [x] I have performed a self-review
- [x] This PR is ready for human review

## Checklist

- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [x] I have updated the CHANGELOG.md if applicable

## Additional Notes

The validation is intentionally at the config boundary to match the
CLI's `IntRange(min=1)` contract and the existing fail-fast
`retry_max_attempts` check.

Co-authored-by: JerrettDavis <mxjerrett@gmail.com>

CHANGELOG.md

@@ -125,6 +125,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * **memory:** size the HNSW `index_batch` resize off the assigned-id high-water mark, not the live entry count. hnswlib never reclaims a slot on `mark_deleted` (used by remove/evict), so its usable capacity is bounded by the number of ids ever assigned (`_next_hnsw_id`). `index_batch` computed `required_capacity = len(self._memory_to_hnsw) + len(new_memories)` — the *live* count — which drops below `_next_hnsw_id` after deletion/eviction churn, so the resize was skipped and `add_items` raised `RuntimeError: number of elements exceeds the specified limit`, crashing the save path on the HNSW backend. It now resizes off `_next_hnsw_id`, matching the single-item `index()` guard.
 * **memory:** apply the `turn_id` scope filter even when `agent_id` is absent. In `SQLiteMemoryStore._build_query_conditions` the `turn_id` condition was nested inside the `agent_id` block, so a query filtered by `user_id` + `session_id` + `turn_id` (no `agent_id`) dropped the `turn_id` predicate entirely and returned every memory in the session instead of the single turn — an over-broad result that leaks sibling-turn memories into recall (and makes `count()` wrong for that scope). `agent_id` and `turn_id` are now applied independently.
 * **transforms:** stop the lossless `diff` fold from silently dropping lines out of non-diff content. `ContentRouter._lossless_first` tries every `compact_lossless` fold on all content, but the `diff` kind (`diff_strip_index`) is the only one with no exact-inverse check — it removes any line shaped like `index <hex>..<hex>`. Applied to arbitrary text/log/search payloads that happen to contain such a line, that line was deleted with no CCR marker, so it was unrecoverable — a violation of the lossless no-loss contract the method's own docstring promises. The `diff` fold now runs only when the strategy is `DIFF` or the content is diff-shaped (`_looks_like_diff`); genuine diffs still have their `index` bookkeeping folded.
+* **proxy:** reject a 0 `rate_limit_requests_per_minute` when rate limiting is enabled, instead of 500-ing every request. The token-bucket wait computation divides by the per-minute rate (`consume_from_bucket`), so a `rate_limit_requests_per_minute` of 0 raised `ZeroDivisionError` on every request that hit the limiter. The CLI already guards this with `IntRange(min=1)`, but the `HEADROOM_PROXY_CONFIG_JSON` / programmatic config paths bypassed it. `ProxyConfig.__post_init__` now validates `rate_limit_requests_per_minute >= 1` when `rate_limit_enabled` (mirroring the existing `retry_max_attempts` check), so a bad value fails fast at construction with a clear message; it stays inert when limiting is disabled.
 * **memory:** include the Ollama server URL in the embedder cache key so a second backend can't get an embedder bound to the wrong server. `_create_embedder` cached by `(backend, model)` only, but the Ollama embedder is constructed with `base_url=config.ollama_base_url`. Two configs in the same process that shared a backend and model but pointed at different Ollama servers (e.g. a per-project storage router) collided on one cache slot, so the second silently reused the first's embedder and embedded against the wrong host. The cache key now also includes `ollama_base_url`.
 * **tokenizers:** use `o200k_base` for the gpt-4.1 / gpt-4.5 / o4 families in `get_encoding_for_model`. `gpt-4.1*` and `gpt-4.5*` matched the broad `gpt-4` prefix and were encoded with `cl100k_base`, and `o4*` matched no prefix and fell through to the `cl100k_base` default — all three use `o200k_base`, so their token counts were computed with the wrong vocabulary. Added explicit `gpt-4.1`/`gpt-4.5` prefixes ahead of `gpt-4` and an `o4` prefix; `gpt-4` and `gpt-3.5` snapshots still resolve to `cl100k_base`.
 * **cache/ccr:** stop counting a successful eviction as a retrieval in the compression feedback learner, which inverted the learning signal. When an entry is evicted without ever being retrieved, `CompressionStore` emits a synthetic `retrieval_type="eviction_success"` event to mark that the compression was sufficient (the LLM never needed the original). `CompressionFeedback.record_retrieval` had no branch for it, so — because the type is not `"full"` — it was counted as a *search retrieval*, inflating the tool's `retrieval_rate`/`search_rate`. `get_compression_hints` reads a high retrieval rate as "compressing too aggressively" and backs off, so a compression that actually worked pushed the learner toward *less* compression (a standalone repro scores one successful eviction as a 100% retrieval rate). The event is now recognized and left out of the retrieval counters; the compression is still counted by `record_compression` at store time, so a never-retrieved entry correctly yields a low retrieval rate. Genuine retrievals are unaffected.

headroom/proxy/models.py

@@ -440,6 +440,15 @@ class ProxyConfig:
     def __post_init__(self, smart_routing: bool | None = None) -> None:
         if self.retry_enabled and self.retry_max_attempts < 1:
             raise ValueError("retry_max_attempts must be >= 1 when retry_enabled=True")
+        # A 0 (or negative) requests-per-minute limit divides by zero in the
+        # token-bucket wait computation (rate_limit_policy.consume_from_bucket),
+        # 500-ing every request. The CLI already guards this with IntRange(min=1);
+        # fail fast here too so the JSON/programmatic config paths can't produce a
+        # limiter that crashes at request time. Only matters when limiting is on.
+        if self.rate_limit_enabled and self.rate_limit_requests_per_minute < 1:
+            raise ValueError(
+                "rate_limit_requests_per_minute must be >= 1 when rate_limit_enabled=True"
+            )
 
     @property
     def provider_api_overrides(self) -> ProviderApiOverrides:

tests/test_proxy_config_rate_limit.py

@@ -0,0 +1,30 @@
+"""ProxyConfig must reject a 0 requests-per-minute limit when rate limiting is on
+(it would divide by zero in the token-bucket wait computation and 500 every
+request), while leaving it inert when limiting is off."""
+
+from __future__ import annotations
+
+import pytest
+
+from headroom.proxy.models import ProxyConfig
+
+
+def test_zero_rpm_with_limiting_enabled_is_rejected():
+    with pytest.raises(ValueError, match="rate_limit_requests_per_minute must be >= 1"):
+        ProxyConfig(rate_limit_enabled=True, rate_limit_requests_per_minute=0)
+
+
+def test_negative_rpm_with_limiting_enabled_is_rejected():
+    with pytest.raises(ValueError, match="rate_limit_requests_per_minute must be >= 1"):
+        ProxyConfig(rate_limit_enabled=True, rate_limit_requests_per_minute=-5)
+
+
+def test_zero_rpm_is_inert_when_limiting_disabled():
+    # Limiting off -> the bucket is never consulted, so a 0 limit is harmless.
+    config = ProxyConfig(rate_limit_enabled=False, rate_limit_requests_per_minute=0)
+    assert config.rate_limit_requests_per_minute == 0
+
+
+def test_valid_rpm_is_accepted():
+    config = ProxyConfig(rate_limit_enabled=True, rate_limit_requests_per_minute=60)
+    assert config.rate_limit_requests_per_minute == 60