fix: report evicted handles as "evicted", not "not found" (#532) - #543
Open
Shashankss1205 wants to merge 1 commit into
Open
fix: report evicted handles as "evicted", not "not found" (#532)#543Shashankss1205 wants to merge 1 commit into
Shashankss1205 wants to merge 1 commit into
Conversation
Both handle stores bulk-evict the oldest entries when they hit their cap (estimator: 10 oldest; data: oldest ~20%), emitting only a debug log. The caller then saw a generic "not found" — indistinguishable from a typo — for a handle it had legitimately created. Track evicted ids in a bounded tombstone deque in both HandleManager and the Executor data-handle store, and route every not-found site through a shared describe_missing / data_handle_missing helper that says "'<id>' was evicted (handle limit N reached); reload the source / re-create it" when the id was evicted. Eviction is now logged at info level. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Fixes #532.
Problem
Both handle stores silently bulk-evict the oldest entries at their cap (estimator store: 10 oldest via
HandleManager._cleanup_oldest; data store: oldest ~20% viaExecutor._cleanup_oldest_data), emitting only alogger.debug. The caller then got a generic "not found" for a handle it had legitimately created — indistinguishable from a typo. (Confirmed live: a session-start data handle vanished after ~52 loads with a plain not-found.)Fix
Track evicted ids in a bounded tombstone
deque(maxlen=1024)in both stores, and route every not-found site through a shared helper:HandleManager.describe_missing(id)— used byget_instance/get_info(raise),evaluate,export_code,save_model,release_handle.Executor.data_handle_missing(id)— returns the error string + capped available-handles summary, used byinspect_data,split_data,save_data,transform_data,plot_series, and the executor's ownformat_data_handle/release_data_handle.When the id was evicted, the message becomes "'' was evicted (handle limit N reached); reload the source / re-create it with instantiate" instead of "not found". Eviction is now logged at info level.
Testing
tests/test_eviction_tombstones.py(4 tests): estimator + data eviction messages,get_instanceraises the eviction message, andinspect_datasurfaces it end-to-end; a never-seen id still reads as a plain "not found".🤖 Generated with Claude Code