Conversation
3747154 to
b832066
Compare
| def cli() -> None: | ||
| """Entry point for the MCP tool.""" | ||
| warnings.warn( | ||
| "The lgtm-mcp CLI is experimental and may change in future releases.", category=UserWarning, stacklevel=1 |
There was a problem hiding this comment.
I left this CLI undocumented, so that we are free to break it as we want.
Once it's stable, we can remove warnings and add it to the docs.
There was a problem hiding this comment.
it still has better documentation than most open source projects though
|
/lgtm review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🦉 lgtm Review
Score: Needs Work 🔧
🔍 Summary
Overall, this is a great PR that successfully adds the requested MCP server functionality. The approach of using an optional extra is well-implemented, and the corresponding changes to the CI/CD pipeline to test both scenarios (with and without [mcp]) are excellent. The new code is also well-structured and includes appropriate tests.
I have found one major issue that needs to be addressed before this can be merged:
- The MCP server currently does not load configuration from the repository being reviewed (e.g., from a
.lgtm-ai.tomlfile). This means repository-specific settings like model choice, excluded files, and custom prompts will be ignored, which is likely not the desired behavior.
I have also left a couple of minor comments for further improvement. Once the configuration loading is addressed, this PR should be in good shape. Great work!
More information
- Id:
403900f66ff844c9a7d7f49fe97950d5 - Model:
gemini-2.5-pro - Created at:
2025-09-30T14:52:39.833505+00:00
Usage summary
- Request count:
2 - Request tokens:
48,534 - Response tokens:
12,554 - Total tokens:
61,088
See the 📚 lgtm-ai repository for more information about lgtm.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
|
||
| - name: Install all extras | ||
| if: matrix.mcp-extra == true | ||
| run: poetry install --all-extras |
There was a problem hiding this comment.
nitpick: the flag refers only to mcp but all extras will be installed. They're equivalent right now, but worth keeping it in mind.
Not a blocker though
There was a problem hiding this comment.
Yeah, I guess we will re-use this step to install+test the version with all extras. I don't foresee we do like all combinations if we end up having more extras.
| def cli() -> None: | ||
| """Entry point for the MCP tool.""" | ||
| warnings.warn( | ||
| "The lgtm-mcp CLI is experimental and may change in future releases.", category=UserWarning, stacklevel=1 |
There was a problem hiding this comment.
it still has better documentation than most open source projects though
2 participants
New script command
lgtm-mcpthat starts an mcp server ready for AI agents to interact with.I had to adapt our testing infra to actually test with and without the extra.
You can see that we have an extra elements in the matrix, with values
trueorfalse, indicating whether the mcp extra must be tested.You can see in our pipelines that it tests what we want to test:
Running locally, the mcp is working and (for instance) GitHub copilot on VSCode can use the provided tool:
refs #150