Add changelog CLI command for viewing package release notes #337
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds a changelog subsystem under Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI as CortexCLI
participant Fetcher as fetchers.fetch_changelog
participant Parser as parser.parse_changelog
participant Formatter as formatter.format_changelog
participant Stdout as stdout
User->>CLI: run "cortex changelog docker"
CLI->>Fetcher: fetch_changelog("docker")
Fetcher-->>CLI: [entry1, entry2]
loop per entry
CLI->>Parser: parse_changelog(entry)
Parser-->>CLI: parsed{version,date,security,bugs,features}
CLI->>Formatter: format_changelog(parsed)
Formatter-->>CLI: formatted_text
CLI->>Stdout: print(formatted_text)
end
alt entries found
Stdout-->>User: formatted changelogs
CLI-->>User: exit 0
else no entries
Stdout-->>User: "No changelog found"
CLI-->>User: exit 1
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
cortex/changelog/__init__.pycortex/changelog/comparer.pycortex/changelog/exporter.pycortex/changelog/fetchers.pycortex/changelog/formatter.pycortex/changelog/parser.pycortex/changelog/security.pycortex/cli.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
cortex/changelog/comparer.pycortex/changelog/fetchers.pycortex/changelog/formatter.pycortex/changelog/parser.pycortex/cli.pycortex/changelog/security.pycortex/changelog/exporter.py
🧬 Code graph analysis (1)
cortex/cli.py (4)
cortex/changelog/fetchers.py (1)
fetch_changelog(3-23)cortex/changelog/parser.py (1)
parse_changelog(3-23)cortex/changelog/formatter.py (1)
format_changelog(1-15)cortex/first_run_wizard.py (1)
_print_error(746-748)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (3)
cortex/cli.py (3)
792-792: LGTM! Help table entry added correctly.The changelog command is properly documented in the help table with clear usage information.
860-867: LGTM! Argument parser configured correctly.The changelog subcommand parser is properly set up with appropriate help text and required arguments.
940-942: LGTM! Command dispatch implemented correctly.The changelog command is properly wired into the main dispatch logic.
| def compare_versions(old: dict, new: dict) -> str: | ||
| lines = [] | ||
| lines.append(f"What's new in {new['version']}:") | ||
|
|
||
| if new["security"]: | ||
| lines.append(f"- {len(new['security'])} security fix(es)") | ||
| if new["bugs"]: | ||
| lines.append(f"- {len(new['bugs'])} bug fix(es)") | ||
| if new["features"]: | ||
| lines.append(f"- {len(new['features'])} new feature(s)") | ||
|
|
||
| return "\n".join(lines) |
There was a problem hiding this comment.
Add docstring and address unused parameter.
- Missing docstring: Per coding guidelines, all public APIs require docstrings.
- Unused parameter: The
oldparameter is never used. The function namecompare_versionsimplies comparing old vs new, but it only reports statistics from the new version. Either:- Remove the
oldparameter if comparison isn't needed, or - Implement actual comparison logic (e.g., showing what changed between versions)
- Remove the
🔎 Proposed fix (if comparison not needed)
-def compare_versions(old: dict, new: dict) -> str:
+def compare_versions(old: dict, new: dict) -> str:
+ """
+ Compare two changelog versions and summarize differences.
+
+ Args:
+ old: Previous version changelog (currently unused)
+ new: New version changelog
+
+ Returns:
+ Formatted string summarizing the new version
+
+ Note:
+ Currently only summarizes the new version. Full comparison logic TBD.
+ """
lines = []Alternatively, implement actual comparison to show differences between old and new versions, which aligns better with Issue #106 requirements: "Compare versions (e.g., show differences between two versions)."
🤖 Prompt for AI Agents
In cortex/changelog/comparer.py around lines 1 to 12, add a proper docstring
describing the function's purpose, parameters, return type and behavior, and
resolve the unused `old` parameter: either remove `old` from the signature if we
only report stats for `new`, or implement comparison logic that computes and
reports differences between `old` and `new` (e.g., items added/removed in
security, bugs, features and version change), update the return description
accordingly, and ensure tests and callers are updated to match the new
signature/behavior.
There was a problem hiding this comment.
Pull request overview
This PR adds a new cortex changelog <package> CLI command to view package changelogs and release notes. The feature provides a simple way to display formatted changelog entries with visual indicators for security fixes, bug fixes, and new features, without requiring LLM or API dependencies.
Key Changes
- Added changelog CLI command with parser integration and help menu entry
- Created modular changelog processing infrastructure with separate fetchers, parser, and formatter components
- Implemented change categorization system that identifies security fixes, bugs, and features
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| cortex/cli.py | Adds the new changelog command method, argument parser configuration, and help menu entry |
| cortex/changelog/fetchers.py | Implements changelog data fetching (currently with hardcoded sample data for Docker) |
| cortex/changelog/parser.py | Parses changelog entries and categorizes changes into security, bugs, and features |
| cortex/changelog/formatter.py | Formats parsed changelog data with emoji indicators for display |
| cortex/changelog/security.py | Provides helper function to check for security fixes in parsed data |
| cortex/changelog/exporter.py | Implements changelog export functionality to JSON or text files |
| cortex/changelog/comparer.py | Compares two changelog versions and summarizes differences |
| cortex/changelog/init.py | Empty module initializer for the changelog package |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,12 @@ | |||
| def compare_versions(old: dict, new: dict) -> str: | |||
There was a problem hiding this comment.
Missing docstring for the function. Add a docstring that describes the purpose, parameters (old: parsed changelog data for the old version, new: parsed changelog data for the new version), and return value (a formatted string summarizing the differences between versions).
| def compare_versions(old: dict, new: dict) -> str: | |
| def compare_versions(old: dict, new: dict) -> str: | |
| """Compare two parsed changelog entries and summarize new changes. | |
| Parameters | |
| ---------- | |
| old : dict | |
| Parsed changelog data for the old version. | |
| new : dict | |
| Parsed changelog data for the new version. | |
| Returns | |
| ------- | |
| str | |
| A human-readable, formatted string summarizing the differences | |
| between versions, highlighting new security fixes, bug fixes, | |
| and features introduced in the new version. | |
| """ |
|
I am currently working on this issue and will update this thread as I progress. - effiti |
|
@Suyashd999 can you review this? |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
cortex/cli.py (3)
29-45: Docstring and missing features already flagged; consider error handling.Previous reviews correctly identified:
- Missing docstring for this public method (required per coding guidelines)
- Unused changelog modules (security.py, comparer.py, exporter.py) and missing Issue #106 features (search, compare, export)
Additionally, consider wrapping the fetch/parse/format calls in exception handling to gracefully handle unexpected errors from the changelog subsystem.
🔎 Add basic error handling
def changelog(self, package: str) -> int: + """ + Display changelog entries for a package. + + Args: + package: Package name to fetch changelog for + + Returns: + 0 on success, 1 if no changelog found or error + """ from cortex.changelog.fetchers import fetch_changelog from cortex.changelog.parser import parse_changelog from cortex.changelog.formatter import format_changelog - entries = fetch_changelog(package) - - if not entries: - self._print_error(f"No changelog found for package: {package}") - return 1 - - for entry in entries: - parsed = parse_changelog(entry) - print(format_changelog(parsed)) - print() - - return 0 + try: + entries = fetch_changelog(package) + + if not entries: + self._print_error(f"No changelog found for package: {package}") + return 1 + + for entry in entries: + parsed = parse_changelog(entry) + print(format_changelog(parsed)) + print() + + return 0 + except Exception as e: + self._print_error(f"Failed to retrieve changelog: {e}") + return 1
1114-1115: Extra blank line flagged previously.Copilot already noted the extra blank line on line 1115 for consistency with other command entries in the help table.
1370-1373: Dispatch logic correct; extra blank lines already flagged.The changelog command dispatch is correctly implemented. Copilot previously flagged the extra blank lines on 1372-1373 for consistency.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cortex/cli.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
cortex/cli.py
🧠 Learnings (1)
📚 Learning: 2025-12-11T12:03:24.071Z
Learnt from: CR
Repo: cortexlinux/cortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T12:03:24.071Z
Learning: Applies to **/*install*.py : Implement audit logging to ~/.cortex/history.db for all package operations
Applied to files:
cortex/cli.py
🧬 Code graph analysis (1)
cortex/cli.py (4)
cortex/changelog/fetchers.py (1)
fetch_changelog(3-23)cortex/changelog/parser.py (1)
parse_changelog(3-23)cortex/changelog/formatter.py (1)
format_changelog(1-15)cortex/first_run_wizard.py (1)
_print_error(746-748)
🔇 Additional comments (1)
cortex/cli.py (1)
1206-1213: LGTM!The changelog subparser is correctly configured with appropriate help text and follows the established pattern for other commands.
There was a problem hiding this comment.
@SWAROOP323 Can you address coderabbitai comments.
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
@SWAROOP323 CI Checks are failing, Also can u raise a new PR with the CLA Information following #401 PR as a reference.
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
@SWAROOP323 Update your branch.
CLA Verification PassedAll contributors have signed the CLA.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
cortex/cli.py (1)
35-51: Add docstring and integrate or remove unused changelog modules.
Missing docstring: Per coding guidelines, all public APIs require docstrings. This was flagged in previous reviews but remains unaddressed.
Unused modules: The PR introduces
security.py(has_security_fixes),comparer.py(compare_versions), andexporter.py(export_changelog), but none are imported or used here. Issue #106 specifies:
- Search functionality through changelogs
- Compare versions (
cortex changelog compare <package> <v1> <v2>)- Export changelogs
- Highlight security fixes
Consider integrating these features to meet the full requirements, or remove the unused code if deferring to a future PR.
🔎 Add docstring
def changelog(self, package: str) -> int: + """ + Display changelog entries for a package. + + Args: + package: Package name to fetch changelog for + + Returns: + 0 on success, 1 if no changelog found + """ from cortex.changelog.fetchers import fetch_changelogcortex/changelog/exporter.py (1)
4-10: Address all previous review feedback: add docstring, return type, error handling, and fix text export format.The function still has the same four issues flagged in previous reviews:
- Missing docstring: Per coding guidelines, all public APIs require docstrings describing purpose, parameters, return value, and exceptions.
- Missing return type hint: Should be
-> None.- No error handling: File operations can raise
IOError,PermissionError, orOSError. Wrap in try/except and provide context.- Unclear text format: Line 10 uses
str(data), which produces Python dict representation (e.g.,{'key': 'value'}), not human-readable text. Use JSON for both branches or implement proper text formatting.🔎 Proposed fix addressing all four issues
def export_changelog(data: dict, filename: str) -> None: + """ + Export changelog data to a file. + + Args: + data: Dictionary containing changelog data + filename: Output filename (.json for JSON format, others for plain text) + + Raises: + IOError: If file cannot be written + """ + try: - if filename.endswith(".json"): - with open(filename, "w") as f: - json.dump(data, f, indent=2) - else: - with open(filename, "w") as f: - f.write(str(data)) + with open(filename, "w") as f: + # Use JSON for both branches for readability + json.dump(data, f, indent=2) + except (IOError, OSError) as e: + raise IOError(f"Failed to write changelog to {filename}: {e}") from e
🧹 Nitpick comments (1)
cortex/cli.py (1)
35-51: Add test coverage for the changelog command.The repository has comprehensive testing for other CLI commands in
tests/test_cli.py, but the new changelog command lacks test coverage. This was noted in previous reviews.Consider adding tests to cover:
- Successful changelog retrieval and display
- Handling packages with no changelog found (return code 1)
- Parsing and formatting of different entry types (security, bugs, features)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cortex/changelog/exporter.pycortex/changelog/fetchers.pycortex/changelog/parser.pycortex/cli.py
🚧 Files skipped from review as they are similar to previous changes (2)
- cortex/changelog/fetchers.py
- cortex/changelog/parser.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
cortex/changelog/exporter.pycortex/cli.py
🧠 Learnings (1)
📚 Learning: 2025-12-11T12:03:24.071Z
Learnt from: CR
Repo: cortexlinux/cortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T12:03:24.071Z
Learning: Applies to **/*install*.py : Implement audit logging to ~/.cortex/history.db for all package operations
Applied to files:
cortex/cli.py
🧬 Code graph analysis (1)
cortex/cli.py (4)
cortex/changelog/fetchers.py (1)
fetch_changelog(3-23)cortex/changelog/formatter.py (1)
format_changelog(1-15)cortex/changelog/parser.py (1)
parse_changelog(3-23)cortex/first_run_wizard.py (1)
_print_error(746-748)
🪛 GitHub Actions: CI
cortex/cli.py
[error] 1-1: Command failed: black --check . --exclude "(venv|.venv|build|dist)". Formatting would be applied to this file (would reformat).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test (3.11)
- GitHub Check: test (3.10)
- GitHub Check: test (3.12)
🔇 Additional comments (1)
cortex/cli.py (1)
1664-1671: LGTM!The argparse configuration for the changelog command is correctly structured with appropriate help text and a required package argument.
@Anshgrover23 CI failures have been resolved and the branch has been updated. |
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
@SWAROOP323 add a video, also docs file missing.
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
tests also missing. Do not attempt low-quality PR's.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
cortex/changelog/fetchers.py (2)
2-21: Stub implementation (duplicate of past review).Past reviews have already noted that this hardcoded implementation only supports "docker" and silently returns empty lists for all other packages. This doesn't meet the PR objectives (issue #106) which requires:
- Search functionality through changelogs
- Version comparison support
- Fetching from actual package repositories
The stub approach is acceptable for initial implementation but should be clearly documented with a TODO comment or raise NotImplementedError for unsupported packages.
1-21: Missing docstring (duplicate of past review).Past reviews have already flagged that this function lacks a docstring. Per coding guidelines, all public APIs require docstrings that document:
- Purpose of the function
- Parameters (package: str)
- Return value (list of changelog entry dicts with keys: version, date, changes)
- Current limitation (stub implementation supporting only "docker")
cortex/changelog/parser.py (2)
6-13: Broad keyword matching (duplicate of past review).Past reviews have already noted that the "fix" keyword on line 10 is very broad and could misclassify entries. Examples:
- "Fixed typo in documentation" → categorized as bug fix
- "Performance fix included" in a feature description → categorized as bug fix
The current if-elif structure also means a change can only belong to one category. If this is intentional prioritization (security > bugs > features), it should be documented in a docstring.
1-21: Missing docstring (duplicate of past review).Past reviews have already flagged that this function lacks a docstring. Per coding guidelines, all public APIs require docstrings that document:
- Purpose of the function
- Parameters (entry: dict with version, date, changes keys)
- Return value (dict with version, date, security, bugs, features)
- Categorization logic (keyword-based classification)
🧹 Nitpick comments (1)
cortex/changelog/fetchers.py (1)
1-1: Consider more specific type hints for dict contents.The return type
list[dict]is vague about the dictionary structure. Consider usinglist[dict[str, Any]]or defining a TypedDict to document the expected keys (version, date, changes) and their value types. This improves type safety and IDE autocomplete.🔎 Example with more specific types
+from typing import Any + -def fetch_changelog(package: str) -> list[dict]: +def fetch_changelog(package: str) -> list[dict[str, Any]]:Or even better with TypedDict:
from typing import TypedDict class ChangelogEntry(TypedDict): version: str date: str changes: list[str] def fetch_changelog(package: str) -> list[ChangelogEntry]:
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cortex/changelog/fetchers.pycortex/changelog/parser.pycortex/cli.pydocs/changelog.mdtests/test_changelog_cli.py
✅ Files skipped from review due to trivial changes (1)
- docs/changelog.md
🚧 Files skipped from review as they are similar to previous changes (1)
- cortex/cli.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
cortex/changelog/fetchers.pycortex/changelog/parser.pytests/test_changelog_cli.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain >80% test coverage for pull requests
Files:
tests/test_changelog_cli.py
| for change in entry["changes"]: | ||
| lower = change.lower() | ||
| if "cve" in lower or "security" in lower: | ||
| security.append(change) | ||
| elif "bug" in lower or "fix" in lower: | ||
| bugs.append(change) | ||
| else: | ||
| features.append(change) | ||
|
|
||
| return { | ||
| "version": entry["version"], | ||
| "date": entry["date"], | ||
| "security": security, | ||
| "bugs": bugs, | ||
| "features": features, | ||
| } |
There was a problem hiding this comment.
Add input validation to prevent KeyError.
The function assumes the input entry dict contains the keys "changes", "version", and "date". If a malformed entry is passed (e.g., from a future change to fetchers.py), this will raise a KeyError at runtime.
🔎 Proposed fix with validation
def parse_changelog(entry: dict) -> dict:
+ # Validate required keys
+ required_keys = {"version", "date", "changes"}
+ missing_keys = required_keys - entry.keys()
+ if missing_keys:
+ raise ValueError(f"Missing required keys in changelog entry: {missing_keys}")
+
security = []
bugs = []
features = []Alternatively, use .get() with defaults:
- for change in entry["changes"]:
+ for change in entry.get("changes", []):
lower = change.lower()
if "cve" in lower or "security" in lower:
security.append(change)
elif "bug" in lower or "fix" in lower:
bugs.append(change)
else:
features.append(change)
return {
- "version": entry["version"],
- "date": entry["date"],
+ "version": entry.get("version", "unknown"),
+ "date": entry.get("date", "unknown"),
"security": security,
"bugs": bugs,
"features": features,
}🤖 Prompt for AI Agents
In cortex/changelog/parser.py around lines 6 to 21, the code assumes entry
contains "changes", "version", and "date" and will raise KeyError for malformed
input; validate upfront by checking entry is a dict and that "changes" exists
(use entry.get("changes", []) and ensure it's iterable), and use
entry.get("version") and entry.get("date") with sensible defaults (e.g., None or
empty string) or raise a clear ValueError if required fields are missing; ensure
downstream code handles the defaults or the raised error.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| def test_changelog_command_runs(): | ||
| result = subprocess.run( | ||
| [sys.executable, "-m", "cortex.cli", "changelog", "docker"], | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| assert result.returncode == 0 | ||
| assert result.stdout.strip() != "" |
There was a problem hiding this comment.
Add docstring, timeout, and expand test coverage.
The test has several issues:
- Missing docstring: Per coding guidelines, docstrings are required. Add a brief description of what the test validates.
- No timeout on subprocess: The subprocess.run call lacks a timeout parameter, which could cause the test to hang indefinitely if the CLI command stalls.
- Insufficient coverage: This single happy-path test doesn't meet the >80% coverage requirement. The test should cover:
- Error cases (invalid package names)
- Empty output scenarios
- Non-zero exit codes
- Command not found errors
🔎 Proposed improvements
def test_changelog_command_runs():
+ """Test that the changelog command executes successfully for a known package."""
result = subprocess.run(
[sys.executable, "-m", "cortex.cli", "changelog", "docker"],
capture_output=True,
text=True,
+ timeout=10,
)
assert result.returncode == 0
assert result.stdout.strip() != ""
+
+
+def test_changelog_command_unknown_package():
+ """Test that the changelog command handles unknown packages gracefully."""
+ result = subprocess.run(
+ [sys.executable, "-m", "cortex.cli", "changelog", "nonexistent_package"],
+ capture_output=True,
+ text=True,
+ timeout=10,
+ )
+ # Depending on implementation, could be 0 with empty output or non-zero
+ assert result.returncode == 0
+ assert result.stdout.strip() == ""As per coding guidelines, test coverage should exceed 80%.
🤖 Prompt for AI Agents
In tests/test_changelog_cli.py around lines 5-12, add a module-level or
function-level docstring describing what the test(s) validate, and refactor the
single test into multiple tests: (1) keep a happy-path test but add a timeout to
subprocess.run (e.g., timeout=30) and assert stdout is non-empty; (2) add
test_invalid_package that runs the CLI with a known-bad package name and asserts
a non-zero returncode and that stderr contains an error message; (3) add
test_empty_output that uses monkeypatch or a helper to simulate subprocess.run
returning returncode==0 but stdout=="" and assert the code treats empty output
appropriately (fail or raise as your CLI expects); (4) add test_non_zero_exit
that simulates subprocess.run returning a non-zero code and verifies the test
asserts that as an error; and (5) add test_command_not_found that monkeypatches
subprocess.run to raise FileNotFoundError and asserts the test handles that
exception; ensure all new tests include timeouts on subprocess.run where used
and update assertions to cover stderr/stdout as needed to raise coverage above
80%.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
@SWAROOP323 Missing Demonstration video and Kindly Address Coderabbitai comments.
|
4 participants
Related Issue
Closes #106
Summary
Adds a new
cortex changelog <package>command to view package changelogs and release notes.Features
Example