Skip to content

fix(learn): refresh context-file timestamp when LLM returns no new recommendations#259

Open
gglucass wants to merge 2 commits intochopratejas:mainfrom
gglucass:fix/writer-refresh-timestamp-on-empty-recs
Open

fix(learn): refresh context-file timestamp when LLM returns no new recommendations#259
gglucass wants to merge 2 commits intochopratejas:mainfrom
gglucass:fix/writer-refresh-timestamp-on-empty-recs

Conversation

@gglucass
Copy link
Copy Markdown
Contributor

Description

When headroom learn produces zero recommendations (LLM returns empty
context_file_rules / memory_file_rules arrays), the writers
short-circuit before touching the target files — so a successful run
that had nothing new to report leaves CLAUDE.md / MEMORY.md /
AGENTS.md / GEMINI.md completely untouched, including the
*Auto-generated by headroom learn on YYYY-MM-DD — do not edit manually*
timestamp. Users have no visible signal that the run actually happened
— looking at the file, it's indistinguishable from "learn never ran."

I hit this today: headroom learn on a project with 155 sessions /
10,732 tool calls completed successfully with "No actionable patterns found." and exit 0, but the timestamp in my CLAUDE.md stayed frozen
at the previous run's date. It looked broken even though it wasn't.

This PR drops the leading if context_recs: / if memory_recs: /
if not recommendations: guards in ClaudeCodeWriter, CodexWriter,
and GeminiWriter, and moves the skip decision into _merge_into_file
so the "there's a marker block to refresh" case is handled uniformly
across all four output files.

Fixes #(no linked issue — surfaced organically while iterating on
headroom learn in headroom-desktop)

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
  • Code refactoring (no functional changes)

Changes Made

  • _merge_into_file signature: strstr | None. Returns None
    only when there are no new recommendations AND no existing marker
    block to refresh. Docstring updated to spell out the contract.
  • ClaudeCodeWriter, CodexWriter, GeminiWriter: drop the leading
    if …recs: / if not recommendations: guards. Call
    _merge_into_file unconditionally; skip the write when it returns
    None. Net result: when a marker block exists, today's date and the
    carried-forward sections are always re-emitted; when no block exists
    and no recs are produced, nothing is written (no empty-block
    creation).
  • 11 new unit tests covering the four writer paths (ClaudeCodeWriter
    CLAUDE.md + MEMORY.md, CodexWriter AGENTS.md, GeminiWriter
    GEMINI.md) plus direct _merge_into_file contract tests.
  • CHANGELOG entry under [Unreleased] → Fixed.

Testing

  • Unit tests pass (pytest)
  • Linting passes (ruff check .)
  • Type checking passes (mypy headroom)
  • New tests added for new functionality
  • Manual testing performed

Manual end-to-end was not run against the patched code in this PR —
the fix is test-covered and the behavior is small enough to validate
from the unit-test surface. Happy to run an end-to-end pass if a
reviewer wants confirmation.

Test Output

$ uv run --frozen --extra dev pytest tests/test_learn/
======================== 156 passed, 1 skipped in 2.94s ========================

$ uv run --frozen --extra dev ruff check .
All checks passed!

$ uv run --frozen --extra dev mypy headroom
Success: no issues found in 327 source files

Representative new tests from tests/test_learn/test_writer.py:

TestMergeIntoFile::test_no_recs_and_no_file_returns_none PASSED
TestMergeIntoFile::test_no_recs_and_file_without_block_returns_none PASSED
TestMergeIntoFile::test_no_recs_and_existing_block_refreshes_timestamp PASSED
TestClaudeCodeWriterTimestampRefresh::test_empty_recs_refreshes_existing_claude_md_block PASSED
TestClaudeCodeWriterTimestampRefresh::test_empty_recs_refreshes_existing_memory_md_block PASSED
TestClaudeCodeWriterTimestampRefresh::test_empty_recs_and_no_prior_files_writes_nothing PASSED
TestClaudeCodeWriterTimestampRefresh::test_empty_recs_and_file_without_block_leaves_it_alone PASSED
TestCodexWriterTimestampRefresh::test_empty_recs_refreshes_existing_agents_md_block PASSED
TestCodexWriterTimestampRefresh::test_empty_recs_and_no_prior_files_writes_nothing PASSED
TestGeminiWriterTimestampRefresh::test_empty_recs_refreshes_existing_gemini_md_block PASSED
TestGeminiWriterTimestampRefresh::test_empty_recs_and_no_prior_file_writes_nothing PASSED

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have updated the CHANGELOG.md if applicable

Documentation checkbox left unchecked because this is an internal
behavior fix with no user-facing docs section to update (behavior is
not spec'd in the README). The _merge_into_file docstring was
updated inline and the CHANGELOG covers the user-visible change.

Screenshots (if applicable)

N/A — behavior change is a timestamp string in a markdown file.

Additional Notes

Signature change on _merge_into_file (private, leading underscore) is
not a public-API break. The three writers are the only callers.

The section-level carry-forward behavior from #231 is preserved:
prior-section carry-forward continues to work, and a re-run with new
recs still wins over same-named prior sections.

…commendations

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.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

❌ Patch coverage is 96.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
headroom/learn/writer.py 96.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant