From f7a92c6eb26f8e8e8e399f095413f09aae3b155e Mon Sep 17 00:00:00 2001 From: Denis Date: Mon, 1 Jun 2026 18:12:36 +0500 Subject: [PATCH] fix(library): rebuild SymbolLibraryManager when cache is empty When a project is opened for the first time (e.g. right after create_project) the sym-lib-table may not exist on disk yet. SymbolLibraryManager.__init__ succeeds but leaves self.libraries={}. The original guard in use_project() was: if self.library_manager.project_path == project_path: return Because the path already matched the newly-created manager, the early-return fired and the manager was never rebuilt once the sym-lib-table appeared. All subsequent list_symbols / search calls returned nothing. Fix: also require that at least one library was loaded before treating the cache as valid. Adds three unit tests that cover: - rebuild triggered when project_path matches but libraries={} - no spurious rebuild when libraries are already loaded - rebuild on project_path change (existing behaviour) --- python/commands/library_symbol.py | 6 +- ...test_symbol_library_empty_cache_rebuild.py | 130 ++++++++++++++++++ 2 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 tests/test_symbol_library_empty_cache_rebuild.py diff --git a/python/commands/library_symbol.py b/python/commands/library_symbol.py index 1bf78cd..44acc05 100644 --- a/python/commands/library_symbol.py +++ b/python/commands/library_symbol.py @@ -607,7 +607,11 @@ class SymbolLibraryCommands: """ if project_path is None: return - if self.library_manager.project_path == project_path: + # Patch (SER2RJ45): keep cache when project_path matches AND libraries were + # actually loaded. Original early-return skipped rebuild even when the cache + # was empty (e.g. first call after create_project, before sym-lib-table existed). + if (self.library_manager.project_path == project_path + and len(self.library_manager.libraries) > 0): return logger.info(f"Rebuilding SymbolLibraryManager for project: {project_path}") self.library_manager = SymbolLibraryManager(project_path=project_path) diff --git a/tests/test_symbol_library_empty_cache_rebuild.py b/tests/test_symbol_library_empty_cache_rebuild.py new file mode 100644 index 0000000..a8b0ff3 --- /dev/null +++ b/tests/test_symbol_library_empty_cache_rebuild.py @@ -0,0 +1,130 @@ +""" +Regression test for the "empty-cache early-return" bug in SymbolLibraryCommands. + +Scenario +-------- +When a project is opened for the first time (e.g. right after `create_project`) +the `sym-lib-table` may not exist on disk yet. `SymbolLibraryManager.__init__` +succeeds but leaves `self.libraries` empty. + +The original `_rebuild_if_needed` guard was: + + if self.library_manager.project_path == project_path: + return # ← BUG: triggers even when libraries == {} + +Because `project_path` already matched, the manager was never rebuilt when the +sym-lib-table finally appeared, so subsequent list_symbols / search_symbols calls +returned nothing. + +Fix: also require that at least one library was loaded before skipping the rebuild. +""" + +import sys +import threading +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent / "python")) + +from commands.library_symbol import SymbolLibraryCommands, SymbolLibraryManager + + +def _make_empty_manager(project_path) -> SymbolLibraryManager: + """Return a SymbolLibraryManager that matches the project but has no libraries loaded.""" + manager = SymbolLibraryManager.__new__(SymbolLibraryManager) + manager.project_path = project_path + manager.libraries = {} # <-- empty: sym-lib-table wasn't there yet + manager.symbol_cache = {} + manager._cache_lock = threading.Lock() + return manager + + +@pytest.mark.unit +class TestSymbolLibraryEmptyCacheRebuild: + """Ensure _rebuild_if_needed re-initialises the manager when the cache is empty.""" + + def test_rebuild_triggered_when_libraries_empty(self): + """ + If the manager has a matching project_path but no libraries, calling + _rebuild_if_needed with the same path MUST trigger a rebuild (not early-return). + """ + project_path = "/fake/project/test.kicad_pro" + + cmds = SymbolLibraryCommands.__new__(SymbolLibraryCommands) + cmds.library_manager = _make_empty_manager(project_path) + + rebuild_count = {"n": 0} + original_manager = cmds.library_manager + + def fake_init(self, project_path=None): + rebuild_count["n"] += 1 + self.project_path = project_path + self.libraries = {"SomeLib": "/fake/SomeLib.kicad_sym"} + self.symbol_cache = {} + self._cache_lock = threading.Lock() + + with patch.object(SymbolLibraryManager, "__init__", fake_init): + cmds.use_project(Path(project_path)) + + assert rebuild_count["n"] == 1, ( + "Expected use_project to construct a new SymbolLibraryManager " + "when libraries={}, but it returned early instead." + ) + assert cmds.library_manager is not original_manager, ( + "The manager instance should have been replaced after rebuild." + ) + assert len(cmds.library_manager.libraries) > 0 + + def test_no_rebuild_when_libraries_already_loaded(self): + """ + When the manager already has libraries loaded for this project, skip rebuild + (the original happy-path must still work). + """ + project_path = "/fake/project/test.kicad_pro" + + cmds = SymbolLibraryCommands.__new__(SymbolLibraryCommands) + manager = _make_empty_manager(Path(project_path)) + manager.libraries = {"SomeLib": "/fake/SomeLib.kicad_sym"} # non-empty + cmds.library_manager = manager + original_manager = cmds.library_manager + + rebuild_count = {"n": 0} + + def fake_init(self, project_path=None): + rebuild_count["n"] += 1 + + with patch.object(SymbolLibraryManager, "__init__", fake_init): + cmds.use_project(Path(project_path)) + + assert rebuild_count["n"] == 0, ( + "Should NOT rebuild when libraries are already loaded for the same project." + ) + assert cmds.library_manager is original_manager + + def test_rebuild_on_different_project_path(self): + """Changing project path always triggers rebuild (existing behaviour).""" + old_path = "/fake/project/old.kicad_pro" + new_path = "/fake/project/new.kicad_pro" + + cmds = SymbolLibraryCommands.__new__(SymbolLibraryCommands) + manager = _make_empty_manager(Path(old_path)) + manager.libraries = {"SomeLib": "/fake/SomeLib.kicad_sym"} + cmds.library_manager = manager + + rebuild_count = {"n": 0} + + def fake_init(self, project_path=None): + rebuild_count["n"] += 1 + self.project_path = project_path + self.libraries = {} + self.symbol_cache = {} + self._cache_lock = threading.Lock() + + with patch.object(SymbolLibraryManager, "__init__", fake_init): + cmds.use_project(Path(new_path)) + + assert rebuild_count["n"] == 1, ( + "Switching to a different project path must always trigger rebuild." + )