Skip to content

fix(recall): exclude superseded lessons from lessons.jsonl retrieval - #60

Merged
codejunkie99 merged 1 commit into
codejunkie99:masterfrom
diazMelgarejo:fix/recall-supersession-filter
Aug 6, 2026
Merged

fix(recall): exclude superseded lessons from lessons.jsonl retrieval#60
codejunkie99 merged 1 commit into
codejunkie99:masterfrom
diazMelgarejo:fix/recall-supersession-filter

Conversation

@diazMelgarejo

@diazMelgarejo diazMelgarejo commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

recall.py's _load_structured() already dedupes lessons.jsonl by latest row per id — handling retraction correctly, since retract_lesson.py appends a same-id row rather than editing in place. It has no supersession awareness though: supersession creates a new id whose supersedes field points at the old one, and the old row's own status stays "accepted" forever (supersession never edits it). Proactive recall can return both the stale and the replacement guidance for the same topic.

render_lessons.py's _build_auto_section already computes exactly this — an old-id → new-id map, accepted-supersessions-only (a provisional --supersedes must not retire the old lesson before its replacement is itself accepted, or retrieval momentarily has no active guidance on that topic at all).

Change

Extracted that computation into a public superseded_by_map(lessons) function in render_lessons.py, and import it from recall.py rather than re-deriving the same rule a second time. Retrieval and rendering now share one source of truth for "which lessons are currently retired," instead of two copies that can drift apart — which is how this gap happened in the first place.

Minimal diff: 2 files, ~29 lines of actual code change (plus a new test file).

Tests

tests/test_recall_supersession.py (new):

  • superseded lesson excluded from _load_structured()
  • recall() ranks the replacement, not the superseded lesson (concrete trigger: querying "git branch -f cherry-pick rollback update-ref" previously ranked a superseded "silent no-op" lesson alongside its corrected "exit 128" replacement)
  • provisional supersession does not retire the old lesson
  • direct unit test on superseded_by_map itself, independent of the recall/render integration
python3 -m pytest tests/test_recall_supersession.py tests/test_retract_lesson.py -v
# 8 passed

Full suite: 179 passed, 1 pre-existing failure unrelated to this change (test_unknown_executable_is_a_structured_start_failure — a macOS PermissionError-vs-FileNotFoundError platform quirk; confirmed via git stash that it fails identically on a clean checkout of master).

Note

Exclude superseded lessons from _load_structured retrieval in recall

  • Extracts a new superseded_by_map() function in render_lessons.py that maps retired lesson IDs to their accepted replacements; shared between rendering and retrieval.
  • Updates recall.py to compute this map after loading lessons and skip any lesson whose ID appears as superseded by an accepted successor.
  • Adds tests/test_recall_supersession.py covering exclusion of superseded lessons, ranking of replacements, and the edge case where a provisional (non-accepted) supersession does not retire the old lesson.
  • Behavioral Change: _load_structured now returns only the replacement lesson when a supersession is accepted, not both the old and new lessons.

Macroscope summarized fb51f82.

recall.py's _load_structured() already deduped lessons.jsonl by latest
row per id (handling retraction, which appends a same-id row rather
than editing in place) but had no supersession awareness: supersession
creates a NEW id whose supersedes field points at the old one, and the
old row's own status stays "accepted" forever since supersession never
edits it. Proactive recall could return both the stale and replacement
guidance for the same topic.

render_lessons.py's _build_auto_section already computed exactly this
-- an old-id -> new-id map, accepted-supersessions-only (a provisional
--supersedes must not retire the old lesson before its replacement is
itself accepted). Extracted that computation into a public
superseded_by_map(lessons) function and import it from recall.py,
rather than re-deriving the same rule a second time -- retrieval and
rendering now share one source of truth for "which lessons are
currently retired" instead of two copies that can drift apart.

Tests: tests/test_recall_supersession.py (superseded lesson excluded
from _load_structured, recall() ranks the replacement not the
superseded lesson, provisional supersession does not retire the old
lesson) + a direct unit test on superseded_by_map itself. Full suite:
179 passed, 1 pre-existing unrelated failure (confirmed via git stash
against a clean checkout: test_unknown_executable_is_a_structured_start_failure,
a macOS PermissionError-vs-FileNotFoundError platform quirk, not
touched by this change).
@diazMelgarejo

Copy link
Copy Markdown
Contributor Author

Related, independent PRs from the same review pass — each stands on its own, no dependency between them:

cc @codejunkie99 — happy to answer questions on any of these, or squash/reorder however's easiest to review.

@codejunkie99
codejunkie99 merged commit 31f1d32 into codejunkie99:master Aug 6, 2026
1 check passed
codejunkie99 pushed a commit that referenced this pull request Aug 6, 2026
Patch release covering #60, #61, #62, #63: superseded-lesson filtering in
recall, the doubled skills/ path in upgrade's loop-skill copy, UTF-8 I/O in
learn.py, and a leaked file handle.

Collapses the stacked per-version README history (v0.9.0 through v0.18.0)
into a single pointer at CHANGELOG.md, and relaxes the onboarding docs test
from an exact version pin to the 0.19.x series so patch releases keep
asserting the loop docs and sandbox caveat without editing the assertion
each time.
cursor Bot pushed a commit to diazMelgarejo/Perpetua-Tools that referenced this pull request Aug 8, 2026


Bumps vendor/agentic-stack from 00eda65c (v0.9.0-era, unchanged since
2026-07-16) to 17f1bf65 (v0.19.0-9) -- 18 commits of genuine upstream
progress (bounded agentic loops, MiniMax provider, Autohand Code CLI
adapter) plus 3 PT-authored fixes not yet merged upstream:

- codejunkie99/agentic-stack#60 -- recall.py excludes superseded
  lessons (shared superseded_by_map helper with render_lessons.py)
- codejunkie99/agentic-stack#61 -- UTF-8 stdout/stderr + candidate
  writes in learn.py
- codejunkie99/agentic-stack#62 -- context manager in
  _lesson_already_appended

All 3 verified MERGEABLE with passing tests before pinning to them.
Also includes a 4th fix found and applied during this same pass:
harness_manager/upgrade.py's loop-skill copy doubled the "skills/"
path segment (.agent/skills/skills/loop-x/ instead of
.agent/skills/loop-x/), both in the CLI's own --dry-run report and the
real file copy -- silent because no existing test exercised a fresh
(non-pre-seeded) loop-skill destination. Fixed + regression test on
the fork; not yet opened as its own upstream PR.

Because none of the above have merged upstream yet, .gitmodules
temporarily points at the diazMelgarejo fork instead of
codejunkie99/agentic-stack -- documented inline with the exact revert
path (repoint the url, then
scripts/git/agentic-stack-submodule-sync.sh upgrade) once all 4 land.
Full provenance chain in
.agent/.agentic-stack-blend-state.json:last_blend.note.

Verified: a fresh clone of the fork successfully checks out 17f1bf6,
confirming `git submodule update --init` will resolve correctly for
anyone cloning PT with the current .gitmodules.
cursor Bot pushed a commit to diazMelgarejo/Perpetua-Tools that referenced this pull request Aug 8, 2026


Bumps vendor/agentic-stack from 00eda65c (v0.9.0-era, unchanged since
2026-07-16) to 17f1bf65 (v0.19.0-9) -- 18 commits of genuine upstream
progress (bounded agentic loops, MiniMax provider, Autohand Code CLI
adapter) plus 3 PT-authored fixes not yet merged upstream:

- codejunkie99/agentic-stack#60 -- recall.py excludes superseded
  lessons (shared superseded_by_map helper with render_lessons.py)
- codejunkie99/agentic-stack#61 -- UTF-8 stdout/stderr + candidate
  writes in learn.py
- codejunkie99/agentic-stack#62 -- context manager in
  _lesson_already_appended

All 3 verified MERGEABLE with passing tests before pinning to them.
Also includes a 4th fix found and applied during this same pass:
harness_manager/upgrade.py's loop-skill copy doubled the "skills/"
path segment (.agent/skills/skills/loop-x/ instead of
.agent/skills/loop-x/), both in the CLI's own --dry-run report and the
real file copy -- silent because no existing test exercised a fresh
(non-pre-seeded) loop-skill destination. Fixed + regression test on
the fork; not yet opened as its own upstream PR.

Because none of the above have merged upstream yet, .gitmodules
temporarily points at the diazMelgarejo fork instead of
codejunkie99/agentic-stack -- documented inline with the exact revert
path (repoint the url, then
scripts/git/agentic-stack-submodule-sync.sh upgrade) once all 4 land.
Full provenance chain in
.agent/.agentic-stack-blend-state.json:last_blend.note.

Verified: a fresh clone of the fork successfully checks out 17f1bf6,
confirming `git submodule update --init` will resolve correctly for
anyone cloning PT with the current .gitmodules.
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.

2 participants