From 8a905232091d993fac9e19a59bc449f201d4cdf3 Mon Sep 17 00:00:00 2001 From: Abhay Singh Date: Wed, 12 Aug 2026 10:18:22 +0530 Subject: [PATCH] fix(transforms/adaptive-sizer): honor max_k on small-input fast path (#2319) ## Description `compute_optimal_k` in the adaptive sizer takes a `max_k` argument documented as "Never return more than this (None = no cap)". Every tier honors that contract except the small-input fast path. ```python n = len(items) effective_max = max_k if max_k is not None else n # Tier 1: Fast path if n <= 8: return n ``` The near-total-redundancy branch returns `min(k, effective_max)`, the standard tier ends with `k = max(min_k, min(k, effective_max))`, and the zlib validator clamps to `max_k` too. Only the `n <= 8` fast path returns the raw item count, ignoring the cap. So a caller that passes a tight budget on a small list gets back more items than it asked for. For example `compute_optimal_k(items_of_len_8, max_k=5)` returns `8`, not `5`. The downstream compressor then keeps 8 items when it budgeted for 5, over-filling whatever search/log budget the cap represented. ## Fix Return `min(n, effective_max)` on the fast path, matching what the other tiers already do: ```python if n <= 8: return min(n, effective_max) ``` When `max_k` is `None`, `effective_max` is `n`, so `min(n, n) == n` and the existing "return n unchanged" behavior is preserved. Only the capped case changes. ## 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/transforms/adaptive_sizer.py`: clamp the `n <= 8` fast path to `effective_max` so `max_k` is honored on small inputs. - `tests/test_adaptive_sizer.py`: add `test_small_array_respects_max_k` asserting a small array honors a tight `max_k` and is unchanged when the cap is loose. - `CHANGELOG.md`: Bug Fixes entry. ## Testing - [ ] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ uvx ruff@0.15.17 check headroom/transforms/adaptive_sizer.py tests/test_adaptive_sizer.py All checks passed! $ uvx ruff@0.15.17 format --check headroom/transforms/adaptive_sizer.py tests/test_adaptive_sizer.py 2 files already formatted $ uvx mypy@1.20.2 --ignore-missing-imports headroom/transforms/adaptive_sizer.py Success: no issues found in 1 source file ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17` / `uvx mypy@1.20.2`. A full `pytest` OOMs this box (ML-stack import), so I reproduced the tier-1 logic with a dependency-free script and left the full pytest to CI. - Exact command / steps: ran both the OLD (`return n`) and NEW (`return min(n, effective_max)`) fast-path logic for `n=8` across `max_k` in `{3, 5, 20, None}` in a standalone script. - Observed result: OLD returned `8` for every case (ignoring the cap); NEW returned `3, 5, 8, 8` respectively, matching the documented contract and leaving the uncapped case unchanged. - Not tested: the end-to-end search/log compressor path that supplies `max_k`; the added unit test exercises `compute_optimal_k` directly, and the standalone proof pins the fast-path arithmetic. ## 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 "unit tests pass locally" box is unchecked because this box's ML-stack import OOMs a local pytest run; the added test is a pure dataclass-free check that runs under the normal CI pytest job, and the behavior is corroborated by the standalone proof above. --------- Co-authored-by: JerrettDavis --- headroom/transforms/adaptive_sizer.py | 7 ++++++- tests/test_adaptive_sizer.py | 12 ++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/headroom/transforms/adaptive_sizer.py b/headroom/transforms/adaptive_sizer.py index 9a867d60a..5f73aabd8 100644 --- a/headroom/transforms/adaptive_sizer.py +++ b/headroom/transforms/adaptive_sizer.py @@ -51,7 +51,12 @@ def compute_optimal_k( # Tier 1: Fast path if n <= 8: - return n + # Still honor the caller's hard cap: ``max_k`` is documented as "never + # return more than this". Returning the raw ``n`` let a small ``max_k`` + # (e.g. a tight search/log budget) be exceeded on tiny inputs, so the + # caller kept more items than it asked for. The near-duplicate branch + # below already clamps to ``effective_max``; do the same here. + return min(n, effective_max) # Check for near-total redundancy unique_count = count_unique_simhash(items) diff --git a/tests/test_adaptive_sizer.py b/tests/test_adaptive_sizer.py index f20746d56..f9ae55c6d 100644 --- a/tests/test_adaptive_sizer.py +++ b/tests/test_adaptive_sizer.py @@ -107,6 +107,18 @@ class TestSmallArrays: items = _make_unique_items(8) assert compute_optimal_k(items) == 8 + def test_small_array_respects_max_k(self): + """A small array (n <= 8) must still honor a tight ``max_k`` cap. + + ``max_k`` is documented as "never return more than this"; the fast path + used to return the raw ``n`` and blow past a small cap. + """ + items = _make_unique_items(8) + assert compute_optimal_k(items, max_k=5) == 5 + assert compute_optimal_k(items, max_k=3) == 3 + # A cap >= n leaves the array unchanged. + assert compute_optimal_k(items, max_k=20) == 8 + class TestNearTotalRedundancy: def test_identical_items_returns_min(self):