diff --git a/features/steps/tdd_budget_eviction_deletes_not_demotes_steps.py b/features/steps/tdd_budget_eviction_deletes_not_demotes_steps.py new file mode 100644 index 000000000..bd61a8701 --- /dev/null +++ b/features/steps/tdd_budget_eviction_deletes_not_demotes_steps.py @@ -0,0 +1,223 @@ +"""Step definitions for TDD Issue #1152 — budget eviction deletes instead of demotes. + +These steps exercise ``ContextTierService`` to verify that hot-tier fragments +evicted by budget enforcement (``_enforce_hot_budget``) are **demoted to the +warm tier** rather than permanently deleted. + +Bug #1152: ``_enforce_hot_budget()`` in ``TierRuntimeMixin`` does +``del self._hot[oldest_id]`` — the evicted fragment is destroyed. The +specification's downward tier lifecycle (§Plan Lifecycle ACMS Actions: +"Hot context archived to warm") requires demotion, not deletion. + +Similarly, ``evict_lru()`` in ``ContextTierService`` permanently deletes +fragments from the tier store without demoting them to the next lower tier. + +The assertions in these steps will **fail** until the bug is fixed, +proving the bug exists. The ``@tdd_expected_fail`` tag on the feature +inverts the result so CI passes. +""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta + +from behave import given, then, when +from behave.runner import Context + +from cleveragents.application.services.context_tiers import ContextTierService +from cleveragents.domain.models.acms.tiers import ( + ContextTier, + TierBudget, + TieredFragment, +) + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_eviction_fragment( + fragment_id: str, + tier: ContextTier, + token_count: int = 50, + content: str = "test content", + last_accessed: datetime | None = None, +) -> TieredFragment: + """Create a ``TieredFragment`` for eviction tests.""" + fragment: TieredFragment = TieredFragment( + fragment_id=fragment_id, + content=content, + tier=tier, + token_count=token_count, + project_name="test-project", + ) + if last_accessed is not None: + fragment.last_accessed = last_accessed + return fragment + + +# --------------------------------------------------------------------------- +# Scenario 1: Budget-evicted fragment demoted to warm (not deleted) +# --------------------------------------------------------------------------- + + +@given( + "a context tier service with a hot tier budget of {n:d} tokens for eviction test" +) +def step_eviction_service_small_budget(context: Context, n: int) -> None: + """Create a ContextTierService with a constrained hot tier budget.""" + service: ContextTierService = ContextTierService(settings=None) + service._budget = TierBudget( + max_tokens_hot=n, + max_decisions_warm=500, + max_decisions_cold=5000, + ) + context.eviction_service = service + context.eviction_budget_tokens = n + + +@given( + "the hot tier contains two 50-token fragments filling the budget for eviction test" +) +def step_eviction_fill_hot_tier(context: Context) -> None: + """Fill the hot tier with exactly two 50-token fragments (100 tokens).""" + service: ContextTierService = context.eviction_service + + # Fragment A: oldest (lower last_accessed) + frag_a: TieredFragment = _make_eviction_fragment( + fragment_id="frag-evict-oldest", + tier=ContextTier.HOT, + token_count=50, + content="oldest fragment content", + last_accessed=datetime.now(tz=UTC) - timedelta(minutes=10), + ) + service.store(frag_a) + + # Fragment B: newer + frag_b: TieredFragment = _make_eviction_fragment( + fragment_id="frag-evict-newer", + tier=ContextTier.HOT, + token_count=50, + content="newer fragment content", + last_accessed=datetime.now(tz=UTC) - timedelta(minutes=5), + ) + service.store(frag_b) + + context.eviction_oldest_id = "frag-evict-oldest" + context.eviction_newer_id = "frag-evict-newer" + + metrics = service.get_metrics() + assert metrics.hot_count == 2, ( + f"Expected 2 fragments in hot tier, got {metrics.hot_count}" + ) + + +@when("I store a third 50-token fragment in the hot tier triggering budget eviction") +def step_eviction_store_overflow(context: Context) -> None: + """Store a fragment that exceeds the hot tier budget, triggering eviction.""" + service: ContextTierService = context.eviction_service + overflow_frag: TieredFragment = _make_eviction_fragment( + fragment_id="frag-evict-overflow", + tier=ContextTier.HOT, + token_count=50, + content="overflow fragment content", + ) + service.store(overflow_frag) + + +@then("the evicted fragment should exist in the warm tier not be permanently deleted") +def step_eviction_check_warm_tier(context: Context) -> None: + """Assert the budget-evicted fragment was demoted to warm, not deleted. + + Bug #1152: ``_enforce_hot_budget()`` does ``del self._hot[oldest_id]`` + which permanently destroys the fragment. The correct behaviour is to + demote the fragment to the warm tier, preserving it in the lower tier + consistent with the specification's downward lifecycle flow. + """ + service: ContextTierService = context.eviction_service + oldest_id: str = context.eviction_oldest_id + + # The evicted fragment should NOT be in the hot tier + assert oldest_id not in service._hot, ( + f"Fragment '{oldest_id}' is still in the hot tier after budget " + f"eviction — budget enforcement did not evict it." + ) + + # The evicted fragment SHOULD be in the warm tier (demoted, not deleted) + assert oldest_id in service._warm, ( + f"Fragment '{oldest_id}' was evicted from the hot tier but is NOT " + f"in the warm tier. _enforce_hot_budget() permanently deletes " + f"evicted fragments via 'del self._hot[oldest_id]' instead of " + f"demoting them to the warm tier (bug #1152). The specification " + f"(§Plan Lifecycle ACMS Actions) describes a downward lifecycle: " + f"'Hot context archived to warm.' Evicted fragments should be " + f"demoted to warm, not destroyed." + ) + + # Verify the fragment's tier metadata is updated to WARM + demoted: TieredFragment = service._warm[oldest_id] + assert demoted.tier == ContextTier.WARM, ( + f"Fragment '{oldest_id}' is in the warm store but its tier " + f"metadata is {demoted.tier}, expected WARM." + ) + + +# --------------------------------------------------------------------------- +# Scenario 2: evict_lru also demotes to warm instead of deleting +# --------------------------------------------------------------------------- + + +@when("I call evict_lru on the hot tier to remove one fragment") +def step_eviction_call_evict_lru(context: Context) -> None: + """Call evict_lru(HOT, 1) to evict the oldest hot-tier fragment.""" + service: ContextTierService = context.eviction_service + evicted_ids: list[str] = service.evict_lru(ContextTier.HOT, count=1) + context.eviction_lru_evicted_ids = evicted_ids + + +@then( + "the LRU-evicted fragment should exist in the warm tier not be permanently deleted" +) +def step_eviction_lru_check_warm_tier(context: Context) -> None: + """Assert the LRU-evicted fragment was demoted to warm, not deleted. + + Bug #1152: ``evict_lru()`` does ``del store[fid]`` which permanently + destroys the fragment. The correct behaviour is to demote the + fragment to the next lower tier (hot → warm), preserving it + consistent with the specification's downward lifecycle flow. + """ + service: ContextTierService = context.eviction_service + evicted_ids: list[str] = context.eviction_lru_evicted_ids + + assert len(evicted_ids) == 1, f"Expected 1 evicted fragment, got {len(evicted_ids)}" + + evicted_id: str = evicted_ids[0] + + # The evicted fragment should be the oldest one + assert evicted_id == context.eviction_oldest_id, ( + f"Expected oldest fragment '{context.eviction_oldest_id}' to be " + f"evicted, but '{evicted_id}' was evicted instead." + ) + + # The evicted fragment should NOT be in the hot tier + assert evicted_id not in service._hot, ( + f"Fragment '{evicted_id}' is still in the hot tier after eviction." + ) + + # The evicted fragment SHOULD be in the warm tier (demoted, not deleted) + assert evicted_id in service._warm, ( + f"Fragment '{evicted_id}' was evicted from the hot tier but is NOT " + f"in the warm tier. evict_lru() permanently deletes evicted " + f"fragments via 'del store[fid]' instead of demoting them to the " + f"warm tier (bug #1152). The specification (§Plan Lifecycle ACMS " + f"Actions) describes a downward lifecycle: 'Hot context archived " + f"to warm.' Evicted fragments should be demoted to warm, not " + f"destroyed." + ) + + # Verify the fragment's tier metadata is updated to WARM + demoted: TieredFragment = service._warm[evicted_id] + assert demoted.tier == ContextTier.WARM, ( + f"Fragment '{evicted_id}' is in the warm store but its tier " + f"metadata is {demoted.tier}, expected WARM." + ) diff --git a/features/tdd_budget_eviction_deletes_not_demotes.feature b/features/tdd_budget_eviction_deletes_not_demotes.feature new file mode 100644 index 000000000..46c2ac720 --- /dev/null +++ b/features/tdd_budget_eviction_deletes_not_demotes.feature @@ -0,0 +1,28 @@ +@tdd_expected_fail @tdd_issue @tdd_issue_1152 @mock_only +Feature: TDD Issue #1152 — budget eviction permanently deletes hot-tier fragments instead of demoting to warm + As a developer + I want to verify that budget-evicted hot-tier fragments are demoted to the warm tier + So that context data follows the spec's downward tier lifecycle rather than being destroyed + + # This test captures bug #1152. When the hot-tier token budget is + # exceeded, _enforce_hot_budget() permanently deletes evicted fragments + # via ``del self._hot[oldest_id]`` instead of demoting them to the warm + # tier. The specification (§Plan Lifecycle ACMS Actions) describes a + # downward lifecycle: "Hot context archived to warm. Warm context ages + # to cold based on retention policy." + # + # The @tdd_expected_fail tag inverts the result so CI passes while the + # bug is still present. When bug #1152 is fixed, the @tdd_expected_fail + # tag must be removed so the test runs normally. + + Scenario: Budget-evicted hot-tier fragment is demoted to warm tier instead of deleted + Given a context tier service with a hot tier budget of 100 tokens for eviction test + And the hot tier contains two 50-token fragments filling the budget for eviction test + When I store a third 50-token fragment in the hot tier triggering budget eviction + Then the evicted fragment should exist in the warm tier not be permanently deleted + + Scenario: Budget eviction via evict_lru also demotes to warm tier instead of deleting + Given a context tier service with a hot tier budget of 100 tokens for eviction test + And the hot tier contains two 50-token fragments filling the budget for eviction test + When I call evict_lru on the hot tier to remove one fragment + Then the LRU-evicted fragment should exist in the warm tier not be permanently deleted diff --git a/robot/helper_tdd_budget_eviction_deletes_not_demotes.py b/robot/helper_tdd_budget_eviction_deletes_not_demotes.py new file mode 100644 index 000000000..11948a074 --- /dev/null +++ b/robot/helper_tdd_budget_eviction_deletes_not_demotes.py @@ -0,0 +1,226 @@ +"""Helper script for tdd_budget_eviction_deletes_not_demotes.robot smoke tests. + +Each subcommand exercises ``ContextTierService`` to reproduce bug #1152. +The helper reports the **real** outcome: it exits 0 and prints the sentinel +when the expected behaviour is observed (bug fixed), and exits 1 when the +bug is still present. The ``tdd_expected_fail_listener`` on the Robot side +handles pass/fail inversion while the bug remains open. + +Bug #1152: ``_enforce_hot_budget()`` permanently deletes hot-tier fragments +via ``del self._hot[oldest_id]`` instead of demoting them to the warm tier. +Similarly, ``evict_lru()`` permanently deletes via ``del store[fid]``. +""" + +from __future__ import annotations + +import sys +from collections.abc import Callable +from datetime import UTC, datetime, timedelta +from pathlib import Path +from typing import NoReturn + +# Ensure local source tree is importable. +_ROOT: Path = Path(__file__).resolve().parents[1] +_SRC: str = str(_ROOT / "src") +_ROBOT: str = str(_ROOT / "robot") +for _p in (_SRC, _ROBOT): + if _p not in sys.path: + sys.path.insert(0, _p) + +from cleveragents.application.services.context_tiers import ( # noqa: E402 + ContextTierService, +) +from cleveragents.domain.models.acms.tiers import ( # noqa: E402 + ContextTier, + TierBudget, + TieredFragment, +) + + +def _fail(msg: str) -> NoReturn: + """Print error message to stderr and exit with code 1.""" + print(msg, file=sys.stderr) + sys.exit(1) + + +def _make_fragment( + fragment_id: str, + tier: ContextTier, + token_count: int = 50, + content: str = "test content", + last_accessed: datetime | None = None, +) -> TieredFragment: + """Create a ``TieredFragment`` with the given properties.""" + fragment: TieredFragment = TieredFragment( + fragment_id=fragment_id, + content=content, + tier=tier, + token_count=token_count, + project_name="test-project", + ) + if last_accessed is not None: + fragment.last_accessed = last_accessed + return fragment + + +# --------------------------------------------------------------------------- +# Subcommands +# --------------------------------------------------------------------------- + + +def _budget_eviction_demotes() -> None: + """Verify that budget-evicted hot-tier fragment is demoted to warm. + + Bug #1152: ``_enforce_hot_budget()`` does + ``del self._hot[oldest_id]`` which permanently destroys the + fragment. The correct behaviour is to demote to the warm tier. + """ + service: ContextTierService = ContextTierService(settings=None) + service._budget = TierBudget( + max_tokens_hot=100, + max_decisions_warm=500, + max_decisions_cold=5000, + ) + + # Fill hot tier to exactly 100 tokens (2 x 50-token fragments) + frag_a: TieredFragment = _make_fragment( + fragment_id="frag-evict-oldest", + tier=ContextTier.HOT, + token_count=50, + content="oldest fragment", + last_accessed=datetime.now(tz=UTC) - timedelta(minutes=10), + ) + service.store(frag_a) + + frag_b: TieredFragment = _make_fragment( + fragment_id="frag-evict-newer", + tier=ContextTier.HOT, + token_count=50, + content="newer fragment", + last_accessed=datetime.now(tz=UTC) - timedelta(minutes=5), + ) + service.store(frag_b) + + # Store one more fragment to trigger budget eviction + overflow: TieredFragment = _make_fragment( + fragment_id="frag-evict-overflow", + tier=ContextTier.HOT, + token_count=50, + content="overflow fragment", + ) + service.store(overflow) + + # Check: the oldest fragment should NOT be in hot tier + if "frag-evict-oldest" in service._hot: + _fail( + "Fragment 'frag-evict-oldest' is still in hot tier after " + "budget overflow — budget enforcement did not evict it." + ) + + # Check: the oldest fragment SHOULD be in warm tier (demoted, not deleted) + if "frag-evict-oldest" not in service._warm: + _fail( + "Fragment 'frag-evict-oldest' was evicted from hot tier but " + "is NOT in the warm tier. _enforce_hot_budget() permanently " + "deletes evicted fragments instead of demoting them to warm " + "(bug #1152)." + ) + + # Verify tier metadata + demoted: TieredFragment = service._warm["frag-evict-oldest"] + if demoted.tier != ContextTier.WARM: + _fail( + f"Fragment is in warm store but tier metadata is " + f"{demoted.tier}, expected WARM." + ) + + print("tdd-budget-eviction-demotes-ok") + + +def _evict_lru_demotes() -> None: + """Verify that evict_lru() demotes to warm instead of deleting. + + Bug #1152: ``evict_lru()`` does ``del store[fid]`` which + permanently destroys the fragment. The correct behaviour is to + demote to the next lower tier. + """ + service: ContextTierService = ContextTierService(settings=None) + service._budget = TierBudget( + max_tokens_hot=100, + max_decisions_warm=500, + max_decisions_cold=5000, + ) + + # Store two fragments in hot tier + frag_a: TieredFragment = _make_fragment( + fragment_id="frag-lru-oldest", + tier=ContextTier.HOT, + token_count=50, + content="oldest fragment", + last_accessed=datetime.now(tz=UTC) - timedelta(minutes=10), + ) + service.store(frag_a) + + frag_b: TieredFragment = _make_fragment( + fragment_id="frag-lru-newer", + tier=ContextTier.HOT, + token_count=50, + content="newer fragment", + last_accessed=datetime.now(tz=UTC) - timedelta(minutes=5), + ) + service.store(frag_b) + + # Evict the LRU fragment + evicted_ids: list[str] = service.evict_lru(ContextTier.HOT, count=1) + + if len(evicted_ids) != 1: + _fail(f"Expected 1 evicted fragment, got {len(evicted_ids)}") + + evicted_id: str = evicted_ids[0] + if evicted_id != "frag-lru-oldest": + _fail( + f"Expected oldest fragment to be evicted, but '{evicted_id}' was evicted." + ) + + # Check: the evicted fragment should NOT be in hot tier + if evicted_id in service._hot: + _fail(f"Fragment '{evicted_id}' is still in hot tier after eviction.") + + # Check: the evicted fragment SHOULD be in warm tier (demoted, not deleted) + if evicted_id not in service._warm: + _fail( + f"Fragment '{evicted_id}' was evicted from hot tier but is " + f"NOT in the warm tier. evict_lru() permanently deletes " + f"evicted fragments instead of demoting them to warm " + f"(bug #1152)." + ) + + # Verify tier metadata + demoted: TieredFragment = service._warm[evicted_id] + if demoted.tier != ContextTier.WARM: + _fail( + f"Fragment is in warm store but tier metadata is " + f"{demoted.tier}, expected WARM." + ) + + print("tdd-evict-lru-demotes-ok") + + +# --------------------------------------------------------------------------- +# Dispatcher +# --------------------------------------------------------------------------- + +_COMMANDS: dict[str, Callable[[], None]] = { + "budget-eviction-demotes": _budget_eviction_demotes, + "evict-lru-demotes": _evict_lru_demotes, +} + +if __name__ == "__main__": + if len(sys.argv) < 2 or sys.argv[1] not in _COMMANDS: + print( + f"Usage: {sys.argv[0]} <{'|'.join(_COMMANDS)}>", + file=sys.stderr, + ) + sys.exit(1) + cmd: Callable[[], None] = _COMMANDS[sys.argv[1]] + cmd() diff --git a/robot/tdd_budget_eviction_deletes_not_demotes.robot b/robot/tdd_budget_eviction_deletes_not_demotes.robot new file mode 100644 index 000000000..8d2eb6d17 --- /dev/null +++ b/robot/tdd_budget_eviction_deletes_not_demotes.robot @@ -0,0 +1,34 @@ +*** Settings *** +Documentation TDD Issue #1152 — budget eviction permanently deletes hot-tier fragments +... instead of demoting to warm tier. Integration smoke tests verifying that +... ContextTierService._enforce_hot_budget() and evict_lru() demote evicted +... fragments to the warm tier rather than permanently deleting them. +... Tests are tagged tdd_expected_fail so CI passes via result inversion. +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_tdd_budget_eviction_deletes_not_demotes.py + +*** Test Cases *** +TDD Budget Eviction Demotes To Warm Not Deletes + [Documentation] Verify that when _enforce_hot_budget() evicts a fragment due to + ... token budget overflow, the evicted fragment is demoted to the warm + ... tier rather than permanently deleted. + [Tags] tdd_expected_fail tdd_issue tdd_issue_1152 + ${result}= Run Process ${PYTHON} ${HELPER} budget-eviction-demotes cwd=${WORKSPACE} timeout=30s + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-budget-eviction-demotes-ok + +TDD Evict LRU Demotes To Warm Not Deletes + [Documentation] Verify that evict_lru(HOT, 1) demotes the evicted fragment to the + ... warm tier rather than permanently deleting it. + [Tags] tdd_expected_fail tdd_issue tdd_issue_1152 + ${result}= Run Process ${PYTHON} ${HELPER} evict-lru-demotes cwd=${WORKSPACE} timeout=30s + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-evict-lru-demotes-ok