-
-
Notifications
You must be signed in to change notification settings - Fork 52
feat: implement smart update recommendations #667
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: implement smart update recommendations #667
Conversation
📝 WalkthroughWalkthroughAdds a Smart Update Recommendations feature: new CLI subcommand Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/CLI
participant CLI as cortex/cli.py
participant Recommender as UpdateRecommender
participant PkgMgr as Package Manager(s)
participant LLM as LLMRouter
participant Output as Console/JSON
User->>CLI: cortex update recommend [--json|--no-llm]
CLI->>Recommender: instantiate (llm_router?)
Recommender->>PkgMgr: get_installed_packages()
PkgMgr-->>Recommender: installed packages & versions
Recommender->>PkgMgr: get_available_updates()
PkgMgr-->>Recommender: available updates + changelogs
Recommender->>Recommender: analyze_change_type(), assess_risk(), categorize_update(), group_updates()
alt use_llm
Recommender->>LLM: analyze_with_llm(updates)
LLM-->>Recommender: llm_analysis_text
end
Recommender-->>CLI: UpdateRecommendation (struct)
CLI->>Output: format as JSON or rich panels
Output-->>User: recommendations output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Summary of ChangesHello @pratyush07-hub, 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 enhances the Cortex system by integrating an intelligent update recommendation engine. This system proactively analyzes available package updates, assesses their potential risks, and provides actionable guidance on the optimal timing for their application. The goal is to empower users with better decision-making capabilities regarding system maintenance, ensuring stability while keeping software up-to-date. Highlights
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces the Smart Update Recommendations System, a valuable new feature that provides AI-powered guidance for system and package updates. The implementation includes a new core module (cortex/update_recommender.py), CLI integration, and comprehensive documentation updates. The system categorizes updates by risk, analyzes version changes, and can leverage LLM support for deeper insights. Overall, the feature is well-designed and adds significant value to Cortex Linux.
I am having trouble creating individual review comments. Click here to see my feedback.
cortex/update_recommender.py (294-303)
Retrieving the current version for each package by running pm info installed name inside the loop is inefficient. For DNF/YUM systems, it would be more efficient to fetch all installed package versions once (similar to dpkg-query) and then look up the current version from that cached data. This avoids multiple subprocess.run calls.
cortex/cli.py (1953-1954)
Catching a generic Exception here is too broad and can mask underlying issues. It's better to catch more specific exceptions that might occur during the import or instantiation of LLMRouter, such as ImportError or ModuleNotFoundError.
except (ImportError, ModuleNotFoundError):
pass
cortex/cli.py (1963)
The overall_risk is currently outputting its integer value (e.g., 1, 2, 3, 4). However, the documentation in docs/SMART_UPDATE_RECOMMENDATIONS.md shows a string representation (e.g., "medium"). To maintain consistency with the documentation and provide a more human-readable output, please use overall_risk.value_str.
"overall_risk": recommendation.overall_risk.value_str,
cortex/cli.py (1959-2008)
The manual conversion of the UpdateRecommendation dataclass to a JSON-serializable dictionary is quite verbose and prone to errors if the dataclass structure changes. Consider using a library like dataclasses-json to automatically handle this serialization, which would make the code more concise and maintainable.
Alternatively, you could implement a to_dict method within the UpdateRecommendation and UpdateInfo dataclasses to encapsulate this serialization logic.
cortex/update_recommender.py (257-263)
The RPM package name parsing parts[0].rsplit(".", 1)[0] might be overly aggressive. If a package name legitimately contains a dot (e.g., python.requests), this logic would incorrectly truncate the name. It's generally safer to assume the first part is the package name unless there's a specific reason to split by a dot.
name, new_ver = parts[0], parts[1]cortex/update_recommender.py (300-301)
The parsing logic l.split(":", 1)[1].strip() for extracting the version from dnf/yum info output is fragile. It assumes the line starts with "Version" and has a colon followed by the version. A more robust approach would be to use a regular expression to extract the version, or to parse the output more carefully, as the format might vary slightly across different RPM-based distributions or versions of dnf/yum.
cortex/update_recommender.py (521-522)
Catching a generic Exception here is too broad. It's better to catch specific exceptions that might occur during the LLM API call, such as APIError from the LLM client library or network-related exceptions. A broad catch can hide other programming errors.
cortex/update_recommender.py (755-756)
Catching a generic Exception here is too broad. Consider catching more specific exceptions that UpdateRecommender methods might raise, such as RuntimeError for LLM issues or subprocess.CalledProcessError for package manager command failures. This improves error handling clarity.
…ve RPM name resolution (formatted)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@cortex/update_recommender.py`:
- Around line 701-717: get_recommendations is calling
is_security_update(pkg_name) with only the package name, so security signals in
changelog/description are never considered; update the loop that builds
UpdateInfo (around PackageVersion.parse, analyze_change_type, assess_risk,
is_security_update) to populate and pass the package description/changelog and
repo/channel metadata into is_security_update and assess_risk (and include those
fields when constructing UpdateInfo) — if metadata isn't present on the update
tuple, fetch the changelog/description from the package source or repo/channel
API before calling is_security_update and assess_risk so security updates (e.g.,
*-security origins) are detected.
- Around line 389-398: The public method analyze_change_type lacks a docstring;
add a concise docstring above analyze_change_type describing what the function
does (determine MAJOR/MINOR/PATCH/UNKNOWN based on PackageVersion comparison),
list the parameters (current: PackageVersion, new: PackageVersion) and their
meaning, and document the return value (ChangeType) including the tie-breaker
behavior for alphanumeric patch differences; ensure the docstring follows the
project's docstring style (one-line summary plus short parameter/return
descriptions).
- Around line 450-456: The _map_score_to_risk function currently uses incorrect
thresholds; update its logic so that scores >=35 return RiskLevel.HIGH, scores
>=15 (and <35) return RiskLevel.MEDIUM, and anything below 15 returns
RiskLevel.LOW; modify the function _map_score_to_risk in update_recommender.py
to use these thresholds (referencing the RiskLevel enum) so the implementation
matches the documented ranges.
- Around line 320-338: In _get_apt_updates, the tuple returned from the APT
regex currently uses the capture order (package, new_version, old_version) which
mismatches the rest of the code that expects (package, old_version, new_version)
(see RPM parser behavior); change the appended tuple from match.groups() to
reorder the captures to (match.group(1), match.group(3), match.group(2)) so
_get_apt_updates returns (name, old_ver, new_ver) consistently with the RPM
parser, and when self._run_pkg_cmd(["apt-get", "update", "-q"]) returns None,
log a warning (using the existing logger) instead of silently returning an empty
list so failures are visible.
In `@docs/SMART_UPDATE_RECOMMENDATIONS.md`:
- Around line 24-53: The fenced example output block under the "### Example
Output" header is missing a language tag, causing markdownlint MD040; update the
fenced code block delimiter to include a language (e.g., change ``` to ```text
or ```console) so the block becomes a labeled code fence and satisfies the
linter while preserving the existing content.
🧹 Nitpick comments (1)
cortex/i18n/locales/en.yaml (1)
487-521: Document interpolation variables for new messages.
Add comments for{risk},{count},{current},{new},{group},{name}per the header guidance in this file.
|
CLA Verification PassedAll contributors have signed the CLA.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cortex/update_recommender.py (1)
525-531: CRITICAL risk is currently unreachable and would be miscategorized.
RiskLevel.CRITICALis defined but never returned by_map_score_to_risk, andcategorize_updateonly defersHIGH. This leaves CRITICAL unused today and mis-handled if thresholds change later. Consider mapping a top band to CRITICAL and deferring CRITICAL updates explicitly.♻️ Suggested adjustment
def _map_score_to_risk(self, score: int) -> RiskLevel: """Map aggregate risk score to RiskLevel enum.""" + if score >= 70: + return RiskLevel.CRITICAL if score >= 35: return RiskLevel.HIGH if score >= 15: return RiskLevel.MEDIUM return RiskLevel.LOW# High risk or major updates should be deferred - if risk_level == RiskLevel.HIGH or change_type == ChangeType.MAJOR: + if risk_level in (RiskLevel.HIGH, RiskLevel.CRITICAL) or change_type == ChangeType.MAJOR: return UpdateCategory.DEFERREDAlso applies to: 646-649
… add unit tests Populate changelog data for breaking-change detection in _get_package_metadata and support DNF/YUM fallback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@cortex/update_recommender.py`:
- Around line 277-309: The __init__ method of the Update Recommender class lacks
an explicit return type; update the constructor signature for the method named
__init__ in this file to include the explicit annotation "-> None" (i.e., change
"def __init__(...):" to "def __init__(...) -> None:") so it satisfies the
type-hint requirements, leaving the body and existing exception handling
(references to InstallationHistory, ContextMemory, logger, self.history,
self.memory, and self.llm_router) unchanged.
- Around line 69-167: PackageVersion.parse currently only detects prereleases
prefixed by - or +, so Debian-style tilde (~) prereleases like "1.2.3~rc1" are
missed; update the prerelease detection in PackageVersion.parse (the pr_match
regex) to include "~" (e.g., change r"[-+](alpha|beta|rc|dev|pre)[\d.]*" to
include "~") and ensure it remains case-insensitive and still uses
pr_match.group(0) for prerelease; no other logic changes required because
core_ver already strips suffixes using the same [-+~] set.
…e hints - Update PackageVersion.parse to support ~ markers\n- Add explicit -> None to UpdateRecommender.__init__\n- Add unit test for ~ version markers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cortex/update_recommender.py`:
- Around line 441-463: The _parse_rpm_check_update function is currently
treating metadata/header lines as packages; add a guard after splitting the line
to skip any line whose version token (parts[1]) contains no digits (e.g., using
a digit check like any(char.isdigit() for char in parts[1])) so only real
package-version lines are processed; ensure this check is applied before calling
_resolve_rpm_name and before constructing the updates list to prevent bogus
update entries.
🧹 Nitpick comments (1)
tests/test_update_recommender.py (1)
69-72: Add a return type hint on the fixture to align with type-hint rules.A small annotation keeps tests consistent with the repo-wide type-hint requirement.
♻️ Suggested tweak
`@pytest.fixture` -def r(self): +def r(self) -> UpdateRecommender: return UpdateRecommender(verbose=True)As per coding guidelines, add type hints for Python code.
… parsing Ignore lines in dnf/yum check-update output that do not contain a digit in the version field (e.g., metadata expiration notices).
|
@Anshgrover23 |
|
Tip For best results, initiate chat on the files or code changes.
✅ Performance optimization for Fedora/RHEL with bulk update fetching These changes significantly enhance the Smart Update Recommendations feature. Great work on implementing the fixes! 🎉 |
@pratyush07-hub Please always write and communicate in professional English with correct grammar. |
Thank you for the feedback. I will address the remaining issues and update the pull request accordingly. |
|
Tip For best results, initiate chat on the files or code changes.
✅ Performance optimization for Fedora/RHEL with bulk update fetching These changes significantly enhance the Smart Update Recommendations feature. I'll defer to |
Anshgrover23
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pratyush07-hub Also in video testing is fragile. Kindly tests using real packages if u can ?
…meouts, and logging
9e289ec to
121746f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@cortex/cli.py`:
- Around line 2627-2654: Wrap the JSON branch of the "recommend" action so any
exception from UpdateRecommender initialization or
recommender.get_recommendations() is caught and always prints a JSON error
object and exits non‑zero; specifically, around the UpdateRecommender(...) and
recommender.get_recommendations(use_llm=use_llm) calls catch Exception as e and
print json.dumps({"error": str(e), "type": e.__class__.__name__}) (or similar)
then return a non‑zero code. Also ensure LLMRouter import/initialization errors
remain captured (LLMRouter) but still yield the same JSON error format when they
bubble up from UpdateRecommender or get_recommendations.
In `@cortex/update_recommender.py`:
- Around line 349-356: The _run_pkg_cmd helper always uses self.timeout, causing
apt-get update and apt list checks to falsely report "no update" when they
exceed the default 30s; modify _run_pkg_cmd to accept an optional timeout
parameter (e.g., timeout: int | None = None) and use that value or fall back to
self.timeout, update callers that perform APT operations (the calls that run
"apt-get update" and "apt list") to pass timeout=self.check_timeout (120s) so
those slow APT commands use the longer timeout while keeping existing behavior
for other package managers.
🧹 Nitpick comments (1)
tests/test_update_recommender.py (1)
7-24: Add type hints to test function signatures for consistency.
Repo guidelines require Python type annotations; consider annotating test params (e.g.,r: UpdateRecommender,mock_run: MagicMock) and adding-> None. As per coding guidelines, please add type hints to Python code.
I agree that testing with real packages is more reliable than using mocks. I tested the system using a real OS scan, but since the environment is already fully updated, it correctly reports that everything is healthy. To show that the logic is not fragile and can handle real-world complexity, I did the following:
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cortex/update_recommender.py`:
- Around line 432-438: get_available_updates currently treats an empty list from
_get_apt_updates the same as "APT not available" and falls back to RPM; change
the logic so _get_apt_updates returns None (or another sentinel) when APT is
unavailable but returns [] when APT is available with no updates, then update
get_available_updates to only call _get_rpm_updates when the apt_result is None
(i.e., APT not present). Refer to get_available_updates, _get_apt_updates, and
_get_rpm_updates when making the change.
🧹 Nitpick comments (1)
cortex/update_recommender.py (1)
538-571: Consider separating “risk notes” from actual breaking changes.
assess_risk()returns general warnings (e.g., pre-release, high-impact package) but these are stored inUpdateInfo.breaking_changesand rendered as “potential breaking changes,” which can overstate risk. Splitting warnings intorisk_notesvs.breaking_changeswould keep the messaging accurate.Also applies to: 727-751, 846-869
| def get_available_updates(self) -> list[dict[str, Any]]: | ||
| """Get list of packages with available updates.""" | ||
| updates = self._get_apt_updates() | ||
| if updates: | ||
| return updates | ||
|
|
||
| return self._get_rpm_updates() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid RPM fallback when APT is present but has no updates.
Right now an empty APT update list triggers RPM checks, which can add unnecessary work and (on mixed systems) surface unrelated RPM updates. Consider distinguishing “APT not available” from “APT has zero updates.”
🔧 Suggested fix
+import shutil
@@
- def get_available_updates(self) -> list[dict[str, Any]]:
+ def get_available_updates(self) -> list[dict[str, Any]]:
"""Get list of packages with available updates."""
- updates = self._get_apt_updates()
- if updates:
- return updates
-
- return self._get_rpm_updates()
+ if shutil.which("apt-get") and shutil.which("apt"):
+ return self._get_apt_updates()
+ return self._get_rpm_updates()🤖 Prompt for AI Agents
In `@cortex/update_recommender.py` around lines 432 - 438, get_available_updates
currently treats an empty list from _get_apt_updates the same as "APT not
available" and falls back to RPM; change the logic so _get_apt_updates returns
None (or another sentinel) when APT is unavailable but returns [] when APT is
available with no updates, then update get_available_updates to only call
_get_rpm_updates when the apt_result is None (i.e., APT not present). Refer to
get_available_updates, _get_apt_updates, and _get_rpm_updates when making the
change.



Related Issue
Closes #91
Summary
This PR introduces the Smart Update Recommendations System, which provides predictive, risk-aware guidance for system and package updates before they are applied.
The system analyzes installed packages, evaluates available updates, and recommends when and how updates should be applied to minimize instability and breaking changes.
Key Features
Smart Update Recommendations
Categorizes updates as Security, Immediate, Scheduled, or Deferred based on risk.
Risk & Impact Analysis
Evaluates updates using version changes, package sensitivity, and changelog keywords, with optional LLM support.
Optional LLM Integration
Predicts breaking changes, groups related updates, and explains recommendations. Works fully without an LLM.
New CLI Command
cortex update recommendComprehensive Documentation
Adds a dedicated feature guide in
docs/SMART_UPDATE_RECOMMENDATIONS.mdand updates existing user and developer guides.Test Suite
Includes 25+ unit tests in
tests/test_update_recommender.pycovering version parsing, risk scoring, and update categorization logic.Demonstration
screencast-from-2026-01-23-02-00-48_IQ5A2AK3.1.mp4
Files Changed
Core
Tests
Docs
AI Disclosure
Used Claude Opus 4.5 (Antigravity Coding Assistant) :
Checklist
type(scope): descriptionor[scope] descriptionpytest tests/)Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.