Skip to content

fix(io): use a context manager in _lesson_already_appended - #62

Merged
codejunkie99 merged 2 commits into
codejunkie99:masterfrom
diazMelgarejo:atomic-03-context-manager-fix
Aug 6, 2026
Merged

fix(io): use a context manager in _lesson_already_appended#62
codejunkie99 merged 2 commits into
codejunkie99:masterfrom
diazMelgarejo:atomic-03-context-manager-fix

Conversation

@diazMelgarejo

@diazMelgarejo diazMelgarejo commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Atomic PR 3 of 3 (PR 1, episodic-mirror, merged as #57; PR 2, UTF-8 fixes, #61 — this PR is built on top of it, so its diff includes #61's commit until that one merges).

Summary

_lesson_already_appended() opened lessons_path with a bare open() and never closed it explicitly — relying on GC/refcounting to close the file handle. Same resource-safety class as the file-handle findings from this project's own recent hygiene passes. Wraps the read in a with open(lessons_path, encoding="utf-8") as f: block; also picks up explicit UTF-8 encoding, consistent with #61.

Tests

Same suite as #61 (built on top of it): 177 passed, 1 pre-existing unrelated failure (test_unknown_executable_is_a_structured_start_failure).

Please merge #61 first — once it lands, this PR's diff collapses to just this fix's own delta.

Note

Fix file handling in _lesson_already_appended to use a context manager with UTF-8 encoding

Updates learn.py to consistently use UTF-8 encoding across file reads and writes, preventing locale-dependent encoding errors.

  • _lesson_already_appended now opens lessons.jsonl with a context manager and explicit UTF-8 encoding instead of relying on system defaults.
  • stage writes candidate JSON files with explicit UTF-8 encoding to preserve non-ASCII content.
  • sys.stdout and sys.stderr are reconfigured to UTF-8 with errors='replace' at module load time.
  • Behavioral Change: files previously read or written using the system locale encoding will now always use UTF-8; this may surface previously hidden encoding mismatches.

Macroscope summarized f685a08.

Two related Windows-compatibility fixes:

- Reconfigure stdout/stderr to UTF-8 (errors=replace) at import, so a lesson
  claim containing non-ASCII (arrows, em-dashes, accented text) doesn't raise
  UnicodeEncodeError under a cp1252 console on Windows.
- Write the candidate JSON file with explicit encoding="utf-8" rather than
  the platform default, so candidates round-trip identically across OSes.

Both are additive and platform-safe (the reconfigure is guarded by hasattr,
a no-op where unavailable). Found and applied downstream in Perpetua-Tools;
contributing back.

Stacked on top of the episodic-mirror fix (atomic-01).
The read-only probe opened lessons.jsonl with a bare open() and relied on
GC to close the handle. Wrap it in a with-statement (and add explicit
encoding="utf-8" for cross-platform consistency) so the file descriptor is
released deterministically. Behavior is otherwise unchanged.

Found and applied downstream in Perpetua-Tools; contributing back.

Stacked on top of atomic-02 (UTF-8 fixes).
@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 dc197e4 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