Conversation
|
/lgtm review |
There was a problem hiding this comment.
🦉 lgtm Review
Score: Needs Work 🔧
🔍 Summary
This PR successfully introduces MCP support for twyn and refactors the CI workflows. The new MCP feature is a great addition.
However, there are two major issues that need to be addressed before this can be merged.
First, the refactoring of the CI/CD pipeline has removed the testing steps from the publish.yml workflow. This is a critical regression that could lead to publishing broken releases without any tests running on the release commit.
Second, the src/twyn/cli.py file has been refactored in a way that significantly reduces code quality. Several small, well-defined functions have been combined into a single, large run function, which is now difficult to read and maintain. It would be best to revert this specific change.
I've also noted a minor typo in the new MCP implementation.
Due to the critical nature of the CI changes and the reduction in code quality, I'm scoring this PR a 3/5. Please address these points.
More information
- Id:
d28fb6e296eb4ece91cb9a74b1227ea3 - Model:
gemini-2.5-pro - Created at:
2026-02-27T14:15:52.560075+00:00
Usage summary
- Request count:
2 - Request tokens:
71,938 - Response tokens:
12,512 - Total tokens:
84,450
Configuration
-
model:
gemini-2.5-pro -
model_url:
None -
technologies:
('Python',) -
categories:
('Correctness', 'Quality', 'Testing', 'Security') -
exclude:
('*.md', 'uv.lock', 'eval/classified_articles.py') -
additional_context:
() -
publish:
True -
output_format:
pretty -
silent:
False -
ai_retries:
None -
ai_input_tokens_limit:
500000 -
issues_url:
None -
issues_regex:
(?:refs?|closes?|resolves?)[:\s]*((?:#\d+)|(?:#?[A-Z]+-\d+))|(?:fix|feat|docs|style|refactor|perf|test|build|ci)\((?:#(\d+)|#?([A-Z]+-\d+))\)!?: -
issues_platform:
None -
compare:
HEAD
See the 📚 lgtm-ai repository for more information about lgtm.
scastlara
left a comment
There was a problem hiding this comment.
Not entirely sure about all the changes to the CI 🤔 But leave that up to you.
The rest looks good.
Adding support for MCP
closes #418