fix: use stable cache dir for tool embeddings and remove exit() call#26
Open
rtmalikian wants to merge 1 commit into
Open
fix: use stable cache dir for tool embeddings and remove exit() call#26rtmalikian wants to merge 1 commit into
rtmalikian wants to merge 1 commit into
Conversation
- Replace bare 'except:' with specific FileNotFoundError/AssertionError handling to avoid catching KeyboardInterrupt, SystemExit, etc. - Use ~/.cache/txagent/embeddings/ instead of CWD-relative paths so the cached .pt file is found regardless of working directory - Auto-migrate legacy CWD-relative .pt files to the new cache location - Remove exit() call after embedding generation — instead, free the RAG encoder GPU memory (del + gc.collect + cuda.empty_cache) and re-load the encoder so the main LLM can continue without restart - Add detailed logging for cache miss, generation, and migration Fixes mims-harvard#10 (FileNotFoundError when embedding cache is in a different dir) Fixes mims-harvard#6 (exit() call prevented users from running on first attempt)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was broken
Issue #10:
FileNotFoundErrorwhen the tool embedding.ptcache file is not in the current working directory. The original code saved embeddings to a CWD-relative path, so users who ran TxAgent from a different directory than their first run would never find the cached file.Issue #6 (partial): The
load_tool_desc_embeddingmethod calledexit()after generating embeddings on first run, forcing users to restart. Combined with vLLM'sspawnmultiprocessing requirement, this created confusingRuntimeErrormessages about bootstrapping.Additionally, the original code used a bare
except:clause that caught ALL exceptions (includingKeyboardInterruptandSystemExit), which masked real errors and made debugging difficult.What the fix does
Stable cache directory: Embeddings are now saved to
~/.cache/txagent/embeddings/instead of the current working directory, so they are found regardless of launch location.Legacy migration: Existing CWD-relative
.ptfiles are automatically moved to the new cache directory for backward compatibility.Specific exception handling: Replaced bare
except:withexcept FileNotFoundErrorandexcept AssertionError— only catches the expected failure modes.No more
exit(): After generating embeddings, the RAG encoder GPU memory is freed (del+gc.collect+torch.cuda.empty_cache()) and the encoder is re-loaded, allowing the main LLM to continue without a restart.Clear logging: Users now see exactly what is happening — cache miss, generation progress, migration, and memory cleanup.
Verification results
All 6 integration tests pass on macOS (no GPU required — uses mocked SentenceTransformer):
Tests verify:
Disclosure: This code was developed with assistance from mimo-2.5-pro (Xiaomi) via Hermes Agent (Nous Research). All changes were reviewed, tested against the actual codebase, and verified for correctness.