Conversation
Summary of ChangesHello @teamauresta, 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 enhances the command-line interface by introducing a standard Highlights
Changelog
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
|
There was a problem hiding this comment.
Code Review
The pull request introduces a new feature to display the application version via --version or -v flags, which is a good addition for user convenience and debugging. The implementation correctly uses importlib.metadata.version to retrieve the package version, ensuring it's not hardcoded. A dedicated test case has been added to verify the functionality of the new flags, which is crucial for maintaining code quality. The use of typer.Exit() after displaying the version is appropriate to terminate the application gracefully. Overall, the changes are well-implemented and align with the stated acceptance criteria.
| assert result.stdout.startswith("code-graph-rag version "), ( | ||
| f"{flag} output did not match expected format: {repr(result.stdout)}" | ||
| ) | ||
| assert result.stderr == "", f"Unexpected stderr for {flag}: {result.stderr}" |
There was a problem hiding this comment.
The assertion assert result.stderr == "", f"Unexpected stderr for {flag}: {result.stderr}" is good for ensuring clean output. However, in some environments, typer.echo might write to stderr instead of stdout depending on how the output stream is configured or redirected. While typer.echo typically writes to stdout, it's safer to check if stderr is empty or contains only expected output, especially when dealing with CLI tools that might have subtle differences in output streams across platforms or configurations. For this specific case, since typer.echo is used, it's highly probable it will write to stdout, but it's a good practice to be aware of this potential edge case.
Greptile SummaryImplements
Issues found:
Confidence Score: 3/5
Important Files Changed
Last reviewed commit: 50b9e49 |
codebase_rag/cli.py
Outdated
| None, | ||
| "--version", | ||
| "-v", | ||
| help="Show the version and exit.", |
There was a problem hiding this comment.
hardcoded string should be moved to cli_help.py as HELP_VERSION
| help="Show the version and exit.", | |
| help=ch.HELP_VERSION, |
Context Used: Rule from dashboard - ## Technical Requirements
Agentic Framework
- PydanticAI Only: This project uses PydanticAI... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: codebase_rag/cli.py
Line: 72
Comment:
hardcoded string should be moved to `cli_help.py` as `HELP_VERSION`
```suggestion
help=ch.HELP_VERSION,
```
**Context Used:** Rule from `dashboard` - ## Technical Requirements
### Agentic Framework
- **PydanticAI Only**: This project uses PydanticAI... ([source](https://app.greptile.com/review/custom-context?memory=d4240b05-b763-467a-a6bf-94f73e8b6859))
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
codebase_rag/cli.py
Outdated
|
|
||
| def _version_callback(value: bool) -> None: | ||
| if value: | ||
| typer.echo(f"code-graph-rag version {get_version('code-graph-rag')}") |
There was a problem hiding this comment.
hardcoded format string should be moved to constants.py as CLI_MSG_VERSION
Also, use app_context.console.print() instead of typer.echo() per coding standards (Loguru Over Print section)
Context Used: Rule from dashboard - ## Technical Requirements
Agentic Framework
- PydanticAI Only: This project uses PydanticAI... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: codebase_rag/cli.py
Line: 40
Comment:
hardcoded format string should be moved to `constants.py` as `CLI_MSG_VERSION`
Also, use `app_context.console.print()` instead of `typer.echo()` per coding standards (Loguru Over Print section)
**Context Used:** Rule from `dashboard` - ## Technical Requirements
### Agentic Framework
- **PydanticAI Only**: This project uses PydanticAI... ([source](https://app.greptile.com/review/custom-context?memory=d4240b05-b763-467a-a6bf-94f73e8b6859))
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
codebase_rag/tests/test_cli_smoke.py
Outdated
| assert result.returncode == 0, ( | ||
| f"{flag} exited with code {result.returncode}: {result.stderr}" | ||
| ) | ||
| assert result.stdout.startswith("code-graph-rag version "), ( |
There was a problem hiding this comment.
hardcoded string should reference the constant from constants.py (e.g., cs.CLI_MSG_VERSION_PREFIX)
Context Used: Rule from dashboard - ## Technical Requirements
Agentic Framework
- PydanticAI Only: This project uses PydanticAI... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: codebase_rag/tests/test_cli_smoke.py
Line: 53
Comment:
hardcoded string should reference the constant from `constants.py` (e.g., `cs.CLI_MSG_VERSION_PREFIX`)
**Context Used:** Rule from `dashboard` - ## Technical Requirements
### Agentic Framework
- **PydanticAI Only**: This project uses PydanticAI... ([source](https://app.greptile.com/review/custom-context?memory=d4240b05-b763-467a-a6bf-94f73e8b6859))
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.50b9e49 to
8aacac7
Compare
|
Closes #239 |
Implements cgr --version and cgr -v commands that display the current package version read from importlib.metadata. Acceptance criteria from issue vitali87#239: - cgr --version and cgr -v both work - Version read from package metadata (not hardcoded) - Output format: code-graph-rag version X.Y.Z Changes: - Add _version_callback using app_context.console.print (highlight=False) - Add HELP_VERSION to cli_help.py - Add CLI_MSG_VERSION to constants.py - Use ch.HELP_VERSION and cs.CLI_MSG_VERSION (no hardcoded strings) - Add test_version_flag() to test_cli_smoke.py referencing cs.CLI_MSG_VERSION
8aacac7 to
66810a3
Compare
Implements cgr --version and cgr -v commands that display the current package version read from importlib.metadata.
Acceptance criteria from issue #239:
Test: added test_version_flag() to test_cli_smoke.py (subprocess pattern)
Summary
Type of Change
Related Issues
Test Plan
make test-paralleloruv run pytest -n auto -m "not integration")make test-integration, requires Docker)Checklist
make pre-commit)# type: ignore,cast(),Any, orobjecttype hints