fix(cli install): pass missing verbose arg and use background-neutral styles#294
Closed
HumanBean17 wants to merge 12 commits into
Closed
fix(cli install): pass missing verbose arg and use background-neutral styles#294HumanBean17 wants to merge 12 commits into
HumanBean17 wants to merge 12 commits into
Conversation
HumanBean17
force-pushed
the
fix/installer-ci-and-colors
branch
4 times, most recently
from
b4e0e6c to
a93cfff
Compare
Explicitly release Kuzu native objects (Connection, Database, QueryResult) before Python's shutdown GC reclaims them in unpredictable order. The use-after-free manifests as a segfault in an asyncio background thread after test_kuzu_queries.py completes. - Add KuzuGraph.close() to release _conn and _db - Add yield-based teardown to all kuzu_graph fixtures in conftest.py - Release kuzu objects in kuzu_db_path fixture after assertions - Release kuzu objects in _open_stale_ontology_graph helper Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous fix only set references to None without calling the native close() methods on kuzu.Connection and kuzu.Database. This led to use-after-free segfaults when Python's GC reclaimed the objects in unpredictable order, especially in asyncio background threads. Changes: - KuzuGraph.close(): Call _conn.close() and _db.close() explicitly - conftest kuzu_db_path fixture: Call conn.close() and db.close() - test_kuzu_queries helper: Call conn.close() and db.close() Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…flicts The segfault was caused by pytest-asyncio's background event loop thread conflicting with Kuzu's internal threading. Setting num_threads=1 on Connection initialization avoids this conflict. Changes: - KuzuGraph.__init__: Set num_threads=1 on Connection - conftest kuzu_db_path fixture: Set num_threads=1 - test_kuzu_queries helper: Set num_threads=1 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The segfault was caused by pytest-asyncio creating background event loops for all tests when asyncio_mode=auto. Kuzu's C++ library conflicts with these background threads, causing segfaults during test execution. Solution: - Switch asyncio_mode from auto to strict in both pytest.ini files - Explicitly mark async test files with pytest.mark.asyncio: - tests/test_mcp_v2.py - tests/test_mcp_tools.py - tests/test_lancedb_e2e.py This prevents pytest-asyncio from creating event loops for kuzu tests while keeping asyncio working for tests that explicitly opt in. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
pytest-asyncio leaves event loops running after async tests complete, causing background threads that conflict with Kuzu's C++ library during subsequent kuzu tests. Added pytest_runtest_teardown hook to explicitly close lingering event loops after each test, ensuring no background asyncio threads exist when kuzu queries execute. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Root cause: importing server.py triggered lancedb module load, which spawns a LanceDBBackgroundEventLoop daemon thread. This background thread conflicts with Kuzu's C++ internals during query execution, causing a segfault in later tests. - Lazy-import search_lancedb in server.py and mcp_v2.py (only needed when search/tables tools are actually called) - Remove ineffective pytest_runtest_teardown hook that closed main- thread event loops but never touched the LanceDB background thread - Revert asyncio_mode back to auto (strict was part of the failed fix chain) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Module-level run_search=None and TABLES=None with _init_search() let tests monkeypatch mcp_v2.run_search while still deferring the real search_lancedb import until first use. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ests The LanceDB background event-loop thread was still being created during test collection via two paths: - search_lancedb.py imported lancedb at module level (line 14) - test_brownfield_overrides.py tests imported java_index_flow_lancedb which imports cocoindex.connectors.lancedb Changes: - search_lancedb.py: replace `import lancedb` with lazy _connect_lancedb() helper that imports on first use - test_brownfield_overrides.py: gate the 3 tests that trigger cocoindex/lancedb imports behind JAVA_CODEBASE_RAG_RUN_HEAVY Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
These tests call run_search() which triggers _connect_lancedb() -> import lancedb -> LanceDBBackgroundEventLoop daemon thread. On CI (Ubuntu) the thread race-conditions with Kuzu C++ queries causing a segfault. On macOS the race didn't manifest due to timing differences. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…led fix chain The 5 prior fix attempts introduced num_threads=1 and explicit close/GC in conftest fixtures. These changes ALTERED Kuzu's C++ threading behavior and likely converted a latent race condition into a reliable segfault on CI. The original code (default num_threads, no explicit close) ran fine alongside the LanceDB background thread for months. The real fix is preventing the LanceDB thread from being created (done in prior commits). Reverting these workarounds restores the stable state. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…gfault test_increment_updates_lance_after_touch_java_file imports lancedb which spawns a LanceDBBackgroundEventLoop daemon thread. This thread conflicts with Kuzu C++ queries in subsequent tests, causing segfaults. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…Kuzu segfault index_dir_has_existing_artifacts imported lancedb to check for existing Lance tables, spawning a LanceDBBackgroundEventLoop daemon thread. This thread causes Kuzu C++ segfaults in subsequent tests (kuzu is archived with known thread-safety bugs). Replace the lancedb import with a filesystem heuristic that checks for data.lance files inside subdirectories. Same detection capability, zero thread creation. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
HumanBean17
force-pushed
the
fix/installer-ci-and-colors
branch
from
4cbf7eb to
4641682
Compare
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.
Summary
Fixes two regressions from #292:
run_build_ast_graph()now requires averbosekeyword argument, butinstaller.py:run_init_if_needed()wasn't passing it —verbose=not quietadded.Test plan
tests/test_installer.py— all 66 tests passtests/test_installer_integration.py— both previously-failing integration tests passruff check— clean🤖 Generated with Claude Code