-
Notifications
You must be signed in to change notification settings - Fork 19
User Preferences & Settings System Issue #26 #200
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
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a persistent user preferences system with YAML storage, validation, import/export, CLI commands for viewing/editing preferences, tests covering behavior and edge cases, documentation of the implementation, and a dependency addition for PyYAML. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CortexCLI
participant PreferencesManager
participant YAML_Store as "YAML Storage"
User->>CortexCLI: edit-pref set ai.creativity CREATIVE
CortexCLI->>CortexCLI: _get_prefs_manager() [lazy init]
CortexCLI->>PreferencesManager: set("ai.creativity","CREATIVE")
PreferencesManager->>PreferencesManager: Coerce & validate value
alt Validation success
PreferencesManager->>PreferencesManager: create backup
PreferencesManager->>YAML_Store: atomic write updated YAML
PreferencesManager-->>CortexCLI: success
CortexCLI-->>User: Preference updated
else Validation failed
PreferencesManager-->>CortexCLI: errors
CortexCLI-->>User: show validation errors
end
sequenceDiagram
participant User
participant CortexCLI
participant PreferencesManager
participant YAML_Store as "YAML Storage"
User->>CortexCLI: check-pref ai.model
CortexCLI->>CortexCLI: _get_prefs_manager() [lazy init]
CortexCLI->>PreferencesManager: get("ai.model")
PreferencesManager->>YAML_Store: read config (if not loaded)
YAML_Store-->>PreferencesManager: parsed config or defaults
PreferencesManager-->>CortexCLI: value + config metadata + validation
CortexCLI-->>User: display formatted preference and status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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.
Pull Request Overview
This PR implements a comprehensive user preferences and settings system for Cortex Linux (Issue #26), allowing users to customize behavior through CLI commands or YAML configuration files. The implementation includes 6 preference categories with validation, import/export functionality, and automatic backup. Additionally, the PR reorganizes the file structure by moving modules from root to a cortex/ directory and updating all test imports accordingly.
Key Changes:
- New user preferences system with YAML-based storage and validation
- 38 comprehensive unit tests with 100% pass rate
- File structure reorganization (modules moved to
cortex/directory)
Reviewed Changes
Copilot reviewed 13 out of 37 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_user_preferences.py | New comprehensive test suite with 38 tests covering all preference functionality |
| test/test_logging_system.py | Updated imports to use cortex.logging_system |
| test/test_llm_router.py | Updated imports and mock patches to use cortex.llm_router |
| test/test_installation_verifier.py | Updated imports to use cortex.installation_verifier |
| test/test_installation_history.py | Updated imports to use cortex.installation_history |
| test/test_error_parser.py | Updated imports to use cortex.error_parser |
| test/test_context_memory.py | Updated imports to use cortex.context_memory |
| cortex/user_preferences.py | New implementation of preferences management system |
| cortex/logging_system.py | New comprehensive logging system |
| cortex/llm_router.py | New LLM routing system for multi-provider support |
| cortex/installation_verifier.py | New installation verification system |
| cortex/installation_history.py | New installation history and rollback system |
| cortex/dependency_resolver.py | New dependency resolution system |
| pr_status.json | Reformatted from single line to pretty-printed JSON |
| issue_status.json | Reformatted from single line to pretty-printed JSON |
| docs/* | Multiple new documentation files for features and guides |
| LLM/requirements.txt | Added PyYAML dependency |
Comments suppressed due to low confidence (10)
test/test_context_memory.py:29
- Import of 'Pattern' is not used.
Import of 'Suggestion' is not used.
test/test_error_parser.py:16 - Import of 'ErrorAnalysis' is not used.
test/test_installation_history.py:20 - Import of 'PackageSnapshot' is not used.
Import of 'InstallationRecord' is not used.
test/test_installation_verifier.py:16 - Import of 'VerificationTest' is not used.
test/test_llm_router.py:19 - Import of 'tempfile' is not used.
test/test_llm_router.py:22 - Import of 'Path' is not used.
test/test_llm_router.py:23 - Import of 'MagicMock' is not used.
test/test_llm_router.py:24 - Import of 'datetime' is not used.
test/test_llm_router.py:35 - Import of 'RoutingDecision' is not used.
test/test_llm_router.py:18 - This import of module unittest is redundant, as it was previously imported on line 10.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
security fail of sonar is not for my changed files, thats for |
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
🧹 Nitpick comments (4)
cortex/cli.py (2)
289-420: Config subcommand implementation is solid; a few polish opportunitiesThe
config()handler correctly delegates toPreferencesManagerfor:
list→list_all()+get_config_info()get/set→get()/set()+save()reset(all or key) →reset()validate→validate()info→get_config_info()export/import→export_json()/import_json()and wraps operations in a
try/except ValueErrorblock to surface schema issues cleanly. That aligns well with the manager API.Minor refactors you might consider:
- For
get, relying onvalue is Noneto detect “missing key” ties behavior to current schemas (where no preference is legitimatelyNone). If you later add a nullable pref, this will misclassify it; an explicitdefaultsentinel could make this more robust.- For
export/import, you can reuse the module-levelPathimport instead of re-importing inside the method.- For
import, a more specific error message for a missing file (e.g.,FileNotFoundError) would be nicer than the generic “Configuration error: ...”.Functionally this is fine as-is; these are just cleanups.
401-420: Value parsing is adequate for current keys; consider extending if you add numeric fields
_parse_config_value()covers booleans, integers, simple comma-separated lists, and strings, which matches the current schema (bool/int/list/string fields inPreferencesManager.set()).If you later introduce float-valued preferences (e.g., temperatures or thresholds), you’ll likely want to extend this helper with a float branch before falling back to string, and perhaps tighten the “list if contains comma” rule for keys known to expect lists.
docs/USER_PREFERENCES_IMPLEMENTATION.md (1)
50-54: Minor Markdown polish (languages and bare URLs)A few small doc cleanups you may want to do:
- Add explicit languages to fenced code blocks that are shell or tree listings, e.g.:
```bash cortex config list~/.config/cortex/preferences.yamlcortex/ ├── user_preferences.py ...- Optionally wrap bare URLs in Markdown link syntax (`[text](url)`) to satisfy MD034, though the current style is still readable. These are non-functional but will keep markdownlint quiet. Also applies to: 356-365, 721-723 </blockquote></details> <details> <summary>cortex/user_preferences.py (1)</summary><blockquote> `142-244`: **Persistence and backup behavior are reasonable; error wrapping could preserve original tracebacks** `PreferencesManager`: - Ensures the config directory exists in `__init__`/`_ensure_config_directory`. - Creates timestamped backups via `_create_backup()` when saving over an existing file. - Handles missing and empty config files in `load()`, creating defaults when needed. - Differentiates YAML parse errors (`ValueError`) from other IO problems (`IOError`). Functionally this is sound. To aid debugging and align with modern style, you might tweak the exception wrapping to preserve the original traceback, e.g.: ```diff - except Exception as e: - raise IOError(f"Failed to create backup: {str(e)}") + except Exception as exc: + raise IOError(f"Failed to create backup: {exc}") from exc ... - except Exception as e: - raise IOError(f"Failed to load config file: {str(e)}") + except Exception as exc: + raise IOError(f"Failed to load config file: {exc}") from exc ... - except Exception as e: - raise IOError(f"Failed to save config file: {str(e)}") + except Exception as exc: + raise IOError(f"Failed to save config file: {exc}") from excThis keeps your custom messages while retaining the original stack context.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
LLM/requirements.txt(1 hunks)cortex/cli.py(11 hunks)cortex/user_preferences.py(1 hunks)docs/USER_PREFERENCES_IMPLEMENTATION.md(1 hunks)issue_status.json(1 hunks)pr_status.json(1 hunks)test/test_context_memory.py(1 hunks)test/test_error_parser.py(1 hunks)test/test_installation_history.py(1 hunks)test/test_installation_verifier.py(1 hunks)test/test_llm_router.py(10 hunks)test/test_logging_system.py(1 hunks)test/test_user_preferences.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
test/test_user_preferences.py (1)
cortex/user_preferences.py (23)
PreferencesManager(130-474)UserPreferences(86-127)VerbosityLevel(21-26)AICreativity(29-33)ConfirmationSettings(37-45)AutoUpdateSettings(49-56)AISettings(60-70)PackageSettings(74-82)to_dict(44-45)to_dict(55-56)to_dict(69-70)to_dict(81-82)to_dict(97-108)from_dict(111-127)load(186-212)save(214-243)get(245-268)reset(337-366)validate(368-401)export_json(403-419)import_json(421-442)get_config_info(444-462)list_all(464-474)
cortex/cli.py (1)
cortex/user_preferences.py (11)
PreferencesManager(130-474)VerbosityLevel(21-26)load(186-212)list_all(464-474)get_config_info(444-462)get(245-268)save(214-243)reset(337-366)validate(368-401)export_json(403-419)import_json(421-442)
cortex/user_preferences.py (1)
cortex/cli.py (1)
main(423-502)
test/test_logging_system.py (1)
cortex/logging_system.py (2)
CortexLogger(90-453)LogContext(456-475)
🪛 LanguageTool
docs/USER_PREFERENCES_IMPLEMENTATION.md
[style] ~737-~737: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...** 1.0 Last Updated: November 18, 2025 Author: Development Team **Revi...
(MISSING_COMMA_AFTER_YEAR)
🪛 markdownlint-cli2 (0.18.1)
docs/USER_PREFERENCES_IMPLEMENTATION.md
51-51: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
356-356: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
721-721: Bare URL used
(MD034, no-bare-urls)
722-722: Bare URL used
(MD034, no-bare-urls)
723-723: Bare URL used
(MD034, no-bare-urls)
🪛 Ruff (0.14.5)
test/test_user_preferences.py
1-1: Shebang is present but file is not executable
(EXE001)
390-390: Local variable prefs1 is assigned to but never used
Remove assignment to unused variable prefs1
(F841)
391-391: Local variable prefs2 is assigned to but never used
Remove assignment to unused variable prefs2
(F841)
412-412: Local variable manager is assigned to but never used
Remove assignment to unused variable manager
(F841)
cortex/cli.py
30-31: try-except-pass detected, consider logging the exception
(S110)
30-30: Do not catch blind exception: Exception
(BLE001)
45-46: try-except-pass detected, consider logging the exception
(S110)
45-45: Do not catch blind exception: Exception
(BLE001)
397-397: Do not catch blind exception: Exception
(BLE001)
398-398: Use explicit conversion flag
Replace with conversion flag
(RUF010)
cortex/user_preferences.py
1-1: Shebang is present but file is not executable
(EXE001)
182-182: Consider moving this statement to an else block
(TRY300)
183-183: Do not catch blind exception: Exception
(BLE001)
184-184: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
184-184: Avoid specifying long messages outside the exception class
(TRY003)
184-184: Use explicit conversion flag
Replace with conversion flag
(RUF010)
207-207: Consider moving this statement to an else block
(TRY300)
210-210: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
210-210: Avoid specifying long messages outside the exception class
(TRY003)
210-210: Use explicit conversion flag
Replace with conversion flag
(RUF010)
211-211: Do not catch blind exception: Exception
(BLE001)
212-212: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
212-212: Avoid specifying long messages outside the exception class
(TRY003)
212-212: Use explicit conversion flag
Replace with conversion flag
(RUF010)
225-225: Avoid specifying long messages outside the exception class
(TRY003)
240-240: Consider moving this statement to an else block
(TRY300)
242-242: Do not catch blind exception: Exception
(BLE001)
243-243: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
243-243: Avoid specifying long messages outside the exception class
(TRY003)
243-243: Use explicit conversion flag
Replace with conversion flag
(RUF010)
290-290: Abstract raise to an inner function
(TRY301)
290-290: Avoid specifying long messages outside the exception class
(TRY003)
295-295: Abstract raise to an inner function
(TRY301)
295-295: Avoid specifying long messages outside the exception class
(TRY003)
297-297: Abstract raise to an inner function
(TRY301)
297-297: Avoid specifying long messages outside the exception class
(TRY003)
302-302: Abstract raise to an inner function
(TRY301)
302-302: Avoid specifying long messages outside the exception class
(TRY003)
304-304: Abstract raise to an inner function
(TRY301)
304-304: Avoid specifying long messages outside the exception class
(TRY003)
306-306: Abstract raise to an inner function
(TRY301)
306-306: Avoid specifying long messages outside the exception class
(TRY003)
311-311: Abstract raise to an inner function
(TRY301)
311-311: Avoid specifying long messages outside the exception class
(TRY003)
314-314: Abstract raise to an inner function
(TRY301)
314-314: Avoid specifying long messages outside the exception class
(TRY003)
316-316: Abstract raise to an inner function
(TRY301)
316-316: Avoid specifying long messages outside the exception class
(TRY003)
321-321: Abstract raise to an inner function
(TRY301)
321-321: Avoid specifying long messages outside the exception class
(TRY003)
323-323: Abstract raise to an inner function
(TRY301)
323-323: Avoid specifying long messages outside the exception class
(TRY003)
330-330: Abstract raise to an inner function
(TRY301)
330-330: Avoid specifying long messages outside the exception class
(TRY003)
332-332: Consider moving this statement to an else block
(TRY300)
335-335: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
335-335: Avoid specifying long messages outside the exception class
(TRY003)
335-335: Use explicit conversion flag
Replace with conversion flag
(RUF010)
362-362: Avoid specifying long messages outside the exception class
(TRY003)
439-439: Avoid specifying long messages outside the exception class
(TRY003)
482-482: Local variable prefs is assigned to but never used
Remove assignment to unused variable prefs
(F841)
test/test_llm_router.py
18-18: Redefinition of unused unittest from line 10
Remove definition: unittest
(F811)
23-23: Redefinition of unused patch from line 11
Remove definition: patch
(F811)
23-23: Redefinition of unused MagicMock from line 11
Remove definition: MagicMock
(F811)
🔇 Additional comments (17)
test/test_error_parser.py (1)
7-16: LGTM! Import namespace updated correctly.The sys.path adjustment and import changes properly align the test with the new cortex package structure.
pr_status.json (1)
1-93: LGTM! Formatting improvement.The JSON array has been reformatted to a more readable pretty-printed format with no data changes.
test/test_installation_verifier.py (1)
7-16: LGTM! Import namespace updated correctly.The changes align with the cortex package restructuring and are consistent with other test files.
issue_status.json (1)
1-646: LGTM! Formatting and reordering change.The JSON array has been reordered and reformatted without any data modifications.
test/test_installation_history.py (1)
9-20: LGTM! Import namespace updated correctly.The changes properly establish the cortex package namespace for this test file.
test/test_logging_system.py (1)
11-12: LGTM! Import namespace updated correctly.The changes align with the package restructuring and are consistent with other test files.
test/test_context_memory.py (1)
24-29: LGTM! Import namespace updated correctly.The import changes properly align with the cortex package structure, once the duplicate path manipulation above is fixed.
LLM/requirements.txt (1)
3-3: No issues found with PyYAML>=6.0 requirement.The latest PyYAML version is 6.0.3, and the requirement
PyYAML>=6.0is both up-to-date and secure. All known critical security advisories documented in GitHub's database affect older versions (< 5.4), and PyYAML 6.0.3 is not vulnerable to any of them. The current specification is appropriate.test/test_llm_router.py (1)
257-321: Patched targets correctly updated tocortex.llm_routerAll
@patchdecorators now referencecortex.llm_router.*(Anthropic, OpenAI, LLMRouter), matching the newfrom cortex.llm_router import ...import and the package namespace used across tests. This keeps the mocks aligned with the actual module under test and avoids subtle import/patch mismatches.Also applies to: 326-392, 393-430, 435-472, 512-553
cortex/cli.py (3)
24-47: Avoid silent failures on preferences load and remove currently unused credential helpers
CortexCLI.__init__eagerly creates aPreferencesManagerand callsload()inside a bareexcept Exception: pass. This completely swallows YAML/IO/validation errors and makes diagnosing config issues harder, while the rest of the CLI currently doesn’t depend on preferences except for theconfigcommand.Also,
_cred_path()and_load_creds()are defined but not used anywhere.Possible improvements:
- self.prefs_manager = PreferencesManager() - try: - self.prefs_manager.load() - except Exception: - pass + self.prefs_manager = PreferencesManager() + try: + self.prefs_manager.load() + except Exception as exc: + # Defer surfacing to explicit config commands, but keep a hint for debugging. + self._print_status("[WARN]", f"Failed to load preferences at startup: {exc}")And either wire
_load_creds()into_get_api_key()or drop these helpers until they’re needed.[ suggest_recommended_refactor]
63-71: Status/summary printing changes look goodThe introduction of
_print_status, along with updated messages ininstall()and theprogress_callback(using[INFO],[PENDING],[SUCCESS],[FAILED]labels), improves consistency and readability of CLI output without changing behavior.Also applies to: 136-142, 157-185
469-476: Config subcommand wiring matches the new APIThe new
configsubparser and dispatch:
- Accepts
action,key, andvaluewith the expected choice set.- Routes to
CortexCLI.config(action, key, value)whenargs.command == 'config'.This matches the documented CLI usage and the
PreferencesManagerAPI surface.Also applies to: 492-493
test/test_user_preferences.py (2)
31-383: Comprehensive coverage of preferences data model and manager lifecycleThe test classes
TestUserPreferences,TestConfirmationSettings,TestAutoUpdateSettings,TestAISettings,TestPackageSettings, andTestPreferencesManagercollectively exercise:
- Default values and dataclass conversions (
to_dict/from_dict).PreferencesManager.load/save/get/set/reset/validate, including invalid keys and type errors.- Backup creation and YAML correctness.
The assertions line up with the actual defaults and validation rules in
cortex.user_preferences. This is a solid, maintainable test matrix for the core functionality.
434-476: Edge case and runner tests are well-targeted
TestEdgeCasescovers important behaviors:
- Empty config file handling.
save()without priorload()raisingRuntimeError.- Implicit loading in
set()and nested key access.The
run_tests()/__main__harness is handy for direct execution and doesn’t interfere with normal unittest discovery.Also applies to: 478-501
cortex/user_preferences.py (3)
21-127: Data model and serialization design is clean and matches usage
VerbosityLevel/AICreativityenums and theConfirmationSettings,AutoUpdateSettings,AISettings,PackageSettings, andUserPreferencesdataclasses provide a clear, type-annotated configuration model.UserPreferences.to_dict()andfrom_dict()correctly compose/decompose nested structures and default values, aligning with both tests and docs.This is a solid foundation for the rest of the preferences system.
245-336: Dot-notation access and validation logic look correctThe
get()andset()implementations:
- Correctly traverse nested structures via a dot-separated key.
- Enforce types and enums:
verbosity→VerbosityLevelvalues.confirmations.*→ bool.auto_update.frequency_hours→ int; otherauto_update.*→ bool.ai.creativity→AICreativityvalues.ai.max_suggestions→ int.packages.default_sources→ list.- Allow simple string values for
theme,language,timezone.- Wrap bad keys/values as
ValueErrorwith a helpful message.
reset()’s logic for both whole and per-key resets is consistent withUserPreferences.to_dict(), andvalidate()correctly enforces enums, allowed AI models, frequency range, and non-empty package sources.Overall this behaves as expected for the CLI and tests.
403-442: JSON import/export flows are straightforward and match tests
export_json()andimport_json():
- Serialize/deserialize via
UserPreferences.to_dict()/from_dict().- Ensure preferences are loaded before export.
- Validate after import and fail fast with a clear
ValueErrorif the imported config is invalid, before saving.That matches the documented semantics and the test expectations in
test_import_jsonandtest_import_invalid_json.
|
tried fixing security issue of sonarcloud, but that was not related to this issue and was related to |
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: 3
♻️ Duplicate comments (1)
cortex/user_preferences.py (1)
444-461: Refinewritabledetection inget_config_info()Right now
get_config_info()reports writability based on the directory only:"writable": os.access(self.config_dir, os.W_OK),This can report
Trueeven when the file itself is read-only, which slightly misleads users inspecting config health.A small refinement would be:
- info = { - "config_path": str(self.config_path), - "exists": self.config_path.exists(), - "writable": os.access(self.config_dir, os.W_OK), - } + exists = self.config_path.exists() + writable = ( + os.access(self.config_path, os.W_OK) + if exists + else os.access(self.config_dir, os.W_OK) + ) + info = { + "config_path": str(self.config_path), + "exists": exists, + "writable": writable, + }That preserves the current behavior when the file doesn’t exist but reports more accurate writability once it does.
🧹 Nitpick comments (3)
cortex/cli.py (1)
24-32: Consider logging (even at debug) when preference/credential load failsIn
__init__and_load_creds()you catchExceptionand silentlypass:try: self.prefs_manager.load() except Exception: # Silently ignore preference loading errors; defaults will be used pass ... try: p = self._cred_path() if p.exists(): return json.loads(p.read_text(encoding="utf-8")) except Exception: # Return empty dict if credentials can't be loaded (file corrupted, etc.) passSwallowing all exceptions is understandable for resilience, but it also makes debugging broken configs or corrupted credential files harder.
I’d suggest at least emitting a low-noise log/print (e.g.,
[INFO] Falling back to defaults due to config error: ...or behind an env-guarded debug mode), while still continuing with defaults/empty creds.Also applies to: 34-49
test/test_user_preferences.py (2)
15-18: Avoid mutatingsys.pathinside tests if possibleAdjusting
sys.pathin the test module can hide real packaging/import issues and makes test behavior differ from normal package usage. If your test runner is invoked from the project root with the package installed (or in editable mode), this block is typically unnecessary.-import sys -import os -sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..')) - -from cortex.user_preferences import ( +import sys +import os + +from cortex.user_preferences import (If you still need direct execution of this file, consider relying on the package being installed/editable instead of manually tweaking
sys.path.
243-275: Tests directly mutate private_preferences; consider using public surfacesThe validation tests (e.g., Lines 250–272) set invalid states via
self.manager._preferences.... This works but tightly couples the tests to the internal representation ofPreferencesManager.If you want these tests to be more future-proof, consider driving invalid states via public APIs, for example:
- Importing from JSON with deliberately bad values and then calling
validate(), or- Introducing a small helper factory in tests that builds a
UserPreferencesinstance and passes it into the manager via a dedicated setter (if you decide to expose one).Not mandatory, but it would reduce reliance on internals if
PreferencesManagerevolves.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cortex/cli.py(11 hunks)cortex/dependency_resolver.py(2 hunks)cortex/installation_history.py(2 hunks)cortex/user_preferences.py(1 hunks)test/test_context_memory.py(1 hunks)test/test_llm_router.py(10 hunks)test/test_user_preferences.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/test_llm_router.py
🧰 Additional context used
🧬 Code graph analysis (3)
cortex/cli.py (4)
LLM/interpreter.py (1)
CommandInterpreter(12-158)cortex/coordinator.py (2)
InstallationCoordinator(44-300)StepStatus(10-15)cortex/installation_history.py (3)
InstallationHistory(68-659)InstallationType(25-31)InstallationStatus(34-39)cortex/user_preferences.py (10)
PreferencesManager(130-474)load(186-212)list_all(464-474)get_config_info(444-462)get(245-268)save(214-243)reset(337-366)validate(368-401)export_json(403-419)import_json(421-442)
cortex/user_preferences.py (1)
cortex/cli.py (1)
main(425-504)
test/test_user_preferences.py (1)
cortex/user_preferences.py (23)
PreferencesManager(130-474)UserPreferences(86-127)VerbosityLevel(21-26)AICreativity(29-33)ConfirmationSettings(37-45)AutoUpdateSettings(49-56)AISettings(60-70)PackageSettings(74-82)to_dict(44-45)to_dict(55-56)to_dict(69-70)to_dict(81-82)to_dict(97-108)from_dict(111-127)load(186-212)save(214-243)get(245-268)reset(337-366)validate(368-401)export_json(403-419)import_json(421-442)get_config_info(444-462)list_all(464-474)
🪛 Ruff (0.14.5)
cortex/cli.py
30-32: try-except-pass detected, consider logging the exception
(S110)
30-30: Do not catch blind exception: Exception
(BLE001)
46-48: try-except-pass detected, consider logging the exception
(S110)
46-46: Do not catch blind exception: Exception
(BLE001)
399-399: Do not catch blind exception: Exception
(BLE001)
400-400: Use explicit conversion flag
Replace with conversion flag
(RUF010)
cortex/user_preferences.py
1-1: Shebang is present but file is not executable
(EXE001)
182-182: Consider moving this statement to an else block
(TRY300)
183-183: Do not catch blind exception: Exception
(BLE001)
184-184: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
184-184: Avoid specifying long messages outside the exception class
(TRY003)
184-184: Use explicit conversion flag
Replace with conversion flag
(RUF010)
207-207: Consider moving this statement to an else block
(TRY300)
210-210: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
210-210: Avoid specifying long messages outside the exception class
(TRY003)
210-210: Use explicit conversion flag
Replace with conversion flag
(RUF010)
211-211: Do not catch blind exception: Exception
(BLE001)
212-212: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
212-212: Avoid specifying long messages outside the exception class
(TRY003)
212-212: Use explicit conversion flag
Replace with conversion flag
(RUF010)
225-225: Avoid specifying long messages outside the exception class
(TRY003)
240-240: Consider moving this statement to an else block
(TRY300)
242-242: Do not catch blind exception: Exception
(BLE001)
243-243: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
243-243: Avoid specifying long messages outside the exception class
(TRY003)
243-243: Use explicit conversion flag
Replace with conversion flag
(RUF010)
290-290: Abstract raise to an inner function
(TRY301)
290-290: Avoid specifying long messages outside the exception class
(TRY003)
295-295: Abstract raise to an inner function
(TRY301)
295-295: Avoid specifying long messages outside the exception class
(TRY003)
297-297: Abstract raise to an inner function
(TRY301)
297-297: Avoid specifying long messages outside the exception class
(TRY003)
302-302: Abstract raise to an inner function
(TRY301)
302-302: Avoid specifying long messages outside the exception class
(TRY003)
304-304: Abstract raise to an inner function
(TRY301)
304-304: Avoid specifying long messages outside the exception class
(TRY003)
306-306: Abstract raise to an inner function
(TRY301)
306-306: Avoid specifying long messages outside the exception class
(TRY003)
311-311: Abstract raise to an inner function
(TRY301)
311-311: Avoid specifying long messages outside the exception class
(TRY003)
314-314: Abstract raise to an inner function
(TRY301)
314-314: Avoid specifying long messages outside the exception class
(TRY003)
316-316: Abstract raise to an inner function
(TRY301)
316-316: Avoid specifying long messages outside the exception class
(TRY003)
321-321: Abstract raise to an inner function
(TRY301)
321-321: Avoid specifying long messages outside the exception class
(TRY003)
323-323: Abstract raise to an inner function
(TRY301)
323-323: Avoid specifying long messages outside the exception class
(TRY003)
330-330: Abstract raise to an inner function
(TRY301)
330-330: Avoid specifying long messages outside the exception class
(TRY003)
332-332: Consider moving this statement to an else block
(TRY300)
335-335: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
335-335: Avoid specifying long messages outside the exception class
(TRY003)
335-335: Use explicit conversion flag
Replace with conversion flag
(RUF010)
362-362: Avoid specifying long messages outside the exception class
(TRY003)
439-439: Avoid specifying long messages outside the exception class
(TRY003)
test/test_user_preferences.py
1-1: Shebang is present but file is not executable
(EXE001)
🔇 Additional comments (6)
cortex/dependency_resolver.py (1)
150-152: Regex hardening for version stripping looks goodThe updated pattern
r'\s*\([^)]*\)'is ReDoS-safe and still correctly strips the version constraints from dependency names in bothDepends:andRecommends:parsing paths. No behavior regression evident.Also applies to: 166-167
cortex/installation_history.py (1)
176-178: Consistent dependency parsing and clarified non-crypto MD5 usage
- Line [176]-[178]: Matching
Depends:lines now uses the same safe regex as the resolver; it still strips version constraints and first-alternative selection behaves as before.- Line [246]-[256]: The expanded
_generate_iddocstring and inline comment make it explicit that MD5 is only for non-cryptographic, human-sized IDs, which appropriately documents the existing behavior.No functional issues spotted here.
Also applies to: 246-256
cortex/cli.py (2)
291-401: Config subcommand wiring and validation path look solidThe new
config()handler:
- Delegates to
PreferencesManagerfor all operations (list/get/set/reset/validate/info/export/import).- Uses
_parse_config_value()to coerce CLI strings into booleans/ints/lists where appropriate.- Surfaces validation errors and unknown keys via caught
ValueErrorwith a clean error message.The flow for reset/validate/info/export/import is straightforward and matches the documented CLI usage in
main()’s epilog. No functional issues spotted here.
471-477: Config subparser and help text integration look correctThe
configsubparser and the corresponding examples in the epilog are aligned:
- Actions are constrained to the supported set via
choices=[...].- Positional
key/valuearguments map correctly ontoCortexCLI.config(...).- The help text (
cortex config list/get/set/reset/validate) matches the implementation.This should give users a clear, discoverable entry point into the new preferences system.
Also applies to: 494-495, 439-443
test/test_context_memory.py (1)
16-20: Updated import path forcortex.context_memorylooks goodAdding the project root via:
sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..')) from cortex.context_memory import (...)resolves the earlier duplicate/ambiguous path setup and aligns the tests with the
cortexpackage namespace. No issues here.test/test_user_preferences.py (1)
132-415: PreferencesManager tests provide strong lifecycle and persistence coverageThe
TestPreferencesManagersuite does a good job exercising load/save, get/set, reset (all and per-key), validation, backup creation, JSON import/export, YAML handling, concurrent-manager access, and directory creation. The scenarios appear consistent with the implementation incortex/user_preferences.pyand should give high confidence in the new preferences system.
|
@Sahilbhatane just keep files related to this issue in this PR. Don't need to restructure repository And add video implementation. |
dde6afd to
d9f2277
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: 3
♻️ Duplicate comments (3)
docs/USER_PREFERENCES_IMPLEMENTATION.md (1)
441-445: File permission claim is inaccurate.The documentation states config files are "created with user-only read/write (600)" but the implementation does not explicitly set file permissions - it relies on the OS umask defaults. Either update the implementation to set permissions explicitly using
os.chmod(path, 0o600)after creation, or revise this documentation to reflect the actual behavior.test/test_user_preferences.py (1)
1-5: Remove unnecessary shebang from test module.This test file has a shebang (
#!/usr/bin/env python3) but is not intended to be executed directly as a script - it's run via the test runner. Ruff flags this as EXE001.-#!/usr/bin/env python3 """ Comprehensive tests for User Preferences & Settings System Tests all preference categories, validation, import/export, and persistence """cortex/user_preferences.py (1)
197-231: Unknown nested keys are silently accepted but not persisted.The
set()method usessetattrwithout validating that the attribute exists on nested dataclasses. For example,set("ai.models", "gpt-4")(typo: "models" vs "model") will:
- Dynamically add a
modelsattribute toAISettings- Return successfully
- Not persist the value because
asdict()only serializes declared dataclass fieldsThis creates confusing "appears to succeed but doesn't stick" behavior.
Add validation before
setattr:# Set the final attribute attr_name = parts[-1] + if not hasattr(obj, attr_name): + raise AttributeError(f"Unknown preference key: {key}") current_value = getattr(obj, attr_name)
🧹 Nitpick comments (9)
test/test_user_preferences.py (2)
15-16: Prefer project-level test configuration over sys.path manipulation.Modifying
sys.pathdirectly in test files is fragile. Consider configuring pytest/unittest properly viapyproject.tomlorsetup.pyso imports work naturally, or use relative imports if the test is part of a package.
359-366: Atomic write test only verifies cleanup, not atomicity.The test verifies no
.tmpfile remains after save, but doesn't verify atomicity under failure conditions. Consider adding a test that simulates a write failure (e.g., via mocking) to ensure the temp file is properly cleaned up and the original file remains intact.cortex/cli.py (4)
284-286: Checkingvalue is Noneis unreliable for detecting missing keys.
manager.get(key)returnsNoneboth for missing keys AND for keys that legitimately haveNoneas a value (though none currently do). More importantly, theget()method returns the default parameter when the key doesn't exist. Since you pass no default, it returnsNone. This works now but is fragile.Consider using a sentinel value or catching
AttributeErrorinstead:- value = manager.get(key) - if value is None: + _MISSING = object() + value = manager.get(key, default=_MISSING) + if value is _MISSING:
341-487: Refactoredit_prefto reduce cognitive complexity.SonarCloud flags this method with cognitive complexity of 39 (limit: 15). Consider extracting each action into separate handler methods.
def edit_pref(self, action: str, key: Optional[str] = None, value: Optional[str] = None): """Edit user preferences""" manager = self._get_prefs_manager() handlers = { 'set': self._handle_set_pref, 'add': self._handle_set_pref, 'update': self._handle_set_pref, 'delete': self._handle_delete_pref, 'remove': self._handle_delete_pref, 'reset-key': self._handle_delete_pref, 'list': lambda m, k, v: self.check_pref(), 'show': lambda m, k, v: self.check_pref(), 'display': lambda m, k, v: self.check_pref(), 'reset-all': self._handle_reset_all, 'validate': self._handle_validate, 'export': self._handle_export, 'import': self._handle_import, } handler = handlers.get(action) if handler: return handler(manager, key, value) else: self._print_error(f"Unknown action: {action}") # ... print available actions return 1
479-487: Improve exception handling: remove unused variable and use exception chaining.The
AttributeErrorhandler capturesebut doesn't use it. Also, the broadExceptioncatch should ideally use exception chaining when re-raising or be more specific.- except AttributeError as e: + except AttributeError: self._print_error(f"Invalid preference key: {key}") print("Use 'cortex check-pref' to see available keys") return 1 except Exception as e: - self._print_error(f"Failed to edit preferences: {str(e)}") - import traceback - traceback.print_exc() + self._print_error(f"Failed to edit preferences: {e!s}") + if os.environ.get('DEBUG'): + import traceback + traceback.print_exc() return 1
415-424: Interactive confirmation blocks non-interactive usage.Using
input()for confirmation prevents this command from being used in scripts or CI pipelines. Consider adding a--yesor--forceflag to bypass confirmation.cortex/user_preferences.py (3)
1-1: Remove shebang from library module.This is a library module, not an executable script. The shebang is unnecessary and triggers Ruff EXE001.
-#!/usr/bin/env python3 """ User Preferences & Settings System Manages persistent user preferences and configuration for Cortex Linux """
132-135: Broad exception catch swallows specific errors.Catching all
Exceptiontypes here will mask issues likePermissionError,yaml.YAMLError, etc. Consider catching more specific exceptions or at minimum logging the exception type.- except Exception as e: - print(f"[WARNING] Could not load preferences: {e}") + except (yaml.YAMLError, ValueError, TypeError) as e: + print(f"[WARNING] Could not load preferences ({type(e).__name__}): {e}") print("[INFO] Using default preferences") return self.preferences
328-337: Double stat() call is inefficient.
get_config_info()callsself.config_path.stat()twice when the file exists. Cache the stat result.def get_config_info(self) -> Dict[str, Any]: """Get configuration metadata""" + exists = self.config_path.exists() + stat_info = self.config_path.stat() if exists else None return { 'config_path': str(self.config_path), - 'config_exists': self.config_path.exists(), - 'config_size_bytes': self.config_path.stat().st_size if self.config_path.exists() else 0, + 'config_exists': exists, + 'config_size_bytes': stat_info.st_size if stat_info else 0, 'last_modified': datetime.fromtimestamp( - self.config_path.stat().st_mtime - ).isoformat() if self.config_path.exists() else None + stat_info.st_mtime + ).isoformat() if stat_info else None }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
LLM/requirements.txt(1 hunks)cortex/cli.py(5 hunks)cortex/user_preferences.py(1 hunks)docs/USER_PREFERENCES_IMPLEMENTATION.md(1 hunks)test/test_user_preferences.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- LLM/requirements.txt
🧰 Additional context used
🧬 Code graph analysis (2)
test/test_user_preferences.py (1)
cortex/user_preferences.py (19)
PreferencesManager(83-337)UserPreferences(71-80)VerbosityLevel(18-23)AICreativity(26-30)ConfirmationSettings(34-39)AutoUpdateSettings(43-47)AISettings(51-58)PackageSettings(62-67)format_preference_value(341-352)print_all_preferences(355-362)get(176-195)reset(233-236)validate(238-262)export_json(264-284)load(104-135)import_json(286-310)get_all_settings(312-326)get_config_info(328-337)save(137-174)
cortex/cli.py (1)
cortex/user_preferences.py (10)
PreferencesManager(83-337)print_all_preferences(355-362)format_preference_value(341-352)get(176-195)validate(238-262)get_config_info(328-337)UserPreferences(71-80)reset(233-236)export_json(264-284)import_json(286-310)
🪛 GitHub Check: SonarCloud Code Analysis
cortex/user_preferences.py
[warning] 174-174: Replace this generic exception class with a more specific one.
cortex/cli.py
[failure] 341-341: Refactor this function to reduce its Cognitive Complexity from 39 to the 15 allowed.
🪛 markdownlint-cli2 (0.18.1)
docs/USER_PREFERENCES_IMPLEMENTATION.md
324-324: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 Ruff (0.14.5)
test/test_user_preferences.py
1-1: Shebang is present but file is not executable
(EXE001)
cortex/user_preferences.py
1-1: Shebang is present but file is not executable
(EXE001)
130-130: Consider moving this statement to an else block
(TRY300)
132-132: Do not catch blind exception: Exception
(BLE001)
171-171: Do not catch blind exception: Exception
(BLE001)
174-174: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
174-174: Create your own exception
(TRY002)
174-174: Avoid specifying long messages outside the exception class
(TRY003)
193-193: Consider moving this statement to an else block
(TRY300)
cortex/cli.py
337-337: Do not catch blind exception: Exception
(BLE001)
338-338: Use explicit conversion flag
Replace with conversion flag
(RUF010)
479-479: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
483-483: Do not catch blind exception: Exception
(BLE001)
484-484: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (2)
test/test_user_preferences.py (1)
137-149: Good test setup with proper isolation.The test fixture correctly uses temporary directories and cleans up after each test. This ensures tests don't interfere with each other or leave artifacts.
cortex/user_preferences.py (1)
137-169: Atomic write implementation looks correct.The
save()method properly implements atomic writes using a temp file andreplace(). This ensures the config file is never left in a partially-written state. Good implementation.
|
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
♻️ Duplicate comments (1)
docs/USER_PREFERENCES_IMPLEMENTATION.md (1)
257-268: Windows path separator remains in test command example.The test command still uses Windows backslash syntax which won't work on Linux/macOS. This was previously flagged but not yet addressed.
🧹 Nitpick comments (4)
cortex/cli.py (2)
479-481: Remove unused exception variable.The exception variable
eis captured but never used. Remove it or log it for debugging purposes.- except AttributeError as e: + except AttributeError: self._print_error(f"Invalid preference key: {key}")
341-487: Consider extracting action handlers to reduce complexity.This method has cognitive complexity of 39 vs the recommended 15. While each action branch is relatively straightforward, extracting them into separate methods (e.g.,
_edit_pref_set,_edit_pref_delete,_edit_pref_export) would improve maintainability and testability.This is a suggested refactor for future improvement rather than a blocking issue.
cortex/user_preferences.py (2)
1-1: Shebang unnecessary for library module.The shebang is present but the file is not executable and is primarily imported as a module. While the
if __name__ == "__main__"block provides a demo, consider removing the shebang or making the file executable if it's intended as a runnable script.
202-236: Validate attribute existence before setting.The
set()method usesgetattrto retrieve the current value (line 219) which will raiseAttributeErrorif the key doesn't exist, but this only works for the final attribute. For nested keys, if a typo is made in the final part,getattrat line 219 will raise an error, so the validation is actually present.However, the error message from a failed
getattrmay not be clear to users. Consider catchingAttributeErrorand re-raising with a more helpful message.def set(self, key: str, value: Any) -> None: """ Set preference value by dot notation key Args: key: Dot notation key (e.g., 'ai.model') value: Value to set """ parts = key.split('.') obj = self.preferences - # Navigate to parent object - for part in parts[:-1]: - obj = getattr(obj, part) - - # Set the final attribute - attr_name = parts[-1] - current_value = getattr(obj, attr_name) + try: + # Navigate to parent object + for part in parts[:-1]: + obj = getattr(obj, part) + + # Set the final attribute + attr_name = parts[-1] + current_value = getattr(obj, attr_name) + except AttributeError: + raise PreferencesError(f"Unknown preference key: {key}") # Type coercion if isinstance(current_value, bool):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cortex/cli.py(5 hunks)cortex/user_preferences.py(1 hunks)docs/USER_PREFERENCES_IMPLEMENTATION.md(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cortex/cli.py (2)
cortex/user_preferences.py (10)
PreferencesManager(88-342)print_all_preferences(360-367)format_preference_value(346-357)get(181-200)validate(243-267)get_config_info(333-342)UserPreferences(76-85)reset(238-241)export_json(269-289)import_json(291-315)logging_system.py (2)
error(219-221)info(211-213)
🪛 GitHub Check: SonarCloud Code Analysis
cortex/cli.py
[failure] 341-341: Refactor this function to reduce its Cognitive Complexity from 39 to the 15 allowed.
🪛 markdownlint-cli2 (0.18.1)
docs/USER_PREFERENCES_IMPLEMENTATION.md
324-324: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 Ruff (0.14.5)
cortex/cli.py
337-337: Do not catch blind exception: Exception
(BLE001)
338-338: Use explicit conversion flag
Replace with conversion flag
(RUF010)
479-479: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
483-483: Do not catch blind exception: Exception
(BLE001)
484-484: Use explicit conversion flag
Replace with conversion flag
(RUF010)
cortex/user_preferences.py
1-1: Shebang is present but file is not executable
(EXE001)
135-135: Consider moving this statement to an else block
(TRY300)
137-137: Do not catch blind exception: Exception
(BLE001)
179-179: Avoid specifying long messages outside the exception class
(TRY003)
198-198: Consider moving this statement to an else block
(TRY300)
🔇 Additional comments (18)
cortex/cli.py (3)
18-22: LGTM: Clean lazy initialization setup.The import structure and lazy initialization pattern for the preferences manager is well-designed and follows best practices.
Also applies to: 29-29
271-275: LGTM: Lazy initialization implemented correctly.The lazy initialization pattern is properly implemented, creating the PreferencesManager only when needed and caching it for subsequent calls.
277-339: Well-implemented preference checking with good UX.The
check_prefmethod provides comprehensive functionality:
- Helpful error messages with available keys listed
- Validation status display
- Configuration metadata
The broad
Exceptioncatch at line 337 is acceptable for a CLI context where graceful degradation is preferred over crashes.cortex/user_preferences.py (15)
18-20: LGTM: Custom exception defined.The custom
PreferencesErrorexception is properly defined and used throughout the module, providing clear error classification.
23-86: LGTM: Well-designed data models.The data models are cleanly structured using dataclasses and enums:
- Clear enumeration of allowed values
- Sensible defaults
- Proper use of
field(default_factory=...)for mutable defaultsThe design provides a solid foundation for the preferences system.
91-107: LGTM: Robust initialization with sensible defaults.The initialization properly handles:
- Custom or default config paths
- Cross-platform path construction
- Automatic directory creation
- Immediate loading of existing config
109-140: LGTM: Graceful config loading with fallback.The
load()method handles config files robustly:
- Creates default config if missing
- Carefully parses nested structures with proper enum handling
- Falls back to defaults on error with user warning
The broad exception catch (line 137) is appropriate here since config corruption shouldn't crash the application—defaulting to safe values is the right choice.
142-179: LGTM: Robust atomic save with backup.The
save()method implements proper safeguards:
- Creates
.yaml.bakbackup before overwriting- Uses temp file + rename for atomic writes (prevents corruption)
- Custom exception with proper chaining (
raise ... from e)- Cleanup of temp file on error
This is a well-implemented persistence layer.
Note: File permissions are not explicitly set (no
os.chmod), which is addressed in the documentation review.
181-200: LGTM: Clean dot-notation getter.The
get()method provides intuitive dot-notation access with proper default handling. The use ofgetattrwithAttributeErrorcatching is the right approach.
238-241: LGTM: Simple and effective reset.The
reset()method correctly reinitializes all preferences to defaults and persists the change immediately.
243-267: LGTM: Non-throwing validation design.The
validate()method returns a list of error messages rather than raising exceptions, which is the right design for batch validation. Users can see all issues at once rather than fixing them one at a time.The validation rules are sensible and match the documented requirements.
269-289: LGTM: Clean JSON export.The
export_json()method provides a useful backup/sharing mechanism with proper metadata inclusion.
291-315: LGTM: JSON import with proper reconstruction.The
import_json()method correctly reconstructs preferences from exported JSON, handles metadata cleanup, and persists the imported configuration.
317-331: LGTM: Comprehensive settings export.The
get_all_settings()method provides a complete snapshot of current preferences for display or inspection.
333-342: LGTM: Config metadata inspection.The
get_config_info()method safely retrieves configuration file metadata with proper existence checks before accessing file stats.
345-357: LGTM: Comprehensive value formatting.The
format_preference_value()helper handles all relevant preference types for user-friendly display.
360-367: LGTM: Clean preference display utility.The
print_all_preferences()function provides a user-friendly overview of all current settings with the config file location.
370-376: LGTM: Useful demo for manual testing.The
__main__block provides a quick way to verify the preferences system is working, which is helpful during development.
Video -https://drive.google.com/file/d/1T-5AJxhzfwx0Q2MDSSYE9L-vqkUT8BBI/view?usp=sharing might be low quality had to decrease it for upload. rest assure everything works and test passes. screenshot of one of the commands -
|




Summary
Implemented a comprehensive user preferences and settings system that allows users to customize Cortex behavior through CLI commands or configuration files. The system includes 6 preference categories with YAML-based storage, validation, import/export functionality, and automatic backup.
Additionally changed some file directories to make proper file structure.
Type of Change
Checklist
Testing
Test Suite
test/test_user_preferences.pyManual Testing
Tested all CLI commands:
config list- Display all preferencesconfig get <key>- Retrieve specific settingconfig set <key> <value>- Update setting valueconfig reset- Restore defaultsconfig validate- Verify configuration integrityconfig info- Show current settings summaryconfig export <file>- Export to YAML fileconfig import <file>- Import from YAML fileFeatures Implemented
Six Preference Categories:
Storage & Persistence:
~/.config/cortex/preferences.yamlValidation:
Documentation:
docs/USER_PREFERENCES_IMPLEMENTATION.mdConfiguration File Location
%USERPROFILE%\.config\cortex\preferences.yaml~/.config/cortex/preferences.yamlSummary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.