From 3021f563fe89d987616ebbe5bf51862450fddecf Mon Sep 17 00:00:00 2001 From: Garm Date: Fri, 24 Apr 2026 12:28:47 +0200 Subject: [PATCH 1/2] fix(learn): refresh context-file timestamp when LLM returns no new recommendations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When `headroom learn` produced zero recommendations, the writers short- circuited via `if context_recs:` / `if memory_recs:` / `if not recommendations:`, so a successful run that had nothing new to report left `CLAUDE.md` / `MEMORY.md` / `AGENTS.md` / `GEMINI.md` completely untouched — including the `*Auto-generated by headroom learn on YYYY-MM-DD*` timestamp. Users had no visible signal that the run actually happened. `_merge_into_file` now returns `str | None` — `None` only when there are no new recommendations AND no existing marker block to refresh. The three writers drop their leading `if ...recs:` guards and skip based on the `None` return instead. When a marker block already exists, the block's timestamp and any carried-forward prior sections are always re-emitted with today's date. When no block exists and no recs are produced, nothing is written (no empty-block creation). Adds 11 unit tests covering the four writer paths and the direct `_merge_into_file` contract. --- CHANGELOG.md | 12 +++ headroom/learn/writer.py | 63 ++++++----- tests/test_learn/test_writer.py | 181 ++++++++++++++++++++++++++++++++ 3 files changed, 231 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a06de616..a98a0bb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed +- **`headroom learn` now refreshes context files even when the LLM returns + zero new recommendations** — previously the writers short-circuited on + empty results, so a successful run with nothing new to report left + `CLAUDE.md` / `MEMORY.md` / `AGENTS.md` / `GEMINI.md` completely + untouched, including the `*Auto-generated by headroom learn on + YYYY-MM-DD*` timestamp. Users had no way to tell from the file whether + the run actually happened. The writers now always rebuild the + `` block when one already exists in the target + file, refreshing the timestamp and re-emitting the carried-forward + sections. When no block exists and no recommendations are produced, + nothing is written (no empty-block creation). `_merge_into_file` now + returns `Optional[str]` to signal the no-op case cleanly. - **`headroom unwrap codex` now actually undoes `headroom wrap codex`** — previously there was no `unwrap codex` subcommand at all, so the injected `model_provider = "headroom"` / `[model_providers.headroom]` block stayed diff --git a/headroom/learn/writer.py b/headroom/learn/writer.py index 8d2168ab..c46ed136 100644 --- a/headroom/learn/writer.py +++ b/headroom/learn/writer.py @@ -161,14 +161,29 @@ def _merge_recommendations( return list(new_recommendations) + carried -def _merge_into_file(file_path: Path, new_recommendations: list[Recommendation]) -> str: - """Merge new recommendations with any existing marker block and rebuild the file.""" +def _merge_into_file( + file_path: Path, new_recommendations: list[Recommendation] +) -> str | None: + """Merge new recommendations with any existing marker block and rebuild the file. + + Returns ``None`` when there is nothing to write: no new recommendations AND + no existing marker block to refresh. Otherwise returns the full new file + contents. + + When the file already contains a marker block, the timestamp and any + carried-forward prior recommendations are always refreshed — even if the + current run produced zero new recommendations — so users can see that + ``headroom learn`` ran successfully. + """ + existing = file_path.read_text() if file_path.exists() else "" + has_block = _MARKER_START in existing + if not new_recommendations and not has_block: + return None merged = _merge_recommendations(file_path, new_recommendations) section = _build_section(merged) - if file_path.exists(): - existing = file_path.read_text() - if _MARKER_START in existing: - return _MARKER_PATTERN.sub(section, existing) + if has_block: + return _MARKER_PATTERN.sub(section, existing) + if existing: return existing.rstrip() + "\n\n" + section + "\n" return section + "\n" @@ -193,17 +208,17 @@ def write( context_recs = [r for r in recommendations if r.target == RecommendationTarget.CONTEXT_FILE] memory_recs = [r for r in recommendations if r.target == RecommendationTarget.MEMORY_FILE] - if context_recs: - claude_md_path = self._resolve_context_path(project) - full_content = _merge_into_file(claude_md_path, context_recs) + claude_md_path = self._resolve_context_path(project) + full_content = _merge_into_file(claude_md_path, context_recs) + if full_content is not None: result.add(claude_md_path, full_content) if not dry_run: claude_md_path.parent.mkdir(parents=True, exist_ok=True) claude_md_path.write_text(full_content) - if memory_recs: - memory_path = self._resolve_memory_path(project) - full_content = _merge_into_file(memory_path, memory_recs) + memory_path = self._resolve_memory_path(project) + full_content = _merge_into_file(memory_path, memory_recs) + if full_content is not None: result.add(memory_path, full_content) if not dry_run: memory_path.parent.mkdir(parents=True, exist_ok=True) @@ -246,17 +261,17 @@ def write( context_recs = [r for r in recommendations if r.target == RecommendationTarget.CONTEXT_FILE] memory_recs = [r for r in recommendations if r.target == RecommendationTarget.MEMORY_FILE] - if context_recs: - agents_md = project.context_file or (project.project_path / "AGENTS.md") - full_content = _merge_into_file(agents_md, context_recs) + agents_md = project.context_file or (project.project_path / "AGENTS.md") + full_content = _merge_into_file(agents_md, context_recs) + if full_content is not None: result.add(agents_md, full_content) if not dry_run: agents_md.parent.mkdir(parents=True, exist_ok=True) agents_md.write_text(full_content) - if memory_recs: - instructions_md = project.memory_file or (project.data_path.parent / "instructions.md") - full_content = _merge_into_file(instructions_md, memory_recs) + instructions_md = project.memory_file or (project.data_path.parent / "instructions.md") + full_content = _merge_into_file(instructions_md, memory_recs) + if full_content is not None: result.add(instructions_md, full_content) if not dry_run: instructions_md.parent.mkdir(parents=True, exist_ok=True) @@ -282,14 +297,12 @@ def write( result = WriteResult() result.dry_run = dry_run - if not recommendations: - return result - gemini_md = project.context_file or (project.project_path / "GEMINI.md") full_content = _merge_into_file(gemini_md, recommendations) - result.add(gemini_md, full_content) - if not dry_run: - gemini_md.parent.mkdir(parents=True, exist_ok=True) - gemini_md.write_text(full_content) + if full_content is not None: + result.add(gemini_md, full_content) + if not dry_run: + gemini_md.parent.mkdir(parents=True, exist_ok=True) + gemini_md.write_text(full_content) return result diff --git a/tests/test_learn/test_writer.py b/tests/test_learn/test_writer.py index 4b2d6d21..4555fc66 100644 --- a/tests/test_learn/test_writer.py +++ b/tests/test_learn/test_writer.py @@ -7,6 +7,9 @@ _MARKER_END, _MARKER_START, ClaudeCodeWriter, + CodexWriter, + GeminiWriter, + _merge_into_file, _parse_prior_recommendations, extract_marker_block, ) @@ -280,3 +283,181 @@ def test_returns_empty_block_when_markers_are_adjacent(self): block = extract_marker_block(content) assert block is not None assert block == f"{_MARKER_START}\n{_MARKER_END}" + + +class TestMergeIntoFile: + """Direct coverage for _merge_into_file — the None-return contract.""" + + def test_no_recs_and_no_file_returns_none(self, tmp_path): + """Empty recs against a non-existent file yields None (nothing to write).""" + assert _merge_into_file(tmp_path / "missing.md", []) is None + + def test_no_recs_and_file_without_block_returns_none(self, tmp_path): + """Empty recs against a file with no marker block yields None.""" + f = tmp_path / "CLAUDE.md" + f.write_text("# My Project\n\nJust regular content.\n") + assert _merge_into_file(f, []) is None + + def test_no_recs_and_existing_block_refreshes_timestamp(self, tmp_path): + """Empty recs against a file with a marker block refreshes the block's timestamp.""" + f = tmp_path / "CLAUDE.md" + f.write_text( + f"{_MARKER_START}\n" + "## Headroom Learned Patterns\n" + "*Auto-generated by `headroom learn` on 2020-01-01 — do not edit manually*\n\n" + "### Carried\n" + "- prior content\n\n" + f"{_MARKER_END}\n" + ) + result = _merge_into_file(f, []) + assert result is not None + # The stale 2020 date is gone; carried section is preserved. + assert "2020-01-01" not in result + assert "### Carried" in result + assert "- prior content" in result + # Still exactly one marker pair. + assert result.count(_MARKER_START) == 1 + assert result.count(_MARKER_END) == 1 + + +class TestClaudeCodeWriterTimestampRefresh: + """ClaudeCodeWriter must refresh existing blocks even with zero new recs.""" + + def test_empty_recs_refreshes_existing_claude_md_block(self, tmp_path): + """A run that produced no recs still refreshes CLAUDE.md's timestamp.""" + proj = _project(tmp_path) + claude_md = proj.project_path / "CLAUDE.md" + claude_md.write_text( + f"# My Project\n\n{_MARKER_START}\n" + "## Headroom Learned Patterns\n" + "*Auto-generated by `headroom learn` on 2020-01-01 — do not edit manually*\n\n" + "### Large Files\n" + "- App.tsx is huge\n\n" + f"{_MARKER_END}\n" + ) + + writer = ClaudeCodeWriter() + result = writer.write([], proj, dry_run=False) + + assert claude_md in result.files_written + content = claude_md.read_text() + # Timestamp refreshed. + assert "2020-01-01" not in content + # Prior content carried forward intact. + assert "My Project" in content + assert "### Large Files" in content + assert "App.tsx is huge" in content + + def test_empty_recs_refreshes_existing_memory_md_block(self, tmp_path): + """A run that produced no recs still refreshes MEMORY.md's timestamp.""" + proj = _project(tmp_path) + memory_md = proj.data_path / "memory" / "MEMORY.md" + memory_md.write_text( + f"{_MARKER_START}\n" + "## Headroom Learned Patterns\n" + "*Auto-generated by `headroom learn` on 2020-01-01 — do not edit manually*\n\n" + "### Prefs\n" + "- prior pref\n\n" + f"{_MARKER_END}\n" + ) + + writer = ClaudeCodeWriter() + result = writer.write([], proj, dry_run=False) + + assert memory_md in result.files_written + content = memory_md.read_text() + assert "2020-01-01" not in content + assert "### Prefs" in content + assert "- prior pref" in content + + def test_empty_recs_and_no_prior_files_writes_nothing(self, tmp_path): + """With no existing files and no recs, neither CLAUDE.md nor MEMORY.md is created.""" + proj = _project(tmp_path) + writer = ClaudeCodeWriter() + + result = writer.write([], proj, dry_run=False) + + assert result.files_written == [] + assert not (proj.project_path / "CLAUDE.md").exists() + assert not (proj.data_path / "memory" / "MEMORY.md").exists() + + def test_empty_recs_and_file_without_block_leaves_it_alone(self, tmp_path): + """A CLAUDE.md with no marker block is not touched when recs are empty.""" + proj = _project(tmp_path) + claude_md = proj.project_path / "CLAUDE.md" + original = "# My Project\n\nHand-written instructions only.\n" + claude_md.write_text(original) + + writer = ClaudeCodeWriter() + result = writer.write([], proj, dry_run=False) + + assert claude_md not in result.files_written + assert claude_md.read_text() == original + + +class TestCodexWriterTimestampRefresh: + """CodexWriter must refresh existing blocks even with zero new recs.""" + + def test_empty_recs_refreshes_existing_agents_md_block(self, tmp_path): + proj = _project(tmp_path) + agents_md = proj.project_path / "AGENTS.md" + agents_md.write_text( + f"{_MARKER_START}\n" + "## Headroom Learned Patterns\n" + "*Auto-generated by `headroom learn` on 2020-01-01 — do not edit manually*\n\n" + "### Env\n" + "- prior env\n\n" + f"{_MARKER_END}\n" + ) + + writer = CodexWriter() + result = writer.write([], proj, dry_run=False) + + assert agents_md in result.files_written + content = agents_md.read_text() + assert "2020-01-01" not in content + assert "### Env" in content + assert "prior env" in content + + def test_empty_recs_and_no_prior_files_writes_nothing(self, tmp_path): + proj = _project(tmp_path) + writer = CodexWriter() + + result = writer.write([], proj, dry_run=False) + + assert result.files_written == [] + assert not (proj.project_path / "AGENTS.md").exists() + + +class TestGeminiWriterTimestampRefresh: + """GeminiWriter must refresh existing blocks even with zero new recs.""" + + def test_empty_recs_refreshes_existing_gemini_md_block(self, tmp_path): + proj = _project(tmp_path) + gemini_md = proj.project_path / "GEMINI.md" + gemini_md.write_text( + f"{_MARKER_START}\n" + "## Headroom Learned Patterns\n" + "*Auto-generated by `headroom learn` on 2020-01-01 — do not edit manually*\n\n" + "### Env\n" + "- prior env\n\n" + f"{_MARKER_END}\n" + ) + + writer = GeminiWriter() + result = writer.write([], proj, dry_run=False) + + assert gemini_md in result.files_written + content = gemini_md.read_text() + assert "2020-01-01" not in content + assert "### Env" in content + assert "prior env" in content + + def test_empty_recs_and_no_prior_file_writes_nothing(self, tmp_path): + proj = _project(tmp_path) + writer = GeminiWriter() + + result = writer.write([], proj, dry_run=False) + + assert result.files_written == [] + assert not (proj.project_path / "GEMINI.md").exists() From 3cef28dcc9bcb7355b947c2808a5b4787e391344 Mon Sep 17 00:00:00 2001 From: Garm Date: Fri, 24 Apr 2026 12:34:32 +0200 Subject: [PATCH 2/2] style(learn): collapse _merge_into_file signature to one line for ruff format --- headroom/learn/writer.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/headroom/learn/writer.py b/headroom/learn/writer.py index c46ed136..4bd1c11f 100644 --- a/headroom/learn/writer.py +++ b/headroom/learn/writer.py @@ -161,9 +161,7 @@ def _merge_recommendations( return list(new_recommendations) + carried -def _merge_into_file( - file_path: Path, new_recommendations: list[Recommendation] -) -> str | None: +def _merge_into_file(file_path: Path, new_recommendations: list[Recommendation]) -> str | None: """Merge new recommendations with any existing marker block and rebuild the file. Returns ``None`` when there is nothing to write: no new recommendations AND