Skip to content

fix: zilliztech/claude-context#421 - #423

Closed
Zewang0217 wants to merge 1 commit into
zilliztech:masterfrom
Zewang0217:fix/issue-421
Closed

fix: zilliztech/claude-context#421#423
Zewang0217 wants to merge 1 commit into
zilliztech:masterfrom
Zewang0217:fix/issue-421

Conversation

@Zewang0217

Copy link
Copy Markdown

Summary

Grammar loaders in packages/core/src/splitter/ast-splitter.ts were required eagerly at module load, unconditionally and without error handling. On macOS arm64, the unsigned tree-sitter-cpp/tree-sitter-scala prebuilds fail dlopen with ERR_DLOPEN_FAILED, crashing the whole MCP server at startup for every user — even ones with pure TypeScript/JavaScript codebases and zero C++/Scala files.

This PR makes per-language grammar loading lazy and defensive. Grammars are now require()d on first use (inside a try/catch), so a broken grammar only degrades that one language:

  • AST splitting is disabled for the affected language
  • A warning is logged ([ASTSplitter] ⚠️ Failed to load ...)
  • Files of that language fall back to the existing LangChain character-based splitter

The rest of the pipeline (and every other language) keeps working normally.

Why this fixes the issue

The crash was self-inflicted: any single broken parser took down the server for all languages, regardless of what the user's codebase actually contains. Lazy, guarded loading confines the failure to exactly the language whose grammar is broken. The upstream unsigned-binary problem (arguably a tree-sitter-cpp/tree-sitter-scala packaging bug) now becomes a graceful degradation instead of a hard, unrecoverable startup crash.

Implementation details

  • Replaced the module-level require block with a LANGUAGE_PARSER_LOADERS map of lazy loader functions.
  • Added loadLanguageParser() with per-language memoization (loadedParsers) and a failedLanguages set so a grammar is only attempted once per process — no repeated dlopen failures at split time.
  • getLanguageConfig() now routes through LANGUAGE_ALIASES → canonical key → loader, keeping alias handling (js/ts/c++/cs, …) intact.
  • isLanguageSupported() is now accurate per-platform: it actually attempts the load rather than hardcoding a static language list, so a language with a broken grammar correctly reports unsupported.
  • New getSupportedLanguages() exposes the canonical languages with AST node definitions.

Verification

  • Added ast-splitter.test.ts covering lazy loading, failed-load fallback (returns null + logs warning, doesn't throw), memoization, and language alias resolution.
  • Core test suite passes: pnpm --filter @zilliz/claude-context-core test.
  • pnpm typecheck and pnpm lint pass.

Closes #421

@Zewang0217
Zewang0217 marked this pull request as ready for review August 13, 2026 02:10
expect(splitter).toBeInstanceOf(AstCodeSplitter);
});

it('falls back to the LangChain splitter for a language whose grammar failed to load', async () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

packages/core/src/splitter/ast-splitter.test.ts line:16
Medium ---- This test proves that a broken C++ grammar falls back, but it does not verify the new failedLanguages memoization contract: call split (or isLanguageSupported) twice and assert the loader/warning runs only once. Without that assertion, a regression that retries the failing native require for every file would still pass while reintroducing repeated dlopen work and warning noise.

@Zewang0217

Copy link
Copy Markdown
Author

Addressed review comment 3772389966 (failedLanguages memoization): added a test that reloads the module fresh (jest.resetModules()), then asserts two split() calls on the broken cpp grammar invoke the loader and console.warn exactly once — a per-file re-require regression now fails the suite. Verified: 8/8 core test suites pass, tsc --noEmit clean. Commit c7f2270.

@Zewang0217

Copy link
Copy Markdown
Author

Superseded by #426 — this PR was auto-closed when the head branch was recreated without shared history with master (my earlier force-push caused it). #426 is the same change (lazy grammar loading + failedLanguages memoization, review feedback addressed) on a branch based on current master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants