diff --git a/headroom/learn/writer.py b/headroom/learn/writer.py index 540fca945..f71671dd6 100644 --- a/headroom/learn/writer.py +++ b/headroom/learn/writer.py @@ -170,6 +170,64 @@ def _merge_markdown_items( return "\n".join(merged) +def _authoritative_item_ids( + recommendations: list[Recommendation], +) -> frozenset[str] | None: + """Union every lifecycle signal the current run carries, or None. + + A section the new run did not re-emit cannot be judged by its own + recommendation — that recommendation is exactly what is missing. The + sets the run *does* carry stand in for it: a producer publishes one set + per run covering all of its live items, so an id absent from every set + in the run is one no producer still claims. Returns ``None`` when no + recommendation carries a signal, which keeps the historical + carry-everything behaviour for runs that cannot speak to lifecycle. + """ + signals = [r.active_item_ids for r in recommendations if r.active_item_ids is not None] + if not signals: + return None + return frozenset().union(*signals) + + +def _prune_carried_section( + content: str, + active_item_ids: frozenset[str], +) -> str | None: + """Drop expired id-tagged bullets from a section the new run did not re-emit. + + This is the deletion path for a heading whose last item expired: the + producer stops emitting the section entirely, so the same-section merge + never runs and the heading would otherwise be carried forward forever. + + Only bullets carrying a `headroom:pattern-id` are removable — they come + from a lifecycle-tracked producer, so their absence from + ``active_item_ids`` means that producer dropped them. Untagged bullets + are kept: unlike the same-section merge, where a still-active legacy + item is re-emitted with an id by the same run and collapses by visible + text, nothing in this path would bring an untagged bullet back, so + deleting it would discard content on no evidence. + + Returns the pruned content, or ``None`` when the section carries no + tagged bullets at all and is therefore not a tracked section to prune. + """ + lines = [line.strip() for line in content.splitlines() if line.strip()] + if not lines or any(not line.startswith("- ") for line in lines): + return None + kept: list[str] = [] + saw_tracked_item = False + for line in lines: + id_match = _PATTERN_ID_PATTERN.search(line) + if id_match is None: + kept.append(line) + continue + saw_tracked_item = True + if id_match.group(1) in active_item_ids: + kept.append(line) + if not saw_tracked_item: + return None + return "\n".join(kept) + + def extract_marker_block(file_content: str) -> str | None: """Return the raw text of the headroom:learn marker block, or None. @@ -233,6 +291,12 @@ def _merge_recommendations( Recommendations that opt into ``preserve_prior_items`` are merged at the item level instead, bounded by their ``active_item_ids`` lifecycle signal so prior items can still expire out of the file. + + That signal also reaches the carried-forward sections. A category whose + last item expires stops producing a recommendation at all, so its + heading never enters the same-section merge; without pruning the carry + path too, those bullets would be pinned in the file forever. Sections + holding no id-tagged items are carried untouched, as before. """ if not file_path.exists(): return new_recommendations @@ -254,7 +318,20 @@ def _merge_recommendations( merged_new.append(recommendation) new_sections = {r.section for r in merged_new} - carried = [p for p in prior if p.section not in new_sections] + run_active_item_ids = _authoritative_item_ids(merged_new) + carried: list[Recommendation] = [] + for prior_recommendation in prior: + if prior_recommendation.section in new_sections: + continue + if run_active_item_ids is not None: + pruned = _prune_carried_section(prior_recommendation.content, run_active_item_ids) + if pruned is not None: + if not pruned: + # Every tracked item under this heading is gone; the + # heading goes with them rather than outliving them. + continue + prior_recommendation = replace(prior_recommendation, content=pruned) + carried.append(prior_recommendation) return merged_new + carried diff --git a/tests/test_learn/test_section_lifecycle.py b/tests/test_learn/test_section_lifecycle.py new file mode 100644 index 000000000..84d45738c --- /dev/null +++ b/tests/test_learn/test_section_lifecycle.py @@ -0,0 +1,218 @@ +"""Removal invariant for merged sections — the deletion half of #2293. + +``test_writer.py`` covers preservation inside a section that the new run +re-emits. The cases here cover the other transition: a section the new run +does not emit at all, which never reaches the same-section merge and so has +to be pruned on the writer's carry-forward path instead. Without that, the +last item of a category can expire and its heading still be pinned in the +file forever. + +The learner-level test lives here rather than in ``test_traffic_learner.py`` +because it asserts on writer behaviour; it drives the real, unmodified +``_patterns_to_recommendations`` to get there. +""" + +from headroom.learn.models import Recommendation, RecommendationTarget +from headroom.learn.writer import _merge_into_file +from headroom.memory.traffic_learner import ( + ExtractedPattern, + PatternCategory, + _patterns_to_recommendations, +) + + +def _rec(section: str, content: str) -> Recommendation: + return Recommendation( + target=RecommendationTarget.CONTEXT_FILE, + section=section, + content=content, + confidence=0.8, + evidence_count=5, + ) + + +def _block(*sections: tuple[str, str]) -> str: + parts = ["", "## Headroom Learned Patterns", ""] + for heading, body in sections: + parts += [f"### {heading}", body, ""] + parts.append("") + return "\n".join(parts) + "\n" + + +class TestCarriedSectionLifecycle: + """The new run's lifecycle signal also governs the sections it omits.""" + + def test_absent_tracked_section_is_removed_once_its_items_expire(self, tmp_path): + """A heading outlives its items unless the carry path is pruned too. + + When a category loses its last pattern the learner stops emitting a + recommendation for that heading at all, so the same-section merge + never runs on it. The signal published by the rest of the run has to + reach the carry path, or the section is pinned forever. + """ + context_file = tmp_path / "AGENTS.md" + context_file.write_text( + _block( + ( + "Learned: architecture", + "- Handlers live under proxy/handlers ", + ), + ( + "Learned: preference", + "- Keep local reviews ", + ), + ), + encoding="utf-8", + ) + recommendation = _rec( + "Learned: preference", + "- Keep local reviews ", + ) + recommendation.preserve_prior_items = True + recommendation.active_item_ids = frozenset({"reviews"}) + + final = _merge_into_file(context_file, [recommendation]) + + assert "### Learned: architecture" not in final + assert "Handlers live under proxy/handlers" not in final + assert final.count("Keep local reviews") == 1 + + def test_absent_tracked_section_keeps_its_still_active_items(self, tmp_path): + """Pruning a carried section is per item, not all-or-nothing. + + A category can be missing from a render because of batching while + still holding live items, so only the ids the run no longer claims + may go. + """ + context_file = tmp_path / "AGENTS.md" + context_file.write_text( + _block( + ( + "Learned: architecture", + "- Handlers live under proxy/handlers \n" + "- Build against the vendored SDK ", + ), + ( + "Learned: preference", + "- Keep local reviews ", + ), + ), + encoding="utf-8", + ) + recommendation = _rec( + "Learned: preference", + "- Keep local reviews ", + ) + recommendation.preserve_prior_items = True + recommendation.active_item_ids = frozenset({"reviews", "handlers"}) + + final = _merge_into_file(context_file, [recommendation]) + + assert "### Learned: architecture" in final + assert "Handlers live under proxy/handlers" in final + assert "Build against the vendored SDK" not in final + + def test_absent_untracked_sections_are_carried_untouched(self, tmp_path): + """Only id-tagged sections are ours to delete. + + Hand-written headings and prose bodies carry no lifecycle signal, + and — unlike the same-section merge, where a still-active legacy + item is re-emitted with an id by the same run — nothing on this path + would bring them back, so they must survive the prune. + """ + context_file = tmp_path / "AGENTS.md" + context_file.write_text( + _block( + ("Hand-written notes", "- Deploy from the release branch"), + ("Project shape", "Prose, deliberately not a markdown list."), + ( + "Learned: preference", + "- Keep local reviews ", + ), + ), + encoding="utf-8", + ) + recommendation = _rec( + "Learned: preference", + "- Keep local reviews ", + ) + recommendation.preserve_prior_items = True + recommendation.active_item_ids = frozenset({"reviews"}) + + final = _merge_into_file(context_file, [recommendation]) + + assert "Deploy from the release branch" in final + assert "Prose, deliberately not a markdown list." in final + + def test_absent_tracked_section_survives_a_run_without_a_lifecycle_signal(self, tmp_path): + """No signal, no deletion. + + A run whose recommendations carry no ``active_item_ids`` cannot + speak to what is still alive, so the carry path stays conservative + rather than reading silence as expiry. + """ + context_file = tmp_path / "AGENTS.md" + context_file.write_text( + _block( + ( + "Learned: architecture", + "- Handlers live under proxy/handlers ", + ), + ("Learned: error recovery", "- Search before reading a guessed path"), + ), + encoding="utf-8", + ) + recommendation = _rec( + "Learned: error recovery", + "- Search before reading a guessed path", + ) + assert recommendation.active_item_ids is None + + final = _merge_into_file(context_file, [recommendation]) + + assert "### Learned: architecture" in final + assert "Handlers live under proxy/handlers" in final + + +class TestTrafficLearnerCategoryLifecycle: + """End-to-end through the real learner rendering.""" + + def test_category_losing_its_last_pattern_drops_its_heading(self, tmp_path): + """An emptied category leaves the file entirely. + + This is the transition the per-item merge cannot see. Once + ``architecture`` holds nothing, ``_patterns_to_recommendations`` + emits no recommendation for that heading, so the removal has to + happen on the writer's carry-forward path instead. The existing + end-to-end case keeps a second item in the same category, so it + never reaches this transition. + """ + keep = ExtractedPattern( + category=PatternCategory.PREFERENCE, + content="User prefers terse output", + importance=0.8, + evidence_count=3, + ) + sole_architecture_pattern = ExtractedPattern( + category=PatternCategory.ARCHITECTURE, + content="Handlers live under headroom/proxy/handlers", + importance=0.8, + evidence_count=3, + ) + context_file = tmp_path / "AGENTS.md" + context_file.write_text( + _merge_into_file( + context_file, + _patterns_to_recommendations([keep, sole_architecture_pattern]), + ), + encoding="utf-8", + ) + assert "### Learned: architecture" in context_file.read_text() + + # Next render: architecture has no live pattern left, so the whole + # category stops being rendered rather than rendering fewer bullets. + final = _merge_into_file(context_file, _patterns_to_recommendations([keep])) + + assert "User prefers terse output" in final + assert "### Learned: architecture" not in final + assert "Handlers live under headroom/proxy/handlers" not in final