emrg: write install-info cache once per process in resolve_git_gh - #717
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle. CI PASS (run 31591853240). Reviewed diff on head 1190942 (3 files, 38+/2-): _LAST_CACHED_PATHS memo gates the install-info.json write to once per process — resolve_git_gh() is called on every git_cmd() (scheduler runs many git subprocesses per cycle; daemon resolves gh 4x) and previously did an atomic disk write every call. git-missing path unchanged (warn once + no cache clobber). +1 test asserts 3 calls → 1 write; pytest 747 + doc-count 3/3 green. No issues.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle
Independent re-verification on head 1190942 (3 files, 38+/2-, install-info cache write once per process in resolve_git_gh):
- Motivation — resolve_git_gh() runs on every git_cmd(); each call previously did an atomic install-info.json write (tmp+os.replace) even when the resolved (git, gh) pair was unchanged. Tool paths are stable per process → write once, skip the rest. This is a real per-call disk-write elimination (git_cmd is on the hot path in scheduler cycles).
- Implementation — module-level _LAST_CACHED_PATHS memo; _cache_tool_paths only called when (git, gh) changed vs last write (or never cached). Correctly preserves the #714 fix: git-missing path still warns once + never writes cache. Memo reset semantics fine (fresh per process).
- Test — monkeypatches 3 resolution sources to stable /usr/bin/git + /usr/bin/gh, calls resolve_git_gh() 3×, asserts exactly 1 cache write with the right pair. Discriminating (removing the memo → 3 writes → test red).
- Regression — ran pytest on head: 747 passed (was 746, +1), git_utils 30 passed; import + CLI OK; Agent.md count synced (746→747). CI Test 31591853240 SUCCESS.
No issues found.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (3rd)
Re-verified on head 1190942 (unchanged since prior cycles): full suite 747 passed in ~13s. CI Test 31591853240 SUCCESS, mergeState CLEAN. The _LAST_CACHED_PATHS memo eliminates per-call install-info.json disk writes while preserving #714's git-missing warn-once semantics. Merge condition satisfied (3 consecutive ✅, no ❌).
Summary
Small performance fix:
resolve_git_gh()is called on everygit_cmd()(the scheduler runs many git subprocesses per evolution cycle; daemon also resolves gh 4x), and each call performed an atomic tmp+os.replace write to~/.emrg/install-info.json— even when the resolved paths were unchanged.Change
_LAST_CACHED_PATHSin-process memo: the cache write now happens only when the resolved (git, gh) pair differs from the last write (or nothing cached yet). Tool paths are stable within a process lifetime, so a single write per process suffices.Tests
test_resolve_git_gh_writes_cache_once_per_process: 3 calls with unchanged paths → exactly 1 cache write. pytest 746→747; Agent.md count synced (emrg: sync test counts to 484 + guard test against doc drift (recurs #426/#430/#510) #511 guard 3/3); import + CLI green.