mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
refactor(memory): isolate injection decision policy (#1952)
## Description Extracts the memory injection decision precedence and skip-reason tag stamping into a pure policy module while preserving the public `MemoryDecision.decide` API used by handlers. This keeps the frozen decision value type separate from the gate policy it wraps. Closes # ## Type of Change - [ ] 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 - [x] Code refactoring (no functional changes) ## Changes Made - Added `headroom.proxy.memory_decision_policy` for pure memory-injection precedence and tag stamping helpers. - Updated `MemoryDecision.decide` and `MemoryDecision.apply_to_tags` to delegate to the extracted policy. - Added direct tests for the extracted policy boundary. - Kept the LiteLLM callback compatibility shim required for repo-wide type checking on fresh branches. ## Testing - [x] 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 python -m pytest tests/test_memory_decision_policy.py tests/test_memory_decision.py tests/test_memory_invariants.py tests/test_litellm_callback.py tests/test_compress_api.py::TestLiteLLMCallback -q 37 passed in 6.74s python -m ruff check . All checks passed! python -m ruff format --check . 1095 files already formatted python -m mypy headroom --ignore-missing-imports Success: no issues found in 409 source files gitleaks protect --staged --no-banner --redact no leaks found ``` ## Real Behavior Proof - Environment: Windows, Python 3.13.13, local worktree based on `headroomlabs/main`. - Exact command / steps: ran focused memory decision tests, memory invariant tests, LiteLLM callback tests, Ruff lint/format checks, mypy over `headroom`, and staged gitleaks scan. - Observed result: all local checks passed; staged secret scan found no leaks. - Not tested: full CI matrix and deployment flows; those are covered by GitHub Actions. ## 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 - [x] New and existing unit tests pass locally with my changes - [ ] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) N/A ## Additional Notes Documentation and changelog updates are not applicable for this internal refactor. GitHub reported existing Dependabot alerts on the default branch during push; this PR does not change dependencies, and the staged secret scan is clean.
This commit is contained in:
parent
235c986c9c
commit
c20f3b1c04
3 changed files with 144 additions and 31 deletions
|
|
@ -33,7 +33,10 @@ from __future__ import annotations
|
|||
from dataclasses import dataclass
|
||||
from typing import Any
|
||||
|
||||
from headroom.proxy.helpers import _headroom_bypass_enabled
|
||||
from headroom.proxy.memory_decision_policy import (
|
||||
apply_memory_skip_reason,
|
||||
decide_memory_injection,
|
||||
)
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
|
|
@ -96,36 +99,20 @@ class MemoryDecision:
|
|||
Comes from ``get_memory_injection_mode()`` which reads
|
||||
``HEADROOM_MEMORY_INJECTION_MODE``.
|
||||
"""
|
||||
bypass = _headroom_bypass_enabled(headers)
|
||||
has_handler = memory_handler is not None
|
||||
has_user = bool(memory_user_id)
|
||||
|
||||
if bypass:
|
||||
reason: str | None = "bypass_header"
|
||||
inject = False
|
||||
elif not has_handler:
|
||||
reason = "no_handler"
|
||||
inject = False
|
||||
elif not has_user:
|
||||
reason = "no_user_id"
|
||||
inject = False
|
||||
elif mode_name == "disabled":
|
||||
reason = "mode_disabled"
|
||||
inject = False
|
||||
elif mode_name == "tool":
|
||||
reason = "mode_tool"
|
||||
inject = False
|
||||
else:
|
||||
reason = None
|
||||
inject = True
|
||||
decision = decide_memory_injection(
|
||||
headers=headers,
|
||||
memory_handler_present=memory_handler is not None,
|
||||
memory_user_id_present=bool(memory_user_id),
|
||||
mode_name=mode_name,
|
||||
)
|
||||
|
||||
return cls(
|
||||
inject=inject,
|
||||
skip_reason=reason,
|
||||
bypass_header_set=bypass,
|
||||
memory_handler_present=has_handler,
|
||||
memory_user_id_present=has_user,
|
||||
mode_name=mode_name,
|
||||
inject=decision.inject,
|
||||
skip_reason=decision.skip_reason,
|
||||
bypass_header_set=decision.bypass_header_set,
|
||||
memory_handler_present=decision.memory_handler_present,
|
||||
memory_user_id_present=decision.memory_user_id_present,
|
||||
mode_name=decision.mode_name,
|
||||
)
|
||||
|
||||
def apply_to_tags(self, tags: dict[str, str]) -> None:
|
||||
|
|
@ -140,5 +127,4 @@ class MemoryDecision:
|
|||
same path the funnel already uses for ``client`` and
|
||||
``passthrough_reason``.
|
||||
"""
|
||||
if self.skip_reason is not None:
|
||||
tags["memory_skip_reason"] = self.skip_reason
|
||||
apply_memory_skip_reason(tags, self.skip_reason)
|
||||
|
|
|
|||
65
headroom/proxy/memory_decision_policy.py
Normal file
65
headroom/proxy/memory_decision_policy.py
Normal file
|
|
@ -0,0 +1,65 @@
|
|||
"""Pure memory-injection decision policy helpers."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from dataclasses import dataclass
|
||||
from typing import Any
|
||||
|
||||
from headroom.proxy.helpers import _headroom_bypass_enabled
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class MemoryInjectionDecision:
|
||||
"""Raw memory-injection decision before wrapping in public value types."""
|
||||
|
||||
inject: bool
|
||||
skip_reason: str | None
|
||||
bypass_header_set: bool
|
||||
memory_handler_present: bool
|
||||
memory_user_id_present: bool
|
||||
mode_name: str
|
||||
|
||||
|
||||
def decide_memory_injection(
|
||||
*,
|
||||
headers: Any,
|
||||
memory_handler_present: bool,
|
||||
memory_user_id_present: bool,
|
||||
mode_name: str,
|
||||
) -> MemoryInjectionDecision:
|
||||
"""Compute the canonical memory-injection decision."""
|
||||
bypass = _headroom_bypass_enabled(headers)
|
||||
|
||||
if bypass:
|
||||
reason: str | None = "bypass_header"
|
||||
inject = False
|
||||
elif not memory_handler_present:
|
||||
reason = "no_handler"
|
||||
inject = False
|
||||
elif not memory_user_id_present:
|
||||
reason = "no_user_id"
|
||||
inject = False
|
||||
elif mode_name == "disabled":
|
||||
reason = "mode_disabled"
|
||||
inject = False
|
||||
elif mode_name == "tool":
|
||||
reason = "mode_tool"
|
||||
inject = False
|
||||
else:
|
||||
reason = None
|
||||
inject = True
|
||||
|
||||
return MemoryInjectionDecision(
|
||||
inject=inject,
|
||||
skip_reason=reason,
|
||||
bypass_header_set=bypass,
|
||||
memory_handler_present=memory_handler_present,
|
||||
memory_user_id_present=memory_user_id_present,
|
||||
mode_name=mode_name,
|
||||
)
|
||||
|
||||
|
||||
def apply_memory_skip_reason(tags: dict[str, str], skip_reason: str | None) -> None:
|
||||
"""Stamp memory skip reason into tags when present."""
|
||||
if skip_reason is not None:
|
||||
tags["memory_skip_reason"] = skip_reason
|
||||
62
tests/test_memory_decision_policy.py
Normal file
62
tests/test_memory_decision_policy.py
Normal file
|
|
@ -0,0 +1,62 @@
|
|||
"""Tests for pure memory-injection decision policy helpers."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from headroom.proxy.memory_decision_policy import (
|
||||
apply_memory_skip_reason,
|
||||
decide_memory_injection,
|
||||
)
|
||||
|
||||
|
||||
def test_decide_memory_injection_allows_happy_path() -> None:
|
||||
decision = decide_memory_injection(
|
||||
headers={},
|
||||
memory_handler_present=True,
|
||||
memory_user_id_present=True,
|
||||
mode_name="auto_tail",
|
||||
)
|
||||
|
||||
assert decision.inject is True
|
||||
assert decision.skip_reason is None
|
||||
|
||||
|
||||
def test_decide_memory_injection_uses_canonical_precedence() -> None:
|
||||
decision = decide_memory_injection(
|
||||
headers={"x-headroom-bypass": "true"},
|
||||
memory_handler_present=False,
|
||||
memory_user_id_present=False,
|
||||
mode_name="disabled",
|
||||
)
|
||||
|
||||
assert decision.inject is False
|
||||
assert decision.skip_reason == "bypass_header"
|
||||
assert decision.bypass_header_set is True
|
||||
assert decision.memory_handler_present is False
|
||||
assert decision.memory_user_id_present is False
|
||||
|
||||
|
||||
def test_decide_memory_injection_reports_mode_reasons() -> None:
|
||||
disabled = decide_memory_injection(
|
||||
headers={},
|
||||
memory_handler_present=True,
|
||||
memory_user_id_present=True,
|
||||
mode_name="disabled",
|
||||
)
|
||||
tool = decide_memory_injection(
|
||||
headers={},
|
||||
memory_handler_present=True,
|
||||
memory_user_id_present=True,
|
||||
mode_name="tool",
|
||||
)
|
||||
|
||||
assert disabled.skip_reason == "mode_disabled"
|
||||
assert tool.skip_reason == "mode_tool"
|
||||
|
||||
|
||||
def test_apply_memory_skip_reason_stamps_only_when_skipping() -> None:
|
||||
tags: dict[str, str] = {}
|
||||
apply_memory_skip_reason(tags, None)
|
||||
assert tags == {}
|
||||
|
||||
apply_memory_skip_reason(tags, "no_user_id")
|
||||
assert tags == {"memory_skip_reason": "no_user_id"}
|
||||
Loading…
Add table
Add a link
Reference in a new issue