fix(cli): defer graph initialization - #436
Conversation
Signed-off-by: Deepak Jain <deepujain@gmail.com>
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Help now avoids eager analyzer initialization, but the lazy load makes the documented package-level graph API import-order dependent: after the first load, later imports receive the submodule rather than an invokable graph. Please preserve a stable export and cover the post-load re-import case. All required checks are green.
| if self._compiled is None: | ||
| with self._lock: | ||
| if self._compiled is None: | ||
| from skillspector.graph import graph as compiled_graph |
There was a problem hiding this comment.
[P1] Keep the public graph export stable after lazy loading. Importing skillspector.graph here makes Python assign that submodule to the package's graph attribute, overwriting the LazyGraph exported by skillspector.__init__. I reproduced this on the exact head: after first.invoke is resolved, a later from skillspector import graph returns the module and graph.invoke raises AttributeError. Preserve the documented package export across import order and add a regression that imports it again after the first lazy load.
Summary
graph.graph.invoke/graph.ainvokeinterface through a thread-safe lightweight proxy.--helpstays quiet.Validation
uv run pytest tests/unit/test_cli.py -q— 102 passed.uv run pytest -m 'not integration and not provider' tests/ -q— 2,856 passed, 13 skipped, 38 deselected, 4 xfailed.uv run ruff check src tests— passed.uv run ruff format --check src tests— 194 files already formatted.uv run skillspector scan tests/fixtures/safe_skill --no-llm --format json— completed successfully with a 100% complete SAFE report; first graph use still emitted unavailable-analyzer warnings.uv run skillspector --help— returned clean help without analyzer warnings; observed startup fell from roughly 17 seconds to roughly 1.3 seconds in the same worktree.git diff --check— passed.Risk
skillspector.graphretain their existing eager behavior; the documented package export and CLI use the lazy proxy.Fixes #435