Conversation
The cicada-scip and cicada-mcp-core workspace packages were never published to PyPI, causing installation failures for users. This inlines all SCIP code directly into cicada/languages/scip/, making SCIP language support (Python, TypeScript, Go, Rust, Java, etc.) work out of the box without extra install steps. - Replace re-export wrappers with actual source (reader, converter, formatter, language_adapters) - Merge raw + wrapper indexers into single GenericSCIPIndexer class - Remove SCIP_AVAILABLE conditional guard (always available now) - Move tests from packages/cicada-scip/tests/ to tests/languages/scip/ - Delete packages/ directory (cicada-scip, cicada-mcp-core, cicada-core) - Add protobuf>=4.0.0 to main dependencies - Update CI workflows, Makefile, .gitignore for new paths - Simplify publish workflow (single package instead of three) - Bump version to 0.6.3
There was a problem hiding this comment.
Sorry @wende, your pull request is larger than the review limit of 150000 diff characters
Summary of ChangesHello @wende, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the project's architecture by consolidating previously separate workspace packages, 'cicada-scip' and 'cicada-mcp-core', directly into the main 'cicada-mcp' package. This change aims to resolve installation complexities, ensure out-of-the-box availability of SCIP language support, and streamline the overall development and deployment process by simplifying the package structure and CI workflows. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #236 +/- ##
==========================================
+ Coverage 81.25% 82.06% +0.81%
==========================================
Files 154 154
Lines 14358 15100 +742
Branches 2850 3046 +196
==========================================
+ Hits 11666 12392 +726
+ Misses 2011 1965 -46
- Partials 681 743 +62
🚀 New features to boost your workflow:
|
|
@codex use your debug powers |
There was a problem hiding this comment.
Code Review
This is a large-scale refactoring that inlines the cicada-scip and cicada-mcp-core packages. This is a significant improvement that simplifies the project structure, dependency management, and installation process. The changes are extensive but appear to be handled consistently across the codebase, including updates to build files, CI configuration, and test paths. The core logic from the separate packages has been successfully integrated into the main cicada package. My review found one minor opportunity for improvement in the robustness of the lazy-loading mechanism for symbol type parsers. Overall, this is a great change that will improve the maintainability and user experience of the project.
| def _get_python_symbols(): | ||
| """Lazy import for Python symbol types module.""" | ||
| global _python_symbols | ||
| if _python_symbols is None: | ||
| try: | ||
| import cicada.languages.python.symbol_types as ps | ||
|
|
||
| _python_symbols = ps | ||
| except ImportError: | ||
| _python_symbols = None | ||
| return _python_symbols | ||
|
|
||
|
|
||
| def _get_typescript_symbols(): | ||
| """Lazy import for TypeScript symbol types module.""" | ||
| global _typescript_symbols | ||
| if _typescript_symbols is None: | ||
| try: | ||
| import cicada.languages.typescript.symbol_types as ts | ||
|
|
||
| _typescript_symbols = ts | ||
| except ImportError: | ||
| _typescript_symbols = None | ||
| return _typescript_symbols |
There was a problem hiding this comment.
The lazy-loading functions _get_python_symbols and _get_typescript_symbols currently suppress ImportError and return None. This can lead to a less clear AttributeError later when get_symbol_type is called on the None value. Since cicada-scip is now inlined, these imports should always succeed. It would be more robust to remove the try...except blocks and let an ImportError propagate if it occurs, as this would indicate a fundamental issue with the package structure.
def _get_python_symbols():
"""Lazy import for Python symbol types module."""
global _python_symbols
if _python_symbols is None:
import cicada.languages.python.symbol_types as ps
_python_symbols = ps
return _python_symbols
def _get_typescript_symbols():
"""Lazy import for TypeScript symbol types module."""
global _typescript_symbols
if _typescript_symbols is None:
import cicada.languages.typescript.symbol_types as ts
_typescript_symbols = ts
return _typescript_symbols|
Summary
Testing
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on March 1
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| 0, | ||
| '', | ||
| 'scip.proto' | ||
| ) |
There was a problem hiding this comment.
Protobuf dependency version too loose for generated code
High Severity
The generated scip_pb2.py imports from google.protobuf import runtime_version, which was introduced in protobuf 5.26.0. However, pyproject.toml specifies protobuf>=4.0.0. Users who install with protobuf 4.x or 5.0–5.25 will get an ImportError when importing any SCIP module, completely breaking all SCIP-based language indexers (Python, TypeScript, Go, Rust, Java, Scala, C, C++, Ruby, C#, VB, Dart).
| if func_keywords: | ||
| func_entry["keywords"] = func_keywords | ||
|
|
||
| modules[file_module_name] = file_module |
There was a problem hiding this comment.
Missing null check for file module name causes invalid key
Low Severity
_get_file_module_name() can return None when the document's relative_path is empty or processes to an empty string (e.g., "__init__.py"). The return value is used directly as a dict key at line 802 (modules[file_module_name]) and as the module's "name" field at line 770. When None, this creates a module keyed by None in the dict, which serializes to "null" in JSON output—producing an invalid module entry.
Additional Locations (1)
…xcept, guard against None module name - Bump protobuf dependency from >=4.0.0 to >=5.26.0 (runtime_version requires it) - Remove try/except around lazy imports since SCIP is now inlined - Add None check for _get_file_module_name to prevent invalid dict key
The inlining commit accidentally removed existing entries for SCIP error messages, README comparison table, and internal workflow improvements.


Summary
cicada-scipandcicada-mcp-coreworkspace packages directly intocicada/languages/scip/, eliminating the monorepo split that caused PyPI installation failures--with cicada-scipneededprotobuf>=4.0.0to main deps, bumps to v0.6.3What changed
cicada/languages/scip/delegating topackages/cicada-scip/cicada/languages/scip/RawSCIPIndexer+ wrapperGenericSCIPIndexer) with delegationGenericSCIPIndexerclassuv tool install cicada-mcp --with cicada-scipuv tool install cicada-mcpif SCIP_AVAILABLE:guard)packages/cicada-scip/tests/andtests/tests/(SCIP tests moved totests/languages/scip/)Files overview
cicada/languages/scip/{reader,converter,formatter,language_adapters}.py— replaced re-export wrappers with actual sourcecicada/languages/scip/indexer.py— merged raw + wrapper into single classcicada/languages/scip/__init__.py— direct imports, no try/exceptcicada/languages/__init__.py— unconditional SCIP language registrationpyproject.toml— removed workspace config, added protobuf dep, bumped to 0.6.3.github/workflows/publish-pypi.yml— removed cicada-mcp-core build/publish steps.github/workflows/test-scip-languages.yml— updated pathsMakefile,.gitignore— updated pathspackages/— deleted entirelytests/languages/scip/— 16 test files moved here from packagesTest plan
make test)from cicada.languages.scip import ...)uv buildproduces clean packageuv lockresolves without workspace packagespip installfrom built wheel worksNote
Medium Risk
Moderate risk due to a large packaging/CI refactor plus substantial new inlined SCIP indexing/conversion code paths that can affect publishing and multi-language indexing behavior.
Overview
SCIP support is now bundled directly into
cicada-mcpby replacingcicada/languages/scip/*re-export shims with the full in-repo implementation (converter, reader, formatter, indexer, adapters) and generating/using checked-inscip_pb2stubs.CI and developer workflows are updated accordingly: GitHub Actions now generates protobufs from
cicada/languages/scip/scip.proto, installs viauv sync --dev, and publishes onlycicada-mcp(removingcicada-mcp-corepublish/wait steps); language tests watch the new path. The language registry now registers SCIP languages unconditionally and removes the “installcicada-scip” guidance, while thepackages/cicada-mcp-coreworkspace package is deleted and Makefile/.gitignore paths are updated to match the new layout.Written by Cursor Bugbot for commit dbc223d. Configure here.