mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(learn): let an emptied category drop its heading
A category whose last pattern expires stops producing a recommendation entirely, so its heading never reaches the same-section merge and _merge_recommendations carried it forward untouched -- pinning the stale bullets forever, which is the lifecycle bug the item-level merge is meant to fix. Route the run's active_item_ids to the carry path as well: a carried section's id-tagged bullets are kept only while the run still claims them, and a section left with none is dropped along with its heading. Sections holding no tagged bullets, and runs that publish no lifecycle signal at all, keep the previous carry-everything behaviour. Change-Id: Ib9cdb389dc203b8b2bdec480409514ea178ebf0c
This commit is contained in:
parent
136dd9e37d
commit
d89568e524
2 changed files with 296 additions and 1 deletions
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
218
tests/test_learn/test_section_lifecycle.py
Normal file
218
tests/test_learn/test_section_lifecycle.py
Normal file
|
|
@ -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:learn:start -->", "## Headroom Learned Patterns", ""]
|
||||
for heading, body in sections:
|
||||
parts += [f"### {heading}", body, ""]
|
||||
parts.append("<!-- headroom:learn:end -->")
|
||||
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 <!-- headroom:pattern-id:handlers -->",
|
||||
),
|
||||
(
|
||||
"Learned: preference",
|
||||
"- Keep local reviews <!-- headroom:pattern-id:reviews -->",
|
||||
),
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
recommendation = _rec(
|
||||
"Learned: preference",
|
||||
"- Keep local reviews <!-- headroom:pattern-id: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 <!-- headroom:pattern-id:handlers -->\n"
|
||||
"- Build against the vendored SDK <!-- headroom:pattern-id:vendored -->",
|
||||
),
|
||||
(
|
||||
"Learned: preference",
|
||||
"- Keep local reviews <!-- headroom:pattern-id:reviews -->",
|
||||
),
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
recommendation = _rec(
|
||||
"Learned: preference",
|
||||
"- Keep local reviews <!-- headroom:pattern-id: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 <!-- headroom:pattern-id:reviews -->",
|
||||
),
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
recommendation = _rec(
|
||||
"Learned: preference",
|
||||
"- Keep local reviews <!-- headroom:pattern-id: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 <!-- headroom:pattern-id: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
|
||||
Loading…
Add table
Add a link
Reference in a new issue