fix(prompt): start PromptCache workers lazily instead of on import#26
Open
aloktripathi1 wants to merge 1 commit into
Open
fix(prompt): start PromptCache workers lazily instead of on import#26aloktripathi1 wants to merge 1 commit into
aloktripathi1 wants to merge 1 commit into
Conversation
PromptCache spawned 2 background _RefreshWorker daemon threads eagerly at construction, including for the module-level singleton created at import. But refresh_async() -- the only thing that feeds those threads -- is never called in the SDK, so every user importing fi.prompt got 2 permanently idle threads. _TaskManager now starts workers lazily on first submit() via double-checked locking. No public API change; refresh_async() works as before, threads just start on demand. Adds 4 regression tests covering lazy startup, on-demand worker creation, idempotent start, and safe shutdown before any submit.
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
1 participant
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.
Closes (link the cache thread issue if you filed one separately)
Importing fi.prompt (per the README's own quickstart,
from fi.prompt import Prompt, PromptTemplate, ModelConfig) eagerly spawns 2 background_RefreshWorker daemon threads via the module-level prompt_cache singleton.
These threads poll an empty queue forever, because refresh_async(), the
only thing that feeds work to them, is never called anywhere in the SDK.
get_by_name() explicitly opts out, per its own comment: "Async refresh logic
is not applied here to simplify the fallback flow." So every user importing
fi.prompt gets 2 permanently idle threads for zero benefit.
Verified with threading.active_count() before and after the README import:
Fix: _TaskManager now starts its workers lazily on the first submit() call
via double-checked locking, instead of eagerly in init. No public API
change, refresh_async() behaves exactly as before, the threads just start
on demand if and when something actually uses them.
4 new regression tests:
Note: the 2 failing tests in test_annotation_queues.py
(TestAnalytics::test_get_analytics, TestExport::test_export_json) are
pre-existing on main and unrelated to this change.