[tutor] Add AI-Powered Installation Tutor (Issue #131) #566
Conversation
Add comprehensive AI tutor for package education with: - Interactive tutoring with Plan→Act→Reflect LangGraph workflow - LLM-powered lessons, code examples, and Q&A (Claude API via LangChain) - SQLite-based progress tracking with topic completion stats - Best practices and step-by-step tutorials - Rich terminal UI with branded output Features: - cortex tutor <package> - Start interactive session - cortex tutor --list - Show studied packages - cortex tutor --progress - View learning progress - cortex tutor --reset - Reset progress Technical: - 266 tests with 85.61% coverage - Lazy imports for non-LLM operations (no API key needed for --list/--progress) - Pydantic models for type safety - 7-layer prompt architecture Closes cortexlinux#131
|
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. 📝 WalkthroughWalkthroughAdds an AI‑powered Intelligent Tutor: CLI subcommand and wiring, interactive TutorAgent and UI, LLM wrappers, deterministic tools and validators, SQLite persistence and caching, Rich terminal branding, Pydantic contracts, prompts, tests, docs, and packaging updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as Tutor CLI
participant Agent as TutorAgent
participant Loader as LessonLoaderTool
participant LLM as LLMRouter
participant Store as SQLiteStore
participant UI as InteractiveTutor
User->>CLI: run "cortex tutor <package>"
CLI->>Agent: teach(package, force_fresh)
Agent->>Loader: load(package, force_fresh)
alt cache hit
Loader-->>Agent: cached lesson
else cache miss
Loader->>LLM: generate_lesson(package)
LLM-->>Loader: lesson JSON
Loader->>Store: cache lesson
Loader-->>Agent: lesson payload
end
Agent->>Store: get_student_profile()
Agent-->>UI: render lesson and menu
loop user interaction
User->>UI: choose action / ask question
alt ask question
UI->>Agent: ask(package, question)
Agent->>LLM: answer_question(...)
LLM-->>Agent: answer payload
Agent->>Store: update progress / quiz results
Agent-->>UI: show answer
else view progress
UI->>Store: get_progress(package)
Store-->>UI: progress data
end
end
User->>UI: exit
UI->>Store: finalize session
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
tests/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (3)📚 Learning: 2026-01-12T20:51:13.828ZApplied to files:
📚 Learning: 2026-01-12T20:51:13.828ZApplied to files:
📚 Learning: 2026-01-12T20:51:13.828ZApplied to files:
🧬 Code graph analysis (1)tests/tutor/test_llm.py (1)
🪛 markdownlint-cli2 (0.18.1)docs/AI_TUTOR.md92-92: Fenced code blocks should have a language specified (MD040, fenced-code-language) 250-250: Fenced code blocks should have a language specified (MD040, fenced-code-language) 310-310: Fenced code blocks should have a language specified (MD040, fenced-code-language) 349-349: Fenced code blocks should have a language specified (MD040, fenced-code-language) 390-390: Fenced code blocks should have a language specified (MD040, fenced-code-language) 432-432: Fenced code blocks should have a language specified (MD040, fenced-code-language) 🔇 Additional comments (3)
✏️ Tip: You can disable this entire section by setting 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 |
CLA Verification PassedAll contributors have signed the CLA.
|
Summary of ChangesHello @srikrishnavansi, 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 integrates an advanced AI-powered tutor into Cortex Linux, providing users with an interactive and personalized way to learn about various software packages. The system intelligently generates educational content, tracks learning progress, and offers quick answers to questions, enhancing the user's ability to understand and utilize system tools effectively. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an excellent and comprehensive pull request that adds a powerful AI Tutor feature. The architecture is well-designed, leveraging modern tools like LangGraph and Pydantic for a robust and maintainable implementation. The code is clean, well-documented, and includes a thorough test suite, which is fantastic to see. The 7-layer prompt architecture is particularly impressive and shows a deep understanding of prompt engineering best practices.
My review focuses on a few minor areas for improvement to enhance maintainability and robustness, such as centralizing CLI argument definitions and removing hardcoded values. Overall, this is a very high-quality contribution.
| generator = LessonGeneratorTool() | ||
| result = generator._run( | ||
| package_name=package_name, | ||
| student_level="beginner", # Could be dynamic based on profile |
There was a problem hiding this comment.
The student_level is hardcoded to 'beginner'. The comment correctly notes that this could be dynamic. To make the tutor more adaptive, you could determine the student's level from their profile. For instance, you could add a simple function to assess the number of mastered_concepts vs weak_concepts and map that to a 'beginner', 'intermediate', or 'advanced' level. This would make the generated lessons more personalized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In @cortex/tutor/cli.py:
- Around line 110-150: The cmd_teach function accepts a fresh flag but never
passes it to the InteractiveTutor; update cmd_teach to forward the fresh flag
when constructing InteractiveTutor (e.g., InteractiveTutor(package,
force_fresh=fresh) or matching param name), and update InteractiveTutor.__init__
in cortex.tutor.agents.tutor_agent to accept and store the corresponding
parameter (force_fresh or fresh) so the interactive session honors the flag.
In @cortex/tutor/config.py:
- Line 44: The db_path field is declared as Path but defaults to None causing a
type mismatch; update the annotation to Optional[Path] (and add from typing
import Optional if missing) for the db_path Field declaration so it accurately
allows None prior to model_post_init, keeping the Field(..., description="Path
to SQLite database") usage and leaving model_post_init behavior unchanged.
In @cortex/tutor/memory/sqlite_store.py:
- Around line 340-377: The singleton profile logic can race when two threads
insert simultaneously; add a UNIQUE constraint to the student_profile table
(e.g., enforce a single row via id PRIMARY KEY or a UNIQUE constant key) and
change _create_default_profile() to perform an atomic upsert (SQLite: INSERT OR
IGNORE or INSERT ... ON CONFLICT DO NOTHING) instead of blindly inserting, then
re-query the table and return the actual stored row; update
get_student_profile() to, after attempting creation, SELECT the profile again
(don’t recurse) so whichever thread succeeded returns the canonical row and
duplicates/constraint errors are avoided.
In @cortex/tutor/tests/test_cli.py:
- Around line 103-115: The test mocks the wrong import location for
InteractiveTutor so cmd_teach still uses the real class; update the patch target
to where cmd_teach looks up the name by replacing
@patch("cortex.tutor.agents.tutor_agent.InteractiveTutor") with
@patch("cortex.tutor.cli.InteractiveTutor") in the test (test_successful_teach)
so the mocked InteractiveTutor instance is injected when cmd_teach("docker")
constructs and calls InteractiveTutor.start.
In @cortex/tutor/tests/test_progress_tracker.py:
- Around line 290-304: The three test stubs in TestConvenienceFunctions
(test_get_learning_progress, test_mark_topic_completed, test_get_package_stats)
should be implemented or removed; to fix, implement them using the temp_db
fixture to create minimal sample data, import and call the actual convenience
functions (get_learning_progress, mark_topic_completed, get_package_stats), then
assert expected behavior: for get_learning_progress verify returned structure
and percentages for a package/topic based on created progress rows, for
mark_topic_completed verify it creates/updates a progress record marking the
topic completed and sets completed_at or equivalent flag, and for
get_package_stats verify aggregate stats (total topics, completed count,
completion rate) match the test data; use the function/class names above to
locate targets and clean up any leftover pass placeholders if you prefer
removing unused tests.
In @cortex/tutor/tools/deterministic/lesson_loader.py:
- Around line 131-149: The clear_cache function's behavior doesn't match its
docstring: clear_cache(package_name) currently overwrites the cache entry via
self.store.cache_lesson(package_name, {}, ttl_hours=0) (marking it
expired/unretrievable to get_cached_lesson) rather than deleting it, and
clear_cache(None) calls self.store.clear_expired_cache() which only removes
already-expired entries. Update the clear_cache docstring to say that passing a
package_name marks that package's cache as expired (using cache_lesson with
ttl_hours=0) and that the function returns 1 on successful marking or 0 on
error; also clarify that calling clear_cache(None) only removes entries whose
expires_at <= now and that the return value is whatever
self.store.clear_expired_cache() returns (number of entries removed). Ensure
references to the symbols clear_cache, self.store.cache_lesson,
self.store.clear_expired_cache, and get_cached_lesson are used in the docstring
to make behavior explicit.
In @docs/AI_TUTOR.md:
- Line 753: The documentation line "Current coverage: **87%** (266 tests)" is
stale; update that exact string to reflect the current test coverage percentage
and test count (replace both the percentage and the test count with the latest
values), and scan for other occurrences of the same string in the docs to keep
coverage numbers consistent; commit the updated line so the README/doc shows the
accurate "Current coverage: **X%** (Y tests)" value.
- Line 6: The coverage badge in the README currently shows 87% ("[](https://github.com/cortexlinux/cortex)"),
which conflicts with the PR's reported 85.61%; update the badge value to match
the actual coverage (replace "87%25" with "85.61%25") or switch to a dynamic
coverage badge that reflects the real-time metric so the percentage stays
accurate.
In @requirements.txt:
- Around line 27-31: The dependency version ranges in requirements (langchain,
langchain-anthropic, langgraph) are too loose and risk pulling breaking major
releases; update the entries for langchain, langchain-anthropic, and langgraph
to use compatible release specifiers (e.g., change langchain>=0.3.0 to an
approximate pin like langchain~=1.2.0, langchain-anthropic>=0.3.0 to
langchain-anthropic~=1.3.0, and langgraph>=0.2.0 to langgraph~=1.0.0) so you
allow patch/minor updates but prevent unintended major version jumps while
leaving pydantic as-is unless you want to similarly tighten it.
🧹 Nitpick comments (42)
cortex/tutor/tools/agentic/examples_provider.py (2)
7-7: Unused import.
Pathfrompathlibis imported but not used in this module.🧹 Suggested fix
-from pathlib import Path from typing import Any, Dict, List, Optional
59-157: Consider reducing duplication between_runand_arun.Both methods share identical prompt construction and response structuring logic. Extracting the common setup into a helper method would improve maintainability.
♻️ Optional refactor to reduce duplication
def _build_chain(self): """Build the prompt-LLM-parser chain.""" prompt = ChatPromptTemplate.from_messages( [ ("system", self._get_system_prompt()), ("human", self._get_generation_prompt()), ] ) return prompt | self.llm | JsonOutputParser() def _build_inputs(self, package_name, topic, difficulty, learning_style, existing_knowledge): """Build input dict for chain invocation.""" return { "package_name": package_name, "topic": topic, "difficulty": difficulty, "learning_style": learning_style, "existing_knowledge": ", ".join(existing_knowledge or []) or "basics", } def _run(self, package_name: str, topic: str, ...) -> Dict[str, Any]: try: chain = self._build_chain() inputs = self._build_inputs(package_name, topic, difficulty, learning_style, existing_knowledge) result = chain.invoke(inputs) examples = self._structure_response(result, package_name, topic) return {"success": True, "examples": examples, "cost_gbp": 0.01} except Exception as e: return {"success": False, "error": str(e), "examples": None}requirements.txt (1)
25-26: Pre-existing duplicate PyYAML entries noted.There are three PyYAML specifications in this file (lines 9, 21, and 25) with conflicting constraints (
>=6.0.0vs==6.0.3). While not introduced by this PR, this could cause dependency resolution issues. Consider consolidating in a follow-up.cortex/tutor/tests/test_branding.py (3)
8-9: Remove unused imports.
MagicMockandStringIOare imported but never used in this test module.Proposed fix
-from unittest.mock import Mock, patch, MagicMock -from io import StringIO +from unittest.mock import Mock, patch
46-70: Consider adding output assertions for stronger tests.These tests accept
capsysbut don't use it to verify the output. While they serve as smoke tests ensuring no exceptions are raised, they don't validate the actual output content.For improved test quality, consider capturing and asserting on key output patterns:
def test_tutor_print_success(self, capsys): tutor_print("Test message", "success") captured = capsys.readouterr() assert "Test message" in captured.out
243-248: Clarify test intent and strengthen assertion.The test name
test_get_user_input_stripssuggests testing strip behavior, but the comment says "get_user_input should return raw input" (no stripping). The assertionassert "spaced" in resultpasses regardless of whether input is stripped or not.Clarify the intended behavior and use a precise assertion:
Proposed fix for raw input (no stripping)
@patch("builtins.input", return_value=" spaced ") - def test_get_user_input_strips(self, mock_input): - """Test input stripping is not done (raw input).""" + def test_get_user_input_returns_raw(self, mock_input): + """Test get_user_input returns raw input without stripping.""" result = get_user_input("Enter value") - # Note: get_user_input should return raw input - assert "spaced" in result + assert result == " spaced "cortex/tutor/tests/test_tutor_agent.py (3)
9-10: Remove unused imports.
tempfileandPathare imported but never used in this test module.Proposed fix
-import tempfile -from pathlib import Path
78-84: Floating-point comparison may be fragile.Direct equality comparison
state["cost_gbp"] == 0.03on floats can fail due to precision issues. While0.02 + 0.01is exact in this case, consider usingpytest.approxfor robustness:assert state["cost_gbp"] == pytest.approx(0.03)
303-317: Consider splitting multiple exception assertions into separate tests.This test validates two different input validation scenarios (empty package name and empty question) in a single test. If the first assertion fails, the second is never executed, potentially hiding additional issues.
Consider splitting into
test_ask_validates_package_nameandtest_ask_validates_questionfor better test isolation.cortex/tutor/tests/test_interactive_tutor.py (2)
8-8: Remove unused import.
callis imported fromunittest.mockbut never used in this test module.Proposed fix
-from unittest.mock import Mock, patch, MagicMock, call +from unittest.mock import Mock, patch, MagicMock
246-262: Consider adding behavior assertions for None lesson handling.The test verifies that methods don't raise exceptions when
lessonisNone, which is valuable. However, adding assertions about the expected behavior (e.g., verifying that appropriate "No content" messages are printed or that methods return early) would strengthen the test.cortex/tutor/prompts/tools/lesson_generator.md (2)
66-74: Add language identifier to fenced code block.Per static analysis (markdownlint MD040), this code block should specify a language. Since it's JSON, add
jsonafter the opening fence.-``` +```json { "package_name": "name of the package to teach",
104-132: Consider adding a language identifier for the workflow code block.This fenced code block (starting at Line 104) lacks a language specification. Since it's pseudo-code/plaintext describing the workflow, you could use
textorplaintextas the identifier to satisfy linting rules.cortex/tutor/tests/test_integration.py (1)
207-216: Consider adding minimal assertions or removing unusedcapsys.The
capsysfixture is captured but not used in the assertions. If verifying Rich console output is impractical, consider removing the capture or adding a comment explaining the intentional no-op check.♻️ Suggested refactor
def test_tutor_print_success(self, capsys): """Test tutor_print with success status.""" tutor_print("Test message", "success") - captured = capsys.readouterr() - # Rich console output is complex, just ensure no errors + # Rich console output is complex; this test verifies no exceptions are raised def test_tutor_print_error(self, capsys): """Test tutor_print with error status.""" tutor_print("Error message", "error") - captured = capsys.readouterr() + # Verifies no exceptions are raised with error statuscortex/tutor/tests/test_validators.py (2)
216-219: Type hint inconsistency:sanitize_inputacceptsNonebut typed asstr.The test correctly verifies that
sanitize_input(None)returns"", but the function signature invalidators.pyis typed assanitize_input(input_text: str). Consider updating the type hint toOptional[str]for consistency, or document this behavior.
240-243: Same type consideration forextract_package_name(None).Similar to
sanitize_input, this testsNoneinput while the function signature expectsstr.cortex/tutor/tests/test_deterministic_tools.py (1)
99-108: Consider usingtmp_pathfixture instead of customtemp_db.Pytest provides a built-in
tmp_pathfixture that handles cleanup automatically. The current approach works, but usingtmp_pathwould be slightly more idiomatic.♻️ Optional refactor
- @pytest.fixture - def temp_db(self): - """Create a temporary database.""" - with tempfile.TemporaryDirectory() as tmpdir: - yield Path(tmpdir) / "test.db" + @pytest.fixture + def temp_db(self, tmp_path): + """Create a temporary database.""" + return tmp_path / "test.db"cortex/tutor/tools/__init__.py (1)
7-11: Consider addingLessonLoaderToolto parent package exports for API consistency.The
cortex/tutor/tools/deterministic/__init__.pyexportsLessonLoaderTool, but this parent package's__all__does not include it. Since other deterministic tools (ProgressTrackerTool,validate_package_name,validate_input) are re-exported here,LessonLoaderToolshould also be added for a complete and consistent public API. This allows users to import it asfrom cortex.tutor.tools import LessonLoaderToolalongside other deterministic tools.cortex/tutor/tests/test_progress_tracker.py (1)
27-44: Consider adding return type hints to fixtures.Per coding guidelines, type hints are required. The fixtures should specify return types.
♻️ Suggested improvement
@pytest.fixture -def temp_db(): +def temp_db() -> Path: """Create a temporary database for testing.""" with tempfile.TemporaryDirectory() as tmpdir: db_path = Path(tmpdir) / "test_progress.db" yield db_path @pytest.fixture -def store(temp_db): +def store(temp_db: Path) -> SQLiteStore: """Create a SQLite store with temp database.""" return SQLiteStore(temp_db) @pytest.fixture -def tracker(temp_db): +def tracker(temp_db: Path) -> ProgressTrackerTool: """Create a progress tracker with temp database.""" return ProgressTrackerTool(temp_db)cortex/tutor/tests/test_agent_methods.py (1)
7-10: Remove unused imports.
tempfileandPathare imported but never used in this test file.♻️ Proposed fix
import pytest from unittest.mock import Mock, patch, MagicMock -import tempfile -from pathlib import Pathcortex/tutor/tests/test_cli.py (1)
10-10: Remove unused import.
StringIOis imported but never used in this file.♻️ Proposed fix
-from io import StringIOcortex/tutor/tests/test_tools.py (2)
8-8: Remove unused import.
MagicMockis imported but never used; onlyMockis used.♻️ Proposed fix
-from unittest.mock import Mock, patch, MagicMock +from unittest.mock import Mock, patch
276-307: Fragile test pattern using__new__to bypass__init__.Using
ConversationHandler.__new__()and manually setting attributes bypasses the constructor, which can lead to tests that don't catch initialization bugs. Consider properly mocking the config dependency and instantiating normally.♻️ Suggested improvement
def test_build_context_empty(self): """Test context building with empty history.""" - with patch("cortex.tutor.tools.agentic.qa_handler.get_config"): - handler = ConversationHandler.__new__(ConversationHandler) - handler.history = [] + with patch("cortex.tutor.tools.agentic.qa_handler.get_config") as mock_config: + mock_config.return_value = Mock( + anthropic_api_key="test_key", + model="claude-sonnet-4-20250514", + ) + with patch("cortex.tutor.tools.agentic.qa_handler.ChatAnthropic"): + handler = ConversationHandler() + handler.history = [] # Reset for test context = handler._build_context() assert "Starting fresh" in contextcortex/tutor/agents/tutor_agent/tutor_agent.py (2)
248-258: Unusedcurrent_stepattribute and missingforce_freshsupport.The
current_stepattribute (line 258) is initialized but never used in the class. Additionally, the CLI's--freshflag (passed tocmd_teach) is not propagated toInteractiveTutor, so users cannot force fresh content generation in interactive mode.Consider either:
- Removing
current_stepif not needed- Adding
force_fresh: bool = Falseparameter to__init__and passing it toself.agent.teach()♻️ Suggested fix to support force_fresh
- def __init__(self, package_name: str) -> None: + def __init__(self, package_name: str, force_fresh: bool = False) -> None: """ Initialize interactive tutor for a package. Args: package_name: Package to learn. + force_fresh: Skip cache and generate fresh content. """ self.package_name = package_name self.agent = TutorAgent(verbose=False) self.lesson: Optional[Dict[str, Any]] = None - self.current_step = 0 + self.force_fresh = force_fresh
439-446: Consider extracting the magic number5as a constant.The default topic count of
5(line 443) is used as a fallback whenstats.get("total", 0)is0. While the inline comment explains it, extracting this as a named constant would improve clarity.cortex/tutor/tools/deterministic/lesson_loader.py (1)
100-106: Consider async SQLite operations in the future.The async method currently delegates to the synchronous implementation. This is acceptable for fast SQLite operations but could be improved with
aiosqliteif async performance becomes important.cortex/tutor/tools/agentic/lesson_generator.py (3)
1-19: Unused imports fromlesson_context.
LessonContext,CodeExample, andTutorialStep(line 18) are imported but not used in this module. The_structure_responsemethod returns a plainDictrather than using these Pydantic models for validation.Consider either:
- Removing unused imports
- Using
LessonContext.model_validate()in_structure_responseto leverage Pydantic validation
21-36:_load_prompt_templateis defined but never called.The function loads a prompt template from a file but is never invoked anywhere in this module. The actual prompts are generated inline by
_get_system_prompt()and_get_generation_prompt().♻️ Consider removing unused function
-# Load prompt template -def _load_prompt_template() -> str: - """Load the lesson generator prompt from file.""" - prompt_path = Path(__file__).parent.parent.parent / "prompts" / "tools" / "lesson_generator.md" - if prompt_path.exists(): - return prompt_path.read_text() - # Fallback inline prompt - return """You are a lesson content generator. Generate comprehensive educational content - for the package: {package_name} - - Student level: {student_level} - Learning style: {learning_style} - Focus areas: {focus_areas} - - Return a JSON object with: summary, explanation, use_cases, best_practices, - code_examples, tutorial_steps, installation_command, confidence.""" - -
142-184: Consider extracting shared prompt construction logic.The
_arunmethod duplicates the prompt construction from_run(lines 105-110 vs 152-157). Extracting this to a helper method would reduce duplication.cortex/tutor/contracts/progress_context.py (1)
98-100:datetime.utcnow()is deprecated; use timezone-aware datetime.
datetime.utcnow()is deprecated since Python 3.12. Usedatetime.now(timezone.utc)for timezone-aware UTC timestamps.♻️ Suggested fix
+from datetime import datetime, timezone -from datetime import datetime ... last_updated: datetime = Field( - default_factory=datetime.utcnow, description="Last update timestamp" + default_factory=lambda: datetime.now(timezone.utc), description="Last update timestamp (UTC)" )cortex/tutor/agents/tutor_agent/graph.py (1)
1-25: Unused import:ExamplesProviderTool.
ExamplesProviderTool(line 24) is imported but never used in this module.♻️ Remove unused import
from cortex.tutor.tools.agentic.qa_handler import QAHandlerTool -from cortex.tutor.tools.agentic.examples_provider import ExamplesProviderToolcortex/tutor/contracts/lesson_context.py (1)
99-101:datetime.utcnow()is deprecated.Same issue as in
progress_context.py. Usedatetime.now(timezone.utc)for timezone-aware UTC timestamps.cortex/tutor/tools/deterministic/validators.py (1)
40-49: Unused constant:KNOWN_PACKAGE_CATEGORIES.This constant is defined but never referenced anywhere in the module. Consider removing it or documenting its intended future use.
🔧 Suggested fix
Either remove the unused constant:
-# Common package categories for validation hints -KNOWN_PACKAGE_CATEGORIES = [ - "system", # apt, systemctl, journalctl - "development", # git, docker, npm, pip - "database", # postgresql, mysql, redis, mongodb - "web", # nginx, apache, curl, wget - "security", # ufw, fail2ban, openssl - "networking", # ssh, netstat, iptables - "utilities", # vim, tmux, htop, grep -]Or add a TODO comment explaining its intended use:
+# TODO: Use for package categorization hints in future validation enhancements KNOWN_PACKAGE_CATEGORIES = [cortex/tutor/tools/deterministic/progress_tracker.py (4)
7-10: Unused import:Path.The
Pathtype is imported but never used in this module.🔧 Suggested fix
from datetime import datetime -from pathlib import Path from typing import Any, Dict, List, Optional
115-117: Async method delegates to sync - no actual async benefit.The
_arunmethod simply calls the synchronous_run, which blocks the event loop. This is acceptable if the underlying SQLite operations are fast, but consider noting this limitation in the docstring.📝 Suggested docstring improvement
async def _arun(self, *args: Any, **kwargs: Any) -> Dict[str, Any]: - """Async version - delegates to sync implementation.""" + """Async version - delegates to sync implementation. + + Note: SQLite operations are synchronous. For true async, consider + aiosqlite in future iterations. + """ return self._run(*args, **kwargs)
258-280: Potential parameter shadowing issue.The
conceptparameter is fetched fromkwargsfirst, then falls back to the function parameter. This is redundant sinceconceptwould already be inkwargsif passed as a keyword argument.🔧 Suggested simplification
def _add_mastered_concept( self, concept: Optional[str] = None, **kwargs: Any, ) -> Dict[str, Any]: """Add a mastered concept to student profile.""" - concept = kwargs.get("concept") or concept if not concept: return {"success": False, "error": "concept required"} self.store.add_mastered_concept(concept) return {"success": True, "message": f"Added mastered concept: {concept}"} def _add_weak_concept( self, concept: Optional[str] = None, **kwargs: Any, ) -> Dict[str, Any]: """Add a weak concept to student profile.""" - concept = kwargs.get("concept") or concept if not concept: return {"success": False, "error": "concept required"} self.store.add_weak_concept(concept) return {"success": True, "message": f"Added weak concept: {concept}"}
301-346: Convenience functions create new tool instances on each call.Each call instantiates a new
ProgressTrackerTool, which opens a new database connection. For repeated operations, this is inefficient. Consider documenting this trade-off or providing a module-level singleton option.📝 Suggested improvement
Add a note about usage patterns:
# Convenience functions for direct usage +# Note: Each function creates a new tool instance. For batch operations, +# instantiate ProgressTrackerTool once and reuse it. def get_learning_progress(package_name: str, topic: str) -> Optional[Dict[str, Any]]:Or provide a singleton pattern:
_default_tool: Optional[ProgressTrackerTool] = None def _get_tool() -> ProgressTrackerTool: global _default_tool if _default_tool is None: _default_tool = ProgressTrackerTool() return _default_toolcortex/tutor/tools/agentic/qa_handler.py (4)
7-8: Unused import:Path.The
Pathtype is imported but never used in this module.🔧 Suggested fix
-from pathlib import Path from typing import Any, Dict, List, Optional
59-162: Significant code duplication between_runand_arun.The sync and async methods share nearly identical logic. Extract the common setup into a helper method.
♻️ Suggested refactor
+ def _build_chain(self): + """Build the prompt chain for Q&A.""" + prompt = ChatPromptTemplate.from_messages( + [ + ("system", self._get_system_prompt()), + ("human", self._get_qa_prompt()), + ] + ) + return prompt | self.llm | JsonOutputParser() + + def _build_invoke_params( + self, + package_name: str, + question: str, + learning_style: str, + mastered_concepts: Optional[List[str]], + weak_concepts: Optional[List[str]], + lesson_context: Optional[str], + ) -> Dict[str, Any]: + """Build parameters for chain invocation.""" + return { + "package_name": package_name, + "question": question, + "learning_style": learning_style, + "mastered_concepts": ", ".join(mastered_concepts or []) or "none specified", + "weak_concepts": ", ".join(weak_concepts or []) or "none specified", + "lesson_context": lesson_context or "starting fresh", + } + def _run(self, ...) -> Dict[str, Any]: try: - prompt = ChatPromptTemplate.from_messages(...) - chain = prompt | self.llm | JsonOutputParser() - result = chain.invoke({...}) + chain = self._build_chain() + params = self._build_invoke_params(...) + result = chain.invoke(params) # ... rest unchanged
105-109: Hardcoded cost estimate may become inaccurate.The
cost_gbp: 0.02is static and may not reflect actual API costs as pricing changes.📝 Suggested improvement
Consider computing based on token usage or making it configurable:
+ # Note: Estimated cost; actual varies by token usage return { "success": True, "answer": answer, - "cost_gbp": 0.02, + "cost_estimate_gbp": 0.02, # Approximate based on ~2K tokens }
268-344: Unbounded conversation history growth.The
historylist grows indefinitely during long sessions. While_build_contextonly uses the last 3 exchanges, the full history is retained in memory.🔧 Suggested fix to cap history size
def ask( self, question: str, ... ) -> Dict[str, Any]: ... # Update history if result.get("success"): self.history.append( { "question": question, "answer": result["answer"].get("answer", ""), } ) + # Keep history bounded to prevent memory growth + max_history = 20 # or make configurable + if len(self.history) > max_history: + self.history = self.history[-max_history:] return resultcortex/tutor/memory/sqlite_store.py (1)
41-48: Replace mutable default values withField(default_factory=list).Pydantic recommends using
Field(default_factory=list)for list defaults to ensure each instance gets a fresh list and to maintain consistency across Pydantic versions. While Pydantic does deepcopy mutable defaults, explicitly usingdefault_factoryis the clearer, future-proof approach.🔧 Suggested fix
+from pydantic import BaseModel, Field -from pydantic import BaseModel class StudentProfile(BaseModel): """Model for student profile.""" id: Optional[int] = None - mastered_concepts: List[str] = [] - weak_concepts: List[str] = [] + mastered_concepts: List[str] = Field(default_factory=list) + weak_concepts: List[str] = Field(default_factory=list) learning_style: str = "reading" # visual, reading, hands-on last_session: Optional[str] = None
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
cortex/cli.pycortex/tutor/__init__.pycortex/tutor/agents/__init__.pycortex/tutor/agents/tutor_agent/__init__.pycortex/tutor/agents/tutor_agent/graph.pycortex/tutor/agents/tutor_agent/state.pycortex/tutor/agents/tutor_agent/tutor_agent.pycortex/tutor/branding.pycortex/tutor/cli.pycortex/tutor/config.pycortex/tutor/contracts/__init__.pycortex/tutor/contracts/lesson_context.pycortex/tutor/contracts/progress_context.pycortex/tutor/memory/__init__.pycortex/tutor/memory/sqlite_store.pycortex/tutor/prompts/agents/tutor/system.mdcortex/tutor/prompts/tools/examples_provider.mdcortex/tutor/prompts/tools/lesson_generator.mdcortex/tutor/prompts/tools/qa_handler.mdcortex/tutor/tests/__init__.pycortex/tutor/tests/test_agent_methods.pycortex/tutor/tests/test_agentic_tools.pycortex/tutor/tests/test_branding.pycortex/tutor/tests/test_cli.pycortex/tutor/tests/test_deterministic_tools.pycortex/tutor/tests/test_integration.pycortex/tutor/tests/test_interactive_tutor.pycortex/tutor/tests/test_progress_tracker.pycortex/tutor/tests/test_tools.pycortex/tutor/tests/test_tutor_agent.pycortex/tutor/tests/test_validators.pycortex/tutor/tools/__init__.pycortex/tutor/tools/agentic/__init__.pycortex/tutor/tools/agentic/examples_provider.pycortex/tutor/tools/agentic/lesson_generator.pycortex/tutor/tools/agentic/qa_handler.pycortex/tutor/tools/deterministic/__init__.pycortex/tutor/tools/deterministic/lesson_loader.pycortex/tutor/tools/deterministic/progress_tracker.pycortex/tutor/tools/deterministic/validators.pydocs/AI_TUTOR.mdrequirements.txt
🧰 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/tutor/agents/__init__.pycortex/tutor/agents/tutor_agent/state.pycortex/tutor/agents/tutor_agent/__init__.pycortex/tutor/tests/__init__.pycortex/tutor/memory/__init__.pycortex/tutor/tools/agentic/__init__.pycortex/tutor/tests/test_branding.pycortex/tutor/tests/test_validators.pycortex/tutor/tests/test_agent_methods.pycortex/tutor/tests/test_cli.pycortex/tutor/config.pycortex/tutor/tools/__init__.pycortex/tutor/tests/test_tools.pycortex/tutor/tools/deterministic/__init__.pycortex/cli.pycortex/tutor/tests/test_tutor_agent.pycortex/tutor/cli.pycortex/tutor/tests/test_interactive_tutor.pycortex/tutor/tests/test_progress_tracker.pycortex/tutor/contracts/progress_context.pycortex/tutor/contracts/lesson_context.pycortex/tutor/tests/test_agentic_tools.pycortex/tutor/tests/test_integration.pycortex/tutor/branding.pycortex/tutor/agents/tutor_agent/graph.pycortex/tutor/tools/agentic/examples_provider.pycortex/tutor/tools/agentic/qa_handler.pycortex/tutor/tools/deterministic/lesson_loader.pycortex/tutor/agents/tutor_agent/tutor_agent.pycortex/tutor/tools/deterministic/progress_tracker.pycortex/tutor/tests/test_deterministic_tools.pycortex/tutor/contracts/__init__.pycortex/tutor/__init__.pycortex/tutor/tools/agentic/lesson_generator.pycortex/tutor/tools/deterministic/validators.pycortex/tutor/memory/sqlite_store.py
{setup.py,setup.cfg,pyproject.toml,**/__init__.py}
📄 CodeRabbit inference engine (AGENTS.md)
Use Python 3.10 or higher as the minimum supported version
Files:
cortex/tutor/agents/__init__.pycortex/tutor/agents/tutor_agent/__init__.pycortex/tutor/tests/__init__.pycortex/tutor/memory/__init__.pycortex/tutor/tools/agentic/__init__.pycortex/tutor/tools/__init__.pycortex/tutor/tools/deterministic/__init__.pycortex/tutor/contracts/__init__.pycortex/tutor/__init__.py
🧠 Learnings (2)
📚 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 **/*.py : Type hints required in Python code
Applied to files:
requirements.txt
📚 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 tests/**/*.py : Maintain >80% test coverage for pull requests
Applied to files:
cortex/tutor/tests/__init__.py
🧬 Code graph analysis (20)
cortex/tutor/agents/__init__.py (2)
cortex/cli.py (1)
tutor(1016-1074)cortex/tutor/agents/tutor_agent/tutor_agent.py (1)
TutorAgent(20-238)
cortex/tutor/agents/tutor_agent/__init__.py (2)
cortex/tutor/agents/tutor_agent/state.py (1)
TutorAgentState(50-104)cortex/tutor/agents/tutor_agent/tutor_agent.py (2)
TutorAgent(20-238)InteractiveTutor(241-447)
cortex/tutor/memory/__init__.py (1)
cortex/tutor/memory/sqlite_store.py (1)
SQLiteStore(51-531)
cortex/tutor/tools/agentic/__init__.py (3)
cortex/tutor/tools/agentic/lesson_generator.py (1)
LessonGeneratorTool(39-303)cortex/tutor/tools/agentic/examples_provider.py (1)
ExamplesProviderTool(19-236)cortex/tutor/tools/agentic/qa_handler.py (1)
QAHandlerTool(19-241)
cortex/tutor/tests/test_validators.py (1)
cortex/tutor/tools/deterministic/validators.py (11)
validate_package_name(52-94)validate_input(97-136)validate_question(139-149)validate_topic(152-174)validate_score(177-193)validate_learning_style(196-211)sanitize_input(214-237)extract_package_name(240-278)get_validation_errors(281-321)validate_all(349-368)ValidationResult(324-346)
cortex/tutor/tests/test_agent_methods.py (2)
cortex/tutor/agents/tutor_agent/graph.py (10)
plan_node(30-87)load_cache_node(90-109)generate_lesson_node(112-154)qa_node(157-199)reflect_node(202-251)fail_node(254-275)route_after_plan(281-297)route_after_act(300-311)create_tutor_graph(317-373)get_tutor_graph(380-390)cortex/tutor/agents/tutor_agent/tutor_agent.py (8)
TutorAgent(20-238)teach(47-94)ask(96-139)get_profile(155-162)update_learning_style(164-175)mark_completed(177-195)reset_progress(197-208)get_packages_studied(210-218)
cortex/tutor/tests/test_cli.py (1)
cortex/tutor/cli.py (7)
create_parser(33-107)cmd_teach(110-150)cmd_question(153-208)cmd_list_packages(211-238)cmd_progress(241-311)cmd_reset(314-345)main(348-386)
cortex/tutor/config.py (6)
cortex/tutor/tools/agentic/examples_provider.py (1)
Config(39-40)cortex/tutor/tools/agentic/lesson_generator.py (1)
Config(62-63)cortex/tutor/tools/agentic/qa_handler.py (1)
Config(39-40)cortex/tutor/tools/deterministic/lesson_loader.py (1)
Config(35-36)cortex/tutor/tools/deterministic/progress_tracker.py (1)
Config(45-46)cortex/logging_system.py (1)
debug(196-198)
cortex/tutor/tools/__init__.py (5)
cortex/tutor/tools/deterministic/progress_tracker.py (1)
ProgressTrackerTool(23-295)cortex/tutor/tools/deterministic/validators.py (2)
validate_package_name(52-94)validate_input(97-136)cortex/tutor/tools/agentic/lesson_generator.py (1)
LessonGeneratorTool(39-303)cortex/tutor/tools/agentic/examples_provider.py (1)
ExamplesProviderTool(19-236)cortex/tutor/tools/agentic/qa_handler.py (1)
QAHandlerTool(19-241)
cortex/tutor/tests/test_tools.py (1)
cortex/tutor/tools/deterministic/lesson_loader.py (4)
LessonLoaderTool(18-149)get_fallback_lesson(227-237)load_lesson_with_fallback(240-276)_run(51-98)
cortex/tutor/tools/deterministic/__init__.py (3)
cortex/tutor/tools/deterministic/progress_tracker.py (1)
ProgressTrackerTool(23-295)cortex/tutor/tools/deterministic/validators.py (2)
validate_package_name(52-94)validate_input(97-136)cortex/tutor/tools/deterministic/lesson_loader.py (1)
LessonLoaderTool(18-149)
cortex/tutor/tests/test_tutor_agent.py (3)
cortex/tutor/tools/agentic/qa_handler.py (1)
_run(59-116)cortex/tutor/tools/deterministic/lesson_loader.py (1)
_run(51-98)cortex/tutor/config.py (1)
reset_config(144-147)
cortex/tutor/cli.py (5)
cortex/tutor/branding.py (8)
print_banner(53-69)tutor_print(35-50)print_table(162-180)print_progress_summary(143-159)print_error_panel(238-251)print_success_panel(254-267)get_user_input(194-216)print_code_example(102-115)cortex/tutor/tools/deterministic/validators.py (1)
validate_package_name(52-94)cortex/tutor/config.py (3)
Config(19-121)from_env(60-89)get_db_path(101-109)cortex/tutor/memory/sqlite_store.py (5)
SQLiteStore(51-531)get_packages_studied(520-531)get_completion_stats(257-286)get_all_progress(171-203)reset_progress(500-518)cortex/tutor/agents/tutor_agent/tutor_agent.py (6)
InteractiveTutor(241-447)start(260-326)TutorAgent(20-238)ask(96-139)get_packages_studied(210-218)reset_progress(197-208)
cortex/tutor/tests/test_interactive_tutor.py (2)
cortex/tutor/tools/agentic/qa_handler.py (1)
ask(286-327)cortex/tutor/memory/sqlite_store.py (1)
get_progress(142-169)
cortex/tutor/tests/test_agentic_tools.py (3)
cortex/tutor/tools/agentic/lesson_generator.py (2)
LessonGeneratorTool(39-303)_structure_response(251-303)cortex/tutor/tools/agentic/examples_provider.py (1)
_structure_response(208-236)cortex/tutor/tools/agentic/qa_handler.py (5)
_structure_response(215-241)QAHandlerTool(19-241)ConversationHandler(268-344)_build_context(329-340)clear_history(342-344)
cortex/tutor/tests/test_integration.py (5)
cortex/tutor/config.py (5)
Config(19-121)get_config(128-141)reset_config(144-147)from_env(60-89)ensure_data_dir(91-99)cortex/tutor/branding.py (2)
tutor_print(35-50)print_banner(53-69)cortex/tutor/contracts/lesson_context.py (6)
LessonContext(36-132)CodeExample(13-21)TutorialStep(24-33)to_json(103-105)from_json(108-110)to_display_dict(120-132)cortex/tutor/contracts/progress_context.py (8)
ProgressContext(66-143)PackageProgress(23-63)TopicProgress(13-20)completion_percentage(33-38)average_score(42-46)is_complete(61-63)get_next_topic(54-59)get_recommendations(115-131)cortex/tutor/cli.py (1)
create_parser(33-107)
cortex/tutor/contracts/__init__.py (2)
cortex/tutor/contracts/lesson_context.py (1)
LessonContext(36-132)cortex/tutor/contracts/progress_context.py (1)
ProgressContext(66-143)
cortex/tutor/__init__.py (2)
cortex/tutor/config.py (1)
Config(19-121)cortex/tutor/branding.py (1)
tutor_print(35-50)
cortex/tutor/tools/agentic/lesson_generator.py (2)
cortex/tutor/config.py (2)
get_config(128-141)Config(19-121)cortex/tutor/contracts/lesson_context.py (3)
LessonContext(36-132)CodeExample(13-21)TutorialStep(24-33)
cortex/tutor/memory/sqlite_store.py (5)
cortex/tutor/contracts/progress_context.py (1)
total_time_seconds(50-52)cortex/tutor/agents/tutor_agent/tutor_agent.py (3)
get_progress(141-153)reset_progress(197-208)get_packages_studied(210-218)cortex/tutor/tools/deterministic/progress_tracker.py (1)
mark_topic_completed(317-331)cortex/hwprofiler.py (1)
profile(421-448)cortex/tutor/tools/deterministic/lesson_loader.py (1)
cache_lesson(108-129)
🪛 markdownlint-cli2 (0.18.1)
docs/AI_TUTOR.md
92-92: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
250-250: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
310-310: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
362-362: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
401-401: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
443-443: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
cortex/tutor/prompts/tools/lesson_generator.md
66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
97-97: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
| def cmd_teach(package: str, verbose: bool = False, fresh: bool = False) -> int: | ||
| """ | ||
| Start an interactive tutoring session. | ||
| Args: | ||
| package: Package name to teach. | ||
| verbose: Enable verbose output. | ||
| fresh: Skip cache. | ||
| Returns: | ||
| Exit code (0 for success, 1 for failure). | ||
| """ | ||
| # Validate package name | ||
| is_valid, error = validate_package_name(package) | ||
| if not is_valid: | ||
| print_error_panel(f"Invalid package name: {error}") | ||
| return 1 | ||
|
|
||
| try: | ||
| # Lazy import - only load when needed (requires API key) | ||
| from cortex.tutor.agents.tutor_agent import InteractiveTutor | ||
|
|
||
| # Start interactive tutor | ||
| interactive = InteractiveTutor(package) | ||
| interactive.start() | ||
| return 0 | ||
|
|
||
| except ValueError as e: | ||
| print_error_panel(str(e)) | ||
| return 1 | ||
| except KeyboardInterrupt: | ||
| console.print() | ||
| tutor_print("Session ended", "info") | ||
| return 0 | ||
| except Exception as e: | ||
| print_error_panel(f"An error occurred: {e}") | ||
| if verbose: | ||
| import traceback | ||
|
|
||
| console.print_exception() | ||
| return 1 |
There was a problem hiding this comment.
--fresh flag is not propagated to InteractiveTutor.
The fresh parameter (line 110) is accepted but never passed to InteractiveTutor (line 133). The flag has no effect in interactive mode.
This connects to the earlier observation that InteractiveTutor.__init__ doesn't accept a force_fresh parameter.
🔧 Suggested fix
def cmd_teach(package: str, verbose: bool = False, fresh: bool = False) -> int:
...
try:
from cortex.tutor.agents.tutor_agent import InteractiveTutor
- interactive = InteractiveTutor(package)
+ interactive = InteractiveTutor(package, force_fresh=fresh)
interactive.start()This requires updating InteractiveTutor.__init__ as noted in the tutor_agent.py review.
🤖 Prompt for AI Agents
In @cortex/tutor/cli.py around lines 110 - 150, The cmd_teach function accepts a
fresh flag but never passes it to the InteractiveTutor; update cmd_teach to
forward the fresh flag when constructing InteractiveTutor (e.g.,
InteractiveTutor(package, force_fresh=fresh) or matching param name), and update
InteractiveTutor.__init__ in cortex.tutor.agents.tutor_agent to accept and store
the corresponding parameter (force_fresh or fresh) so the interactive session
honors the flag.
| @patch.dict("os.environ", {"ANTHROPIC_API_KEY": "sk-ant-test-key"}) | ||
| @patch("cortex.tutor.agents.tutor_agent.InteractiveTutor") | ||
| def test_successful_teach(self, mock_tutor_class): | ||
| """Test successful teach session.""" | ||
| from cortex.tutor.config import reset_config | ||
| reset_config() # Reset config singleton | ||
|
|
||
| mock_tutor = Mock() | ||
| mock_tutor_class.return_value = mock_tutor | ||
|
|
||
| result = cmd_teach("docker") | ||
| assert result == 0 | ||
| mock_tutor.start.assert_called_once() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how InteractiveTutor is imported/used in cli.py
rg -n "InteractiveTutor" cortex/tutor/cli.py -A 2 -B 2Repository: cortexlinux/cortex
Length of output: 364
🏁 Script executed:
#!/bin/bash
# Check the implementation of cmd_teach function
rg -n "def cmd_teach" cortex/tutor/cli.py -A 20Repository: cortexlinux/cortex
Length of output: 789
🏁 Script executed:
#!/bin/bash
# Check the exact patch decorator in test_cli.py
sed -n '103,115p' cortex/tutor/tests/test_cli.pyRepository: cortexlinux/cortex
Length of output: 589
Adjust patch target for InteractiveTutor mocking.
Since InteractiveTutor is imported locally within the cmd_teach function (line 130 of cortex/tutor/cli.py), the patch must target where the name is looked up, not where the class is defined. Change the patch from @patch("cortex.tutor.agents.tutor_agent.InteractiveTutor") to @patch("cortex.tutor.cli.InteractiveTutor") to correctly mock the class instance used by cmd_teach.
🤖 Prompt for AI Agents
In @cortex/tutor/tests/test_cli.py around lines 103 - 115, The test mocks the
wrong import location for InteractiveTutor so cmd_teach still uses the real
class; update the patch target to where cmd_teach looks up the name by replacing
@patch("cortex.tutor.agents.tutor_agent.InteractiveTutor") with
@patch("cortex.tutor.cli.InteractiveTutor") in the test (test_successful_teach)
so the mocked InteractiveTutor instance is injected when cmd_teach("docker")
constructs and calls InteractiveTutor.start.
There was a problem hiding this comment.
@srikrishnavansi Please address this one as well.
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!
|
recheck |
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
@srikrishnavansi Follow the contributing.md guidelines( i.e. add demonstrating video in pr description, add Documentation for the feature you're creating, add tests and AI Usage as well, thanks.
Also, kindly address CODERABBITAI comments.
|
I will update the PR and changes suggested by CodeRabbit. |
srikrishnavansi
changed the title
Fixes identified in PR cortexlinux#566 code review: - Fix coverage badge discrepancy (87% → 85.6%) in docs/AI_TUTOR.md - Fix db_path type annotation to Optional[Path] in config.py - Remove unused ExamplesProviderTool import from graph.py - Add DEFAULT_TUTOR_TOPICS constant to avoid magic numbers - Fix --fresh flag propagation to InteractiveTutor - Fix race condition in profile creation with INSERT OR IGNORE - Tighten version constraints in requirements.txt (<2.0.0 bounds) - Update clear_cache docstring to match actual behavior - Implement empty test methods in TestConvenienceFunctions All 266 tests pass with 86% coverage.
…/cortex into feature/ai-tutor
- Fix import sorting (I001) with ruff format - Modernize type annotations for Python 3.10+: - Replace Dict/List/Tuple with dict/list/tuple - Replace Optional[X] with X | None - Remove deprecated typing imports - Fix f-string without placeholders All 266 tests pass.
…/cortex into feature/ai-tutor
The tutor tests were in cortex/tutor/tests/ but CI runs pytest tests/ - causing 0% coverage on tutor code. Copied tests to tests/tutor/ so CI discovers them and includes tutor code in coverage metrics. All 266 tutor tests pass from new location.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @cortex/tutor/contracts/lesson_context.py:
- Around line 97-99: The Field default_factory for generated_at currently uses
datetime.utcnow (in the generated_at: datetime = Field(...
default_factory=datetime.utcnow ...)), which is deprecated; replace it with a
timezone-aware factory using datetime.now(timezone.utc) and ensure timezone is
imported from datetime (or use datetime.timezone) so generated_at remains
UTC-aware; update the default_factory reference in the generated_at Field and
add the necessary import for timezone at the top of the module.
In @cortex/tutor/contracts/progress_context.py:
- Line 157: The QuizContext.timestamp Field currently uses datetime.utcnow() (in
the default_factory) which is deprecated; change the default_factory for
QuizContext.timestamp to produce a timezone-aware UTC datetime (e.g., use
datetime.now(timezone.utc) or an equivalent lambda), and update imports if
needed to include timezone so the Field default is timezone-aware while keeping
the type as datetime.
- Around line 98-100: The Field default_factory for last_updated in
progress_context.py uses deprecated datetime.utcnow(); update the
default_factory to use datetime.now(timezone.utc) instead and ensure timezone is
imported from datetime (mirror the fix applied in lesson_context.py); modify the
default_factory on the last_updated Field (and any similar uses in this module)
to call datetime.now(timezone.utc) to produce an aware UTC timestamp.
In @cortex/tutor/memory/sqlite_store.py:
- Around line 206-239: The upsert_progress method currently returns
cursor.lastrowid which is 0 when the INSERT triggers the ON CONFLICT ... DO
UPDATE branch; change upsert_progress (learning_progress upsert) to detect that
case and return the actual row id by querying the table for the record after the
upsert (e.g., SELECT id or rowid WHERE package_name = ? AND topic = ?) when
cursor.lastrowid is 0, ensuring the function always returns the real row id for
the given progress.package_name and progress.topic.
- Around line 419-444: The functions add_mastered_concept and add_weak_concept
perform a read-modify-write via get_student_profile() and
update_student_profile(), which opens a TOCTOU race; replace the Python-side
mutation with an atomic database operation (or wrap the operation in a single
transaction) so concurrent updates cannot be lost: implement a single SQL UPDATE
that uses JSON functions to append/remove the concept from the mastered_concepts
or weak_concepts JSON array (and remove it from the opposite array when
promoting/demoting), or execute the read and write inside a DB transaction with
appropriate locking/isolation in the same code paths where get_student_profile()
and update_student_profile() are called so add_mastered_concept and
add_weak_concept no longer rely on separate read and write calls that can
interleave.
🧹 Nitpick comments (46)
cortex/tutor/tests/test_validators.py (2)
7-7: Unusedpytestimport.The
pytestmodule is imported but never used in this test file. Nopytest.fixture,pytest.raises, or other pytest-specific features are utilized.🔧 Suggested fix
-import pytest - from cortex.tutor.tools.deterministic.validators import (
165-180: Consider adding boundary value tests for score validation.The tests cover valid scores and out-of-range values, but consider adding explicit boundary tests for exactly
0.0and1.0to ensure inclusive boundary handling is verified.🧪 Additional boundary test
def test_boundary_scores(self): """Test exact boundary values are valid.""" # Ensure 0.0 and 1.0 are explicitly accepted is_valid, error = validate_score(0.0) assert is_valid, "0.0 should be valid" is_valid, error = validate_score(1.0) assert is_valid, "1.0 should be valid"cortex/tutor/memory/sqlite_store.py (2)
52-57: Docstring claims "connection pooling" but implementation creates new connections.The class docstring mentions "connection pooling," but
_get_connectioncreates a new connection on each call and closes it after use. This is connection-per-request, not pooling.📝 Suggested docstring fix
class SQLiteStore: """ SQLite-based storage for learning progress and student data. - Thread-safe implementation with connection pooling. + Thread-safe implementation with per-request connections. Attributes: db_path: Path to the SQLite database file. """
42-50: UseField(default_factory=list)for explicit mutable defaults.While Pydantic v2 safely deep-copies empty list defaults for each instance, the idiomatic approach is
Field(default_factory=list)to explicitly signal that each instance gets its own list. This also guards against issues if the codebase ever migrates to older Pydantic versions where mutable defaults were not safely handled. Apply tomastered_conceptsandweak_conceptsfields.cortex/tutor/tests/test_deterministic_tools.py (2)
9-9: Unused imports fromunittest.mock.
Mockandpatchare imported but never used in this test file.🔧 Suggested fix
import tempfile from pathlib import Path -from unittest.mock import Mock, patch import pytest
100-109: Duplicatetemp_dbfixture definition.The
temp_dbfixture is defined identically in bothTestLessonLoaderTool(lines 26-30) andTestProgressTrackerTool(lines 100-104). Consider moving it to aconftest.pyfile for reuse.♻️ Suggested refactor
Create
cortex/tutor/tests/conftest.py:import tempfile from pathlib import Path import pytest @pytest.fixture def temp_db(): """Create a temporary database.""" with tempfile.TemporaryDirectory() as tmpdir: yield Path(tmpdir) / "test.db"Then remove the duplicate fixtures from both test classes.
tests/tutor/test_agentic_tools.py (1)
7-7: Remove unused importMagicMock.
MagicMockis imported but never used in this file; onlyMockis utilized.Suggested fix
-from unittest.mock import MagicMock, Mock, patch +from unittest.mock import Mock, patchcortex/tutor/tests/test_agent_methods.py (1)
7-11: Remove unused imports.
tempfileandMagicMockare imported but not used in this file.Suggested fix
-import tempfile from pathlib import Path -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import Mock, patchcortex/tutor/tests/test_branding.py (2)
7-10: Remove unused imports.
StringIOandMagicMockare imported but never used.Suggested fix
-from io import StringIO -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import Mock, patch
244-249: Clarify test assertion for stripping behavior.The test name suggests verifying stripping behavior, but
assert "spaced" in resultpasses whetherresultis" spaced "or"spaced". Consider asserting the exact expected value.Suggested fix for explicit verification
@patch("builtins.input", return_value=" spaced ") def test_get_user_input_strips(self, mock_input): """Test input stripping is not done (raw input).""" result = get_user_input("Enter value") - # Note: get_user_input should return raw input - assert "spaced" in result + # Verify raw input is returned without stripping + assert result == " spaced "tests/tutor/test_tutor_agent.py (1)
7-11: Remove unused imports.
tempfileandMagicMockare imported but not used.Suggested fix
-import tempfile from pathlib import Path -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import Mock, patchcortex/tutor/tests/test_integration.py (2)
10-10: Remove unused import.
MagicMockis imported but not used.Suggested fix
-from unittest.mock import MagicMock, Mock, patch +from unittest.mock import Mock, patch
205-222: Consider removing duplicate branding tests.These tests duplicate coverage from
tests/tutor/test_branding.py. Consider removing them to reduce maintenance overhead, or keep them intentionally as integration-level smoke tests.tests/tutor/test_interactive_tutor.py (2)
1-11: Unused import detected.The
callimport fromunittest.mockis not used anywhere in this file.🧹 Remove unused import
-from unittest.mock import MagicMock, Mock, call, patch +from unittest.mock import MagicMock, Mock, patch
247-264: Test coverage for None lesson is incomplete.This test only exercises 4 of the 6 helper methods (
_show_concepts,_show_examples,_run_tutorial,_show_best_practices). Consider adding_ask_questionand_show_progressfor completeness.Additionally, the test only verifies no exceptions are raised but doesn't assert expected behavior (e.g., that appropriate "no data" messages are printed).
🧪 Extended test with assertions
def test_methods_with_no_lesson(self, mock_agent_class): """Test methods handle None lesson gracefully.""" from cortex.tutor.agents.tutor_agent import InteractiveTutor tutor = InteractiveTutor("docker") tutor.lesson = None tutor.agent = Mock() + tutor.agent.get_progress.return_value = {"success": False} # These should not raise errors - tutor._show_concepts() - tutor._show_examples() - tutor._run_tutorial() - tutor._show_best_practices() + with patch("cortex.tutor.agents.tutor_agent.tutor_agent.tutor_print") as mock_print: + tutor._show_concepts() + tutor._show_examples() + tutor._run_tutorial() + tutor._show_best_practices() + tutor._show_progress()tests/tutor/test_agent_methods.py (1)
411-441: State helpers well-tested, buthas_critical_erroris missing a direct test.The
has_critical_errorfunction is imported (line 33) but not directly tested inTestStateHelpers. It's indirectly tested viaTestRouting.test_route_after_plan_fail_on_error, but a direct unit test would improve coverage.🧪 Add direct test for has_critical_error
def test_has_critical_error_true(self): """Test has_critical_error returns True for non-recoverable errors.""" state = create_initial_state("docker") add_error(state, "test", "Critical error", recoverable=False) assert has_critical_error(state) is True def test_has_critical_error_false(self): """Test has_critical_error returns False for recoverable errors.""" state = create_initial_state("docker") add_error(state, "test", "Minor error", recoverable=True) assert has_critical_error(state) is Falsetests/tutor/test_integration.py (2)
205-222: Branding tests verify no errors but lack output assertions.The
capsys.readouterr()result is captured but not verified. While Rich console output is complex, consider at minimum asserting the output is non-empty for success cases, or use Rich'sConsolewith aStringIOfor testable output.🧪 Add minimal assertions
def test_tutor_print_success(self, capsys): """Test tutor_print with success status.""" tutor_print("Test message", "success") captured = capsys.readouterr() - # Rich console output is complex, just ensure no errors + # Rich console output is complex, verify something was printed + assert len(captured.out) > 0 or len(captured.err) > 0
298-365: Good end-to-end test for cache-hit workflow.The test properly exercises the Plan→Load→Reflect flow with mocked tools and verifies state transitions. The note about removing real API tests is appropriate for CI.
Consider adding a similar test for the cache-miss path (Plan→Generate→Reflect) to cover both main workflows.
cortex/tutor/tests/test_progress_tracker.py (1)
290-351: Redundant imports inside test methods.
Mockandpatchare imported inside each test method. Since these are already available in the module scope (line 9), the local imports are unnecessary.🧹 Remove redundant imports
def test_get_learning_progress(self, temp_db): """Test get_learning_progress function.""" - from unittest.mock import Mock, patch - # Mock the global config to use temp_db mock_config = Mock()Apply similar changes to
test_mark_topic_completedandtest_get_package_stats.cortex/tutor/tests/test_tools.py (1)
277-310: ConversationHandler tests use__new__to bypass initialization.While using
__new__to create uninitialized instances works for testing, it's fragile and couples tests to implementation details. Consider adding a factory method or making the handler more testable.# Alternative: Initialize properly with mocked config with patch("cortex.tutor.tools.agentic.qa_handler.get_config") as mock_config: mock_config.return_value = Mock(anthropic_api_key="test", model="test") with patch("cortex.tutor.tools.agentic.qa_handler.ChatAnthropic"): handler = ConversationHandler() handler.history = [] # Reset history for testcortex/tutor/config.py (2)
45-49: Direct mutation inmodel_post_initworks but considermodel_validator.Pydantic v2 allows mutation in
model_post_init, but using amodel_validatorwithmode='after'is more idiomatic for computed defaults.♻️ Alternative using model_validator
from pydantic import model_validator @model_validator(mode='after') def set_db_path_default(self) -> 'Config': """Set default db_path if not provided.""" if self.db_path is None: object.__setattr__(self, 'db_path', self.data_dir / "tutor_progress.db") return self
100-108: Return type inconsistency with field type.
get_db_path()returnsPathbutdb_pathfield is typed asPath | None. Whilemodel_post_initensures it's always set, the type checker may flag this. Consider adding an assertion or adjusting the type.🔧 Add runtime assertion for type safety
def get_db_path(self) -> Path: """ Get the full path to the SQLite database. Returns: Path: Full path to tutor_progress.db """ self.ensure_data_dir() + assert self.db_path is not None, "db_path should be set by model_post_init" return self.db_pathcortex/tutor/branding.py (2)
7-7: Remove unusedOptionalimport.The code uses modern union syntax (
str | None) throughout, soOptionalis not needed.Proposed fix
-from typing import Literal, Optional +from typing import Literal
218-234: Unuseddescriptionparameter.The
descriptionparameter is accepted but not used. If the intent is to set a default task description, it should be returned with a pre-added task or documented that callers should useprogress.add_task(description).Option 1: Remove unused parameter
-def create_progress_bar(description: str = "Processing") -> Progress: +def create_progress_bar() -> Progress: """ Create a Rich progress bar for long-running operations. - Args: - description: Description text for the progress bar. - Returns: Progress: Configured Rich Progress instance. """Option 2: Use the parameter
+from typing import Tuple + -def create_progress_bar(description: str = "Processing") -> Progress: +def create_progress_bar(description: str = "Processing") -> Tuple[Progress, int]: """ Create a Rich progress bar for long-running operations. Args: description: Description text for the progress bar. Returns: - Progress: Configured Rich Progress instance. + Tuple of (Progress instance, task_id). """ - return Progress( + progress = Progress( SpinnerColumn(), TextColumn("[progress.description]{task.description}"), BarColumn(), TaskProgressColumn(), console=console, ) + task_id = progress.add_task(description, total=100) + return progress, task_idtests/tutor/test_progress_tracker.py (1)
7-7: Remove unusedosimport.The
osmodule is imported but not used in this file.Proposed fix
-import os import tempfilecortex/tutor/agents/tutor_agent/tutor_agent.py (1)
86-92: Calling private_runmethod directly on LangChain tools.The
_runmethod is a private API of LangChain'sBaseTool. For tool invocation, the public API isrun()orinvoke(). Using private methods may break if the LangChain library changes its internal implementation.♻️ Suggested refactor
- self.progress_tool._run( - "update_progress", - package_name=package_name, - topic="overview", - ) + self.progress_tool.run( + "update_progress", + package_name=package_name, + topic="overview", + )Apply the same pattern to all other
._run()calls in this file (Lines 155, 156, 165, 177, 192-197, 210, 220).cortex/tutor/tools/deterministic/lesson_loader.py (2)
100-106: Async method delegates to synchronous implementation.The
_arunmethod simply calls the sync_runmethod, which could block the event loop if the SQLite operations are slow. For a deterministic cache lookup, this is likely acceptable, but consider documenting this limitation.
125-129: Silent exception swallowing incache_lesson.Catching all exceptions and returning
Falsehides the root cause of failures. Consider logging the exception for debugging purposes.♻️ Suggested improvement
+import logging + +logger = logging.getLogger(__name__) + def cache_lesson( self, package_name: str, lesson: dict[str, Any], ttl_hours: int = 24, ) -> bool: ... try: self.store.cache_lesson(package_name, lesson, ttl_hours) return True - except Exception: + except Exception as e: + logger.warning("Failed to cache lesson for %s: %s", package_name, e) return Falsecortex/tutor/tools/agentic/lesson_generator.py (2)
21-36: Unused function_load_prompt_template.This function is defined but never called. The prompts are instead hardcoded in
_get_system_promptand_get_generation_promptmethods. Either use this function or remove it.♻️ Option 1: Remove unused code
-# Load prompt template -def _load_prompt_template() -> str: - """Load the lesson generator prompt from file.""" - prompt_path = Path(__file__).parent.parent.parent / "prompts" / "tools" / "lesson_generator.md" - if prompt_path.exists(): - return prompt_path.read_text() - # Fallback inline prompt - return """You are a lesson content generator. Generate comprehensive educational content - for the package: {package_name} - - Student level: {student_level} - Learning style: {learning_style} - Focus areas: {focus_areas} - - Return a JSON object with: summary, explanation, use_cases, best_practices, - code_examples, tutorial_steps, installation_command, confidence.""" -
82-140: DRY violation:_runand_arunhave duplicated logic.The async
_arunmethod duplicates most of the logic from_run, including prompt construction, chain setup, and error handling. Consider extracting shared logic.♻️ Suggested refactor
def _build_chain(self): """Build the LLM chain for lesson generation.""" prompt = ChatPromptTemplate.from_messages( [ ("system", self._get_system_prompt()), ("human", self._get_generation_prompt()), ] ) return prompt | self.llm | JsonOutputParser() def _build_invoke_params(self, package_name, student_level, learning_style, focus_areas, skip_areas): """Build parameters for chain invocation.""" return { "package_name": package_name, "student_level": student_level, "learning_style": learning_style, "focus_areas": ", ".join(focus_areas or []) or "all topics", "skip_areas": ", ".join(skip_areas or []) or "none", } def _run(self, package_name, student_level="beginner", ...): try: chain = self._build_chain() params = self._build_invoke_params(...) result = chain.invoke(params) # ...Also applies to: 142-184
tests/tutor/test_tools.py (1)
280-309: Fragile test setup using__new__to bypass initialization.Using
__new__to create objects without calling__init__is fragile and can break if the class implementation changes. Consider using proper mocking or fixtures.♻️ Suggested improvement
class TestConversationHandler: """Tests for ConversationHandler.""" def test_build_context_empty(self): """Test context building with empty history.""" - with patch("cortex.tutor.tools.agentic.qa_handler.get_config"): - handler = ConversationHandler.__new__(ConversationHandler) - handler.history = [] - + with patch("cortex.tutor.tools.agentic.qa_handler.get_config") as mock_config: + mock_config.return_value = Mock( + anthropic_api_key="test_key", + model="claude-sonnet-4-20250514", + ) + with patch("cortex.tutor.tools.agentic.qa_handler.ChatAnthropic"): + handler = ConversationHandler(package_name="test") + handler.history = [] + context = handler._build_context() assert "Starting fresh" in contextcortex/tutor/tools/agentic/examples_provider.py (2)
7-8: Unused import:Path.The
Pathimport frompathlibis not used anywhere in this file.♻️ Suggested fix
-from pathlib import Path from typing import Any
59-113: DRY violation:_runand_arunduplicate logic.Same issue as in
lesson_generator.py. The async method duplicates the sync implementation. Consider extracting shared chain-building logic.Also applies to: 115-157
cortex/tutor/agents/tutor_agent/graph.py (2)
374-388: Singleton graph creation is not thread-safe.The
get_tutor_graph()function uses a global variable without synchronization. In a multi-threaded environment, multiple threads could create the graph simultaneously.♻️ Suggested thread-safe implementation
+import threading + # Create singleton graph instance _graph = None +_graph_lock = threading.Lock() def get_tutor_graph() -> StateGraph: """ Get the singleton Tutor Agent graph. Returns: Compiled StateGraph. """ global _graph - if _graph is None: - _graph = create_tutor_graph() + if _graph is None: + with _graph_lock: + if _graph is None: # Double-check locking + _graph = create_tutor_graph() return _graph
44-45: Using private_runmethod on LangChain tools throughout graph nodes.Multiple nodes call
tool._run()which is a private API. Consider using the publicrun()orinvoke()methods for better compatibility with future LangChain versions.Also applies to: 62-63, 122-123, 173-174
cortex/tutor/cli.py (3)
155-210: Consider handling missingvalidation_passedkey more defensively.If
agent.ask()returns a malformed response without thevalidation_passedkey,.get("validation_passed")returnsNone, which is falsy and falls through to the error case. This is reasonable behavior, but you may want to log or handle unexpected response formats explicitly for debugging purposes.
213-240: Docstring is missing theverboseparameter.The
cmd_list_packagesfunction accepts averboseparameter but the docstring doesn't document it.📝 Suggested fix
def cmd_list_packages(verbose: bool = False) -> int: """ List packages that have been studied. + Args: + verbose: Enable verbose output. + Returns: Exit code. """
243-313: Docstring is missing theverboseparameter documentation.Similar to
cmd_list_packages, theverboseparameter is not documented in the docstring.📝 Suggested fix
def cmd_progress(package: str | None = None, verbose: bool = False) -> int: """ Show learning progress. Args: package: Optional package filter. + verbose: Enable verbose output. Returns: Exit code. """cortex/tutor/tools/deterministic/progress_tracker.py (2)
115-117: Async implementation blocks the event loop.The
_arunmethod calls the synchronous_runmethod directly, which will block the event loop if_runperforms I/O (SQLite operations). For a truly async implementation, consider usingasyncio.to_thread()or an async-compatible SQLite library.♻️ Suggested fix to avoid blocking the event loop
async def _arun(self, *args: Any, **kwargs: Any) -> dict[str, Any]: """Async version - delegates to sync implementation.""" - return self._run(*args, **kwargs) + import asyncio + return await asyncio.to_thread(self._run, *args, **kwargs)
301-346: Convenience functions create new tool instances per call.Each convenience function (
get_learning_progress,mark_topic_completed,get_package_stats) instantiates a newProgressTrackerTooland thus a newSQLiteStore. This is inefficient for repeated calls and may have connection overhead. Consider caching or accepting an optional tool/store parameter.cortex/tutor/tools/agentic/qa_handler.py (3)
1-8: Unused import:Pathis imported but never used.The
Pathimport frompathlibis not used in this file.🧹 Remove unused import
-from pathlib import Path from typing import Any
118-162: Code duplication between_runand_arun.The sync and async methods have nearly identical logic. Consider extracting the common prompt/chain setup into a helper method.
♻️ Suggested refactor to reduce duplication
def _build_chain(self): """Build the prompt chain for Q&A.""" prompt = ChatPromptTemplate.from_messages( [ ("system", self._get_system_prompt()), ("human", self._get_qa_prompt()), ] ) return prompt | self.llm | JsonOutputParser() def _build_invoke_params(self, package_name, question, learning_style, mastered_concepts, weak_concepts, lesson_context): """Build parameters for chain invocation.""" return { "package_name": package_name, "question": question, "learning_style": learning_style, "mastered_concepts": ", ".join(mastered_concepts or []) or "none specified", "weak_concepts": ", ".join(weak_concepts or []) or "none specified", "lesson_context": lesson_context or "starting fresh", }
268-344: Unbounded history growth in ConversationHandler.The
historylist grows unboundedly. While_build_contextonly uses the last 3 exchanges, the full history is retained. Consider limiting history size to prevent memory growth in long sessions.♻️ Suggested fix to limit history size
+ MAX_HISTORY_SIZE = 10 # Keep last 10 exchanges + def ask( self, question: str, # ... params ) -> dict[str, Any]: # ... existing code ... # Update history if result.get("success"): self.history.append( { "question": question, "answer": result["answer"].get("answer", ""), } ) + # Trim history to prevent unbounded growth + if len(self.history) > self.MAX_HISTORY_SIZE: + self.history = self.history[-self.MAX_HISTORY_SIZE:] return resulttests/tutor/test_branding.py (3)
7-8: Unused imports:StringIO,MagicMock, andMockare imported but never used.These imports are not utilized in the test file.
🧹 Remove unused imports
-from io import StringIO -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import patch
44-70: Tests lack assertions to verify output content.The
TestTutorPrinttests call the functions but don't assert anything about the output. While smoke testing is valuable, consider adding at least basic assertions usingcapsys.readouterr()to verify expected output patterns.💡 Example of adding assertions
def test_tutor_print_success(self, capsys): """Test success status print.""" tutor_print("Test message", "success") captured = capsys.readouterr() assert "Test message" in captured.out or len(captured.out) > 0
73-91: Tests lack meaningful assertions.Similar to
TestTutorPrint, these tests only verify that functions don't raise exceptions. Consider adding assertions to verify output contains expected content.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (46)
cortex/cli.pycortex/tutor/__init__.pycortex/tutor/agents/tutor_agent/__init__.pycortex/tutor/agents/tutor_agent/graph.pycortex/tutor/agents/tutor_agent/state.pycortex/tutor/agents/tutor_agent/tutor_agent.pycortex/tutor/branding.pycortex/tutor/cli.pycortex/tutor/config.pycortex/tutor/contracts/lesson_context.pycortex/tutor/contracts/progress_context.pycortex/tutor/memory/sqlite_store.pycortex/tutor/tests/test_agent_methods.pycortex/tutor/tests/test_agentic_tools.pycortex/tutor/tests/test_branding.pycortex/tutor/tests/test_cli.pycortex/tutor/tests/test_deterministic_tools.pycortex/tutor/tests/test_integration.pycortex/tutor/tests/test_interactive_tutor.pycortex/tutor/tests/test_progress_tracker.pycortex/tutor/tests/test_tools.pycortex/tutor/tests/test_tutor_agent.pycortex/tutor/tests/test_validators.pycortex/tutor/tools/__init__.pycortex/tutor/tools/agentic/__init__.pycortex/tutor/tools/agentic/examples_provider.pycortex/tutor/tools/agentic/lesson_generator.pycortex/tutor/tools/agentic/qa_handler.pycortex/tutor/tools/deterministic/__init__.pycortex/tutor/tools/deterministic/lesson_loader.pycortex/tutor/tools/deterministic/progress_tracker.pycortex/tutor/tools/deterministic/validators.pydocs/AI_TUTOR.mdrequirements.txttests/tutor/__init__.pytests/tutor/test_agent_methods.pytests/tutor/test_agentic_tools.pytests/tutor/test_branding.pytests/tutor/test_cli.pytests/tutor/test_deterministic_tools.pytests/tutor/test_integration.pytests/tutor/test_interactive_tutor.pytests/tutor/test_progress_tracker.pytests/tutor/test_tools.pytests/tutor/test_tutor_agent.pytests/tutor/test_validators.py
✅ Files skipped from review due to trivial changes (1)
- tests/tutor/init.py
🚧 Files skipped from review as they are similar to previous changes (9)
- docs/AI_TUTOR.md
- cortex/tutor/tests/test_cli.py
- cortex/tutor/tests/test_agentic_tools.py
- cortex/tutor/tests/test_tutor_agent.py
- requirements.txt
- cortex/tutor/init.py
- cortex/tutor/agents/tutor_agent/init.py
- cortex/tutor/tests/test_interactive_tutor.py
- cortex/cli.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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:
tests/tutor/test_agentic_tools.pycortex/tutor/tests/test_integration.pytests/tutor/test_tutor_agent.pycortex/tutor/tools/agentic/__init__.pytests/tutor/test_cli.pycortex/tutor/tests/test_agent_methods.pytests/tutor/test_interactive_tutor.pycortex/tutor/tests/test_progress_tracker.pycortex/tutor/tools/deterministic/__init__.pycortex/tutor/config.pytests/tutor/test_deterministic_tools.pycortex/tutor/tools/__init__.pytests/tutor/test_tools.pycortex/tutor/contracts/progress_context.pycortex/tutor/tests/test_tools.pycortex/tutor/branding.pytests/tutor/test_integration.pytests/tutor/test_progress_tracker.pytests/tutor/test_agent_methods.pycortex/tutor/tests/test_validators.pycortex/tutor/agents/tutor_agent/tutor_agent.pycortex/tutor/memory/sqlite_store.pycortex/tutor/agents/tutor_agent/state.pycortex/tutor/tools/deterministic/progress_tracker.pycortex/tutor/agents/tutor_agent/graph.pycortex/tutor/contracts/lesson_context.pycortex/tutor/tools/deterministic/validators.pycortex/tutor/tools/agentic/lesson_generator.pycortex/tutor/tools/deterministic/lesson_loader.pycortex/tutor/tools/agentic/examples_provider.pycortex/tutor/tests/test_deterministic_tools.pycortex/tutor/cli.pytests/tutor/test_validators.pycortex/tutor/tools/agentic/qa_handler.pytests/tutor/test_branding.pycortex/tutor/tests/test_branding.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain >80% test coverage for pull requests
Files:
tests/tutor/test_agentic_tools.pytests/tutor/test_tutor_agent.pytests/tutor/test_cli.pytests/tutor/test_interactive_tutor.pytests/tutor/test_deterministic_tools.pytests/tutor/test_tools.pytests/tutor/test_integration.pytests/tutor/test_progress_tracker.pytests/tutor/test_agent_methods.pytests/tutor/test_validators.pytests/tutor/test_branding.py
{setup.py,setup.cfg,pyproject.toml,**/__init__.py}
📄 CodeRabbit inference engine (AGENTS.md)
Use Python 3.10 or higher as the minimum supported version
Files:
cortex/tutor/tools/agentic/__init__.pycortex/tutor/tools/deterministic/__init__.pycortex/tutor/tools/__init__.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 tests/**/*.py : Maintain >80% test coverage for pull requests
Applied to files:
cortex/tutor/tests/test_progress_tracker.py
🔇 Additional comments (77)
cortex/tutor/tests/test_validators.py (2)
26-90: Thorough package name validation tests.Good coverage of edge cases including valid names, empty input, whitespace, length limits, blocked patterns, and invalid characters. The blocked pattern tests appropriately cover dangerous shell commands.
266-302: Well-structured integration tests for validation workflow.The
TestGetValidationErrorsandTestValidateAllclasses properly test the aggregation and combined validation flows. Good use of multiple validation types in combination.cortex/tutor/memory/sqlite_store.py (2)
362-390: Race condition handling in_create_default_profileis well implemented.Good use of
INSERT OR IGNOREfollowed by a re-fetch to handle concurrent profile creation attempts. The re-entrant lock (RLock) also helps with nested calls.
474-496: Lesson cache expiration logic is correct.The TTL-based cache properly uses UTC timestamps and the expiration check in
get_cached_lessoncorrectly filters expired entries.cortex/tutor/tools/agentic/__init__.py (1)
1-17: Clean package initialization with proper exports.Well-structured
__init__.pywith a clear docstring explaining the purpose of agentic tools. The__all__properly defines the public API surface.cortex/tutor/tests/test_deterministic_tools.py (3)
23-58: Good cache behavior tests for LessonLoaderTool.Tests properly verify cache miss, cache storage/retrieval, and
force_freshbypass. The use of a temporary database fixture ensures test isolation.
174-179: Good coverage of error handling for invalid actions.The test properly verifies that invalid actions return a failure response with an error message.
89-95: Ensure fallback validation is exhaustive.The test iterates over
FALLBACK_LESSONSbut only checks forpackage_nameandsummaryfields. Consider verifying all required fields that downstream code depends on.tests/tutor/test_validators.py (1)
1-303: Verify whether duplicate test files exist.This review raises a concern about potential duplicate test files. Before acting on this claim:
- Confirm that both
cortex/tutor/tests/test_validators.pyandtests/tutor/test_validators.pyexist in the current state- If both exist, compare them to verify they are identical
- If confirmed as duplicates, determine which should be removed based on your refactoring intent
tests/tutor/test_agentic_tools.py (1)
12-197: Test structure and coverage look solid.The tests properly mock external dependencies (
get_config,ChatAnthropic) and validate_structure_responsebehavior across full and minimal response scenarios for each agentic tool. TheConversationHandlercontext-building tests cover both empty and populated history states.cortex/tutor/tools/deterministic/__init__.py (1)
1-17: Clean package initialization with appropriate exports.The module docstring clearly explains that these are deterministic (non-LLM) tools, and
__all__properly exposes the public API.cortex/tutor/tests/test_agent_methods.py (1)
37-441: Comprehensive test coverage for TutorAgent orchestration.Tests thoroughly cover agent methods, graph node behaviors (success/failure/exception paths), routing decisions, and state helpers. The mocking strategy properly isolates units from external dependencies.
cortex/tutor/tests/test_branding.py (1)
30-227: UI smoke tests provide reasonable coverage.The tests verify that branding functions execute without errors across various input scenarios. For Rich console output, smoke testing is a practical approach since exact output assertions would be brittle.
tests/tutor/test_tutor_agent.py (1)
33-321: Well-structured tests covering state management, graph nodes, and routing.Tests comprehensively validate state creation, error/checkpoint/cost accumulation, critical error detection, and routing logic. The integration tests properly verify input validation.
tests/tutor/test_deterministic_tools.py (1)
1-179: Excellent test coverage for deterministic tools.Tests properly isolate database operations using temporary directories, cover all
ProgressTrackerToolactions including error handling for invalid actions, and verify fallback lesson content including case-insensitivity and required fields. The fixture pattern ensures clean test isolation.cortex/tutor/tests/test_integration.py (1)
298-364: Solid end-to-end workflow test.The test validates the complete cache-hit lesson workflow through
plan_node → load_cache_node → reflect_node, verifying state transitions and final output validation. Good use of mocking to isolate from actual LLM calls.tests/tutor/test_interactive_tutor.py (1)
84-123: Well-structured test fixture.The
mock_tutorfixture provides comprehensive mock data covering all lesson components (examples, tutorial steps, best practices) with properly configured agent mocks. This enables clean, focused test methods.tests/tutor/test_agent_methods.py (2)
37-198: Good coverage of TutorAgent public API.The tests comprehensively cover all public methods (
teach,ask,get_profile,update_learning_style,mark_completed,reset_progress,get_packages_studied) with proper mocking of dependencies.
200-259: Comprehensive node testing with error scenarios.The
generate_lesson_nodetests cover success, API failure, and exception handling paths. The caching behavior on success is properly verified.tests/tutor/test_integration.py (1)
32-81: Config tests are comprehensive.Good coverage of environment loading, API key validation, path expansion, and directory creation with proper use of
monkeypatchandtempfilefor isolation.tests/tutor/test_cli.py (3)
24-87: Parser tests are thorough.All CLI flags and arguments are tested including edge cases like
--resetwith and without package argument.
89-149: Good security test for blocked patterns.The
test_blocked_package_nametest verifies that potentially dangerous input like"rm -rf /"is rejected, which is important for security.
401-453: Main entry point routing is well-tested.Tests verify that CLI arguments are correctly routed to their respective command handlers.
cortex/tutor/tests/test_progress_tracker.py (2)
47-212: Excellent SQLiteStore test coverage.Comprehensive testing of all store operations including CRUD, statistics, profile management, lesson caching with TTL, and reset functionality.
214-288: Good action-based testing of ProgressTrackerTool.All supported actions are tested, including error cases for unknown actions and missing required parameters.
cortex/tutor/tests/test_tools.py (1)
31-122: Good caching and fallback behavior testing.Tests cover cache hit/miss scenarios, force-refresh, fallback templates, and the priority order (cache → fallback → none).
cortex/tutor/config.py (2)
127-141: DocstringRaisesis accurate but indirect.The docstring correctly notes
ValueErrorcan be raised, which happens viaConfig.from_env(). This is accurate.
1-16: Well-structured configuration module.Good use of Pydantic for validation, proper docstrings, type hints, and secure defaults (0o700 permissions for data directory). The environment-based configuration with optional API key requirement enables the lazy import pattern mentioned in the PR objectives.
cortex/tutor/tools/__init__.py (1)
1-20: LGTM!Clean package initialization with proper docstring and centralized exports. The
__all__list correctly exposes the public API for both deterministic and agentic tools.cortex/tutor/contracts/lesson_context.py (2)
101-131: Well-designed serialization and utility methods.The
to_json(),from_json(), andto_display_dict()methods provide a clean API for caching and presentation. Good use of Pydantic's built-in serialization.
56-75: Pydantic v2max_lengthon list fields correctly constrains list size.In Pydantic v2,
Field(max_length=N)on a list field constrains the number of items in the list, not the length of individual string elements. The code is using this correctly and requires no changes.Likely an incorrect or invalid review comment.
cortex/tutor/branding.py (2)
34-50: Well-designed status printing utility.Clean implementation with proper fallback to default config. The status-emoji mapping provides a consistent UI experience.
193-216: Good defensive error handling inget_user_input.Properly handles
EOFErrorandKeyboardInterruptto gracefully cancel operations without crashing.tests/tutor/test_progress_tracker.py (3)
27-44: Well-structured test fixtures.Good use of
tempfile.TemporaryDirectorycontext manager withyieldfor automatic cleanup. The fixture chain (temp_db→store/tracker) provides proper test isolation.
278-288: Good error case coverage.Tests for unknown actions and missing required parameters ensure the tool gracefully handles invalid inputs.
293-350: Proper mocking pattern for convenience functions.Good use of
patchto substitute config, ensuring tests are isolated from the actual database path. The tests verify end-to-end behavior through the public API.cortex/tutor/contracts/progress_context.py (2)
31-63: Well-designed computed fields with proper edge case handling.The
@computed_fielddecorators correctly implement derived properties. Theis_complete()method properly returnsFalsefor empty topic lists, preventing false positives.
159-176: Clean factory method pattern.
from_resultsproperly guards against division by zero and provides sensible defaults. The 70% pass threshold is a reasonable default for quiz assessment.cortex/tutor/agents/tutor_agent/tutor_agent.py (3)
265-275: Lazy imports insidestart()method are appropriate.The PR objectives mention "lazy imports to allow listing/progress commands without an API key." This approach is intentional and correctly defers heavy imports until interactive mode is needed.
309-313: Consider validating menu option range before conversion.The input handling catches
ValueErrorfor non-numeric input, but the range check happens after successful conversion. This is acceptable but could provide clearer feedback.
23-48: TutorAgent class is well-structured with proper docstrings and type hints.The class follows PEP 8 style, includes comprehensive docstrings with examples, and uses type hints throughout. The initialization pattern is clean.
cortex/tutor/tools/deterministic/lesson_loader.py (2)
163-232: FALLBACK_LESSONS provides good offline resilience.The predefined fallback lessons for common packages (docker, git, nginx) ensure the tutor can function without API access. The lower confidence score (0.7) appropriately signals these are fallback content.
18-49: LessonLoaderTool class is well-documented with proper type hints.The class follows PEP 8, has comprehensive docstrings, and uses Pydantic fields appropriately. The Config class enables arbitrary types for the SQLiteStore field.
cortex/tutor/tools/agentic/lesson_generator.py (2)
262-276: Good defensive coding in_structure_response.The method properly handles missing fields with defaults, clamps confidence to valid range (0.0-1.0), and limits list sizes to prevent unbounded responses. This is well-implemented.
186-202: System prompt has appropriate safety rules.The critical rules against fabricating features, URLs, or versions are important for educational content integrity. Good security posture.
tests/tutor/test_tools.py (2)
24-28: Good use of pytest fixture for test isolation.The
temp_dbfixture properly creates an isolated temporary database for each test, ensuring tests don't interfere with each other.
127-181: Test mocking is thorough and at the correct level.The tests mock
ChatAnthropicandget_configat the appropriate module level, and verify the tool's response structure without requiring actual LLM calls.cortex/tutor/tools/agentic/examples_provider.py (2)
159-174: System prompt enforces good safety practices.The critical rules against inventing flags, using real credentials, and flagging dangerous commands are appropriate for educational content generation.
208-236: Good defensive coding in_structure_response.The method properly validates example structure, limits list sizes, and provides sensible defaults. The confidence clamping is correctly implemented.
cortex/tutor/agents/tutor_agent/graph.py (2)
200-249: Reflect node has well-designed validation logic.The confidence calculation accounting for errors and cache staleness, combined with comprehensive output preparation, demonstrates good design. The validation_passed flag provides clear success/failure indication.
315-371: Graph structure is well-organized with clear entry point and edges.The workflow correctly implements the Plan→Act→Reflect pattern with proper conditional routing and terminal states. The conditional edges provide flexibility for different session types.
cortex/tutor/agents/tutor_agent/state.py (4)
50-105: Well-designed TypedDict state schema.The
TutorAgentStateprovides a clear contract for the workflow state with comprehensive documentation. Usingtotal=Falseallows optional fields while maintaining type safety.
107-150: Factory function provides clean state initialization.
create_initial_statesets sensible defaults for all fields, ensuring the state is always in a valid initial configuration. The explicit initialization prevents undefined state issues.
153-171: Helper functions mutate state in-place.The
add_error,add_checkpoint, andadd_costfunctions modify the state dictionary directly. This is appropriate for LangGraph workflows where state is passed through nodes and accumulated.
42-47: ErrorState usestotal=True(default) enforcing required fields.Unlike other state TypedDicts,
ErrorStaterequires all fields (node,error,recoverable), which is appropriate since an error entry should always have complete information.cortex/tutor/cli.py (5)
1-33: LGTM! Well-structured module docstring and imports.The module docstring clearly documents CLI usage patterns. Lazy imports strategy for API-key-dependent modules is a good design choice. The
DEFAULT_TUTOR_TOPICSconstant is appropriately defined at module level.
35-109: LGTM! Argument parser is well-configured.The parser correctly handles all required CLI options including version, verbose mode, package name, list, progress, reset, fresh, and question flags. The
--resetwithnargs="?"andconst="__all__"is a nice pattern for optional argument handling.
112-152: LGTM! Solid error handling in teach command.The function properly validates input before expensive imports, handles
ValueError,KeyboardInterrupt, and general exceptions with appropriate exit codes. The lazy import pattern ensures API key is only required when actually starting a session.
316-347: LGTM! Reset command with proper confirmation flow.The confirmation prompt before destructive operations is good UX practice. The scope calculation handles both
Noneand"__all__"correctly.
350-392: LGTM! Clean command dispatch logic.The
mainfunction correctly handles argument parsing, conditional banner display, and command routing. The fallback toparser.print_help()when no command is specified is appropriate.cortex/tutor/tools/deterministic/validators.py (7)
15-34: LGTM! Comprehensive security patterns for input validation.The blocked patterns list covers common injection vectors (shell commands, fork bombs, Python code injection). This is a solid defense-in-depth approach for a CLI tool that might pass user input to an LLM.
36-48: LGTM! Well-defined validation patterns and categories.The regex pattern for valid package names is appropriately restrictive while allowing common naming conventions (dots, hyphens, underscores). The package categories list provides good context for documentation.
51-93: LGTM! Thorough package name validation with clear error messages.The function correctly handles empty input, length limits, blocked patterns, and format validation. The docstring includes helpful examples demonstrating expected behavior.
96-135: LGTM! Generic input validation with configurable parameters.The function properly handles the
allow_emptyflag and applies length limits before checking blocked patterns.
138-210: LGTM! Specialized validators with appropriate constraints.The
validate_question,validate_topic,validate_score, andvalidate_learning_stylefunctions properly delegate to base validators or implement specific rules. The score validation correctly handles both int and float types.
213-277: LGTM! Input sanitization and package name extraction utilities.The
sanitize_inputfunction removes null bytes and enforces length limits. Theextract_package_namefunction uses a reasonable set of patterns to extract package names from natural language input.
280-367: LGTM! Clean aggregation utilities with ValidationResult class.The
get_validation_errorsandvalidate_allfunctions provide convenient batch validation. TheValidationResultclass implements__bool__and__str__for ergonomic usage.cortex/tutor/tools/deterministic/progress_tracker.py (5)
23-59: LGTM! Well-documented tool class with proper initialization.The class docstring clearly explains the tool's purpose and constraints (no LLM calls). The initialization correctly handles optional
db_pathwith fallback to config default.
61-113: LGTM! Clean action dispatcher with error handling.The dispatcher pattern is well-implemented with a dictionary mapping actions to handlers. Unknown actions return helpful error messages with valid action lists.
119-164: LGTM! Progress retrieval handlers with proper validation.Both
_get_progressand_get_all_progresscorrectly handle missing required parameters and return structured responses.
166-213: LGTM! Progress update handlers with proper data preservation.The
_mark_completedand_update_progresshandlers correctly preserve existing values when updating progress. The time accumulation logic in_update_progressis correctly implemented.
215-295: LGTM! Stats and profile management handlers.The handlers correctly implement statistics retrieval, profile management, and progress reset functionality. Error handling is consistent across all methods.
cortex/tutor/tools/agentic/qa_handler.py (4)
59-116: LGTM! Robust error handling and response structuring.The
_runmethod properly wraps LLM calls in try-except, structures responses consistently, and includes cost estimation. The chain composition withprompt | self.llm | JsonOutputParser()is idiomatic LangChain.
164-213: LGTM! Well-designed system and Q&A prompts.The system prompt includes critical safety rules (no fabrication, no fake URLs, confidence expression). The Q&A prompt template defines a clear JSON schema for structured responses.
215-241: LGTM! Response structuring with validation.The
_structure_responsemethod properly clamps confidence to [0.0, 1.0], limits array lengths for related topics and suggestions, and safely handles missing or malformed code examples.
36-57: Model name is valid and pricing estimate is reasonable.The model identifier
claude-sonnet-4-20250514is confirmed as a valid current Anthropic model. Current pricing is $3.00 per 1M input tokens and $15.00 per 1M output tokens. With max_tokens set to 2048, the estimated cost of ~$0.02 per question is reasonable for typical usage patterns.tests/tutor/test_branding.py (1)
229-249: LGTM! User input tests with proper mocking and assertions.These tests correctly mock
builtins.inputand verify the expected behavior. The test at line 245-249 correctly tests that raw input is returned (checking"spaced" in resultrather than exact match).
|
@Anshgrover23 All checks are now passing! ✅ Summary of updates:
Ready for review. Please take a look when you get a chance. Thanks! |
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
@srikrishnavansi Your SonarQubeCloud Quality Gate is failing kindly address that issues. Also address coderabbit comments that are still open.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @cortex/tutor/tools/deterministic/progress_tracker.py:
- Around line 1-15: The file fails CI due to Black formatting; run the formatter
on this module (e.g., black
cortex/tutor/tools/deterministic/progress_tracker.py) and commit the changes so
imports and docstring adhere to Black style; ensure the module-level docstring,
imports (Path, Any, get_config, LearningProgress, SQLiteStore) and blank lines
are reformatted by Black and then push the reformatted file.
- Around line 59-68: The broad except Exception in the actions dispatch swallows
unexpected errors and loses stack traces; replace it by catching only expected
exception types (e.g., ValueError/KeyError or your domain-specific errors) or,
if you must handle all exceptions here, log the full traceback before returning
the error dict (use logging.exception or include traceback.format_exc()) for the
call to actions[action](...) so you surface debugging details while still
returning {"success": False, "error": ...}.
- Around line 232-240: The _reset_progress tool currently performs a destructive
delete without safeguards; add a boolean dry_run parameter to _reset_progress
(e.g., dry_run: bool = False) that when true calls
self.store.count_progress(package_name) or otherwise returns the count without
calling self.store.reset_progress, and add an explicit confirmation parameter
(e.g., confirm: bool = False) that must be True for deletion to proceed; update
the returned payload to indicate dry_run vs actual deletion and modify the tool
registration (the action registered at line 49) to expose these new parameters
or require confirm=True to execute deletion so agents cannot delete records
accidentally.
🧹 Nitpick comments (2)
cortex/tutor/tools/deterministic/progress_tracker.py (2)
208-230: Redundantkwargs.get("concept")extraction.Lines 214 and 226 have
concept = kwargs.get("concept") or concept, butconceptis already passed as a direct parameter from_run. The kwargs extraction is unnecessary since the dispatcher doesn't passconceptvia kwargs—it's a named parameter.Suggested simplification
def _add_mastered_concept( self, concept: str | None = None, **kwargs: Any, ) -> dict[str, Any]: """Add a mastered concept to student profile.""" - concept = kwargs.get("concept") or concept if not concept: return {"success": False, "error": "concept required"} self.store.add_mastered_concept(concept) return {"success": True, "message": f"Added mastered concept: {concept}"} def _add_weak_concept( self, concept: str | None = None, **kwargs: Any, ) -> dict[str, Any]: """Add a weak concept to student profile.""" - concept = kwargs.get("concept") or concept if not concept: return {"success": False, "error": "concept required"} self.store.add_weak_concept(concept) return {"success": True, "message": f"Added weak concept: {concept}"}
248-269: Convenience functions call private_runmethod.These public API functions call
tool._run(), which by Python convention (underscore prefix) is a private method. This couples the public API to internal implementation details.Consider either renaming
_runtorunif it's intended as part of the public interface, or adding thin public methods on the class that these convenience functions can call.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cortex/tutor/tools/deterministic/lesson_loader.pycortex/tutor/tools/deterministic/progress_tracker.py
🚧 Files skipped from review as they are similar to previous changes (1)
- cortex/tutor/tools/deterministic/lesson_loader.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide for Python code
Include type hints in Python code
Add docstrings for all public APIs in Python code
Use dry-run mode by default for all installation operations
Do not use silent sudo - require explicit user confirmation for privilege escalation
Implement Firejail sandboxing for execution of untrusted code
Log all operations to ~/.cortex/history.db for audit purposes
Files:
cortex/tutor/tools/deterministic/progress_tracker.py
🪛 GitHub Actions: CI
cortex/tutor/tools/deterministic/progress_tracker.py
[error] 1-1: Black formatting check failed: 1 file would be reformatted. Command: 'black --check . --exclude "(venv|\.venv|build|dist)"'.
⏰ 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.12)
- GitHub Check: test (3.11)
- GitHub Check: test (3.10)
| class ProgressTrackerTool: | ||
| """Deterministic tool for tracking learning progress.""" | ||
|
|
||
| _ERR_PKG_TOPIC_REQUIRED: str = "package_name and topic required" | ||
|
|
||
| def __init__(self, db_path: Path | None = None) -> None: | ||
| """Initialize the progress tracker tool.""" | ||
| if db_path is None: | ||
| config = get_config() | ||
| db_path = config.get_db_path() | ||
| self.store = SQLiteStore(db_path) |
There was a problem hiding this comment.
Missing audit logging per coding guidelines.
The coding guidelines require logging all operations to ~/.cortex/history.db for audit purposes. This tool performs various write operations (mark completed, update progress, reset progress, update profile) but doesn't implement any audit logging.
Consider adding a logging mechanism to track mutations, especially for reset_progress which is destructive.
- Fix Black formatting in progress_tracker.py - Add logger.exception for better debugging on failures
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @cortex/tutor/tools/deterministic/progress_tracker.py:
- Around line 121-137: The code in _mark_completed incorrectly treats 0.0 as
falsy by using "score or 1.0"; change the logic to only default when score is
None (e.g., compute final_score = score if score is not None else 1.0) and pass
final_score to self.store.mark_topic_completed and include final_score in the
returned dict so a legitimate 0.0 score is preserved.
- Around line 155-161: The LearningProgress construction uses falsy checks that
drop valid zero scores and can reset a previously completed topic; change score
to use an explicit None check (score if score is not None else (existing.score
if existing else 0.0)) and compute completed by preserving an already-completed
state (e.g. completed = (existing.completed if existing else False) or
completed) before building the LearningProgress instance so adding time doesn't
overwrite a prior completed=True and 0.0 scores remain unchanged.
🧹 Nitpick comments (4)
cortex/tutor/tools/deterministic/progress_tracker.py (4)
62-72: Add audit logging for operations to ~/.cortex/history.db.Per coding guidelines, all operations should be logged to
~/.cortex/history.dbfor audit purposes. The dispatcher is an ideal location to add this, capturing each action with relevant parameters. Consider logging successful operations and failures separately.Based on learnings, operations should be logged to history.db for audit purposes.
♻️ Suggested implementation
try: - return actions[action]( + result = actions[action]( package_name=package_name, topic=topic, score=score, time_seconds=time_seconds, **kwargs, ) + logger.info( + "Progress action=%s package=%s topic=%s success=%s", + action, + package_name, + topic, + result.get("success", False), + ) + return result except Exception as e: logger.exception("Progress tracker action '%s' failed", action) return {"success": False, "error": str(e)}Note: The above adds basic logging. For full audit compliance, you may need to wire this into the
history.dbaudit store used elsewhere in cortex.
212-222: Redundant kwargs lookup forconcept.The
concept = kwargs.get("concept") or conceptline is unnecessary. Sinceconceptis already a named parameter, callers would pass it directly. The same applies to_add_weak_concept.♻️ Suggested simplification
def _add_mastered_concept( self, concept: str | None = None, **kwargs: Any, ) -> dict[str, Any]: """Add a mastered concept to student profile.""" - concept = kwargs.get("concept") or concept if not concept: return {"success": False, "error": "concept required"}
259-277: Convenience functions call private_runmethod and create new instances per call.These functions invoke
_run(a private method by convention). Consider either:
- Renaming
_runtorunif it's intended for public use, or- Adding dedicated public methods to
ProgressTrackerToolthat these functions delegate to.Additionally, each call instantiates a new
ProgressTrackerTool(and thus a newSQLiteStore). For one-off usage this is fine, but if these are called repeatedly, consider a module-level singleton or passing in a shared instance.♻️ Alternative approach with public methods
# Add public methods to ProgressTrackerTool: class ProgressTrackerTool: # ... existing code ... def get_progress(self, package_name: str, topic: str) -> dict[str, Any] | None: """Public API: Get learning progress for a specific topic.""" result = self._run("get_progress", package_name=package_name, topic=topic) return result.get("progress") # Then convenience functions become: _default_tracker: ProgressTrackerTool | None = None def _get_tracker() -> ProgressTrackerTool: global _default_tracker if _default_tracker is None: _default_tracker = ProgressTrackerTool() return _default_tracker def get_learning_progress(package_name: str, topic: str) -> dict[str, Any] | None: """Get learning progress for a specific topic.""" return _get_tracker().get_progress(package_name, topic)
236-248: Destructive reset operation lacks safeguards.
_reset_progresspermanently deletes progress records. While this is internal tool code (not an installation operation), consider adding aconfirm: bool = Falseparameter or returning a preview before deletion for safety, especially since this could be invoked through the CLI.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cortex/tutor/tools/deterministic/progress_tracker.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide for Python code
Include type hints in Python code
Add docstrings for all public APIs in Python code
Use dry-run mode by default for all installation operations
Do not use silent sudo - require explicit user confirmation for privilege escalation
Implement Firejail sandboxing for execution of untrusted code
Log all operations to ~/.cortex/history.db for audit purposes
Files:
cortex/tutor/tools/deterministic/progress_tracker.py
🧠 Learnings (1)
📚 Learning: 2026-01-12T20:51:13.817Z
Learnt from: CR
Repo: cortexlinux/cortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-12T20:51:13.817Z
Learning: Applies to **/*.py : Log all operations to ~/.cortex/history.db for audit purposes
Applied to files:
cortex/tutor/tools/deterministic/progress_tracker.py
🧬 Code graph analysis (1)
cortex/tutor/tools/deterministic/progress_tracker.py (3)
cortex/tutor/config.py (2)
get_config(130-143)get_db_path(103-111)cortex/tutor/memory/sqlite_store.py (3)
LearningProgress(19-28)get_progress(146-173)upsert_progress(209-250)cortex/tutor/contracts/progress_context.py (1)
total_time_seconds(50-52)
⏰ 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). (5)
- GitHub Check: Test (Python 3.11)
- GitHub Check: Test (Python 3.10)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.10)
🔇 Additional comments (2)
cortex/tutor/tools/deterministic/progress_tracker.py (2)
1-17: LGTM!The module docstring clearly communicates the deterministic nature (no LLM calls), and imports are appropriate. The
StudentProfiletype is accessed via the store's return value, which is acceptable.
20-30: LGTM!Good use of a class-level constant for the repeated error message, proper type hints, and sensible initialization that defaults to the global config's DB path.
- Use explicit None check instead of falsy check for score - Preserve existing completed state when updating progress
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @cortex/tutor/tools/deterministic/progress_tracker.py:
- Around line 140-179: The code in _update_progress treats completed as a
boolean defaulting to False and checks it with "if completed:", so callers
cannot explicitly set completed=False to revert completion; change the parameter
to accept None (e.g., completed: bool | None = None) and replace the branch with
"if completed is not None: final_completed = completed else: final_completed =
existing.completed if existing else False" (using the existing variable names
_update_progress, completed, existing, final_completed) so explicit False is
honored; update any call sites if they relied on the old default.
🧹 Nitpick comments (4)
cortex/tutor/tools/deterministic/progress_tracker.py (4)
62-72: Missing audit logging for operations.Per coding guidelines, all operations should be logged to
~/.cortex/history.dbfor audit purposes. Currently only exceptions are logged. Consider adding audit logging for successful operations as well, especially for state-mutating actions likemark_completed,update_progress, andreset.Based on learnings, operations should be logged for audit purposes.
224-246: Redundant kwargs lookup for named parameter.The
conceptparameter is already defined as a named parameter, sokwargs.get("concept") or conceptis confusing. This suggests the dispatcher's parameter passing design could be cleaner—either passconceptexplicitly in_runor rely solely on**kwargs.Simpler approach if concept should only come from kwargs
def _add_mastered_concept( self, - concept: str | None = None, **kwargs: Any, ) -> dict[str, Any]: """Add a mastered concept to student profile.""" - concept = kwargs.get("concept") or concept + concept = kwargs.get("concept") if not concept: return {"success": False, "error": "concept required"} self.store.add_mastered_concept(concept) return {"success": True, "message": f"Added mastered concept: {concept}"}Apply the same pattern to
_add_weak_concept.
271-289: Convenience functions create new instances on each call.Each convenience function instantiates a new
ProgressTrackerTool(and thus a newSQLiteStore/DB connection). For batch operations or frequent calls, this adds overhead. Additionally, these public functions call the private_runmethod.Consider either:
- Accepting an optional
toolparameter for reuse, or- Providing a module-level singleton/factory pattern.
Example with optional tool reuse
-def get_learning_progress(package_name: str, topic: str) -> dict[str, Any] | None: +def get_learning_progress( + package_name: str, + topic: str, + tool: ProgressTrackerTool | None = None, +) -> dict[str, Any] | None: """Get learning progress for a specific topic.""" - tool = ProgressTrackerTool() + tool = tool or ProgressTrackerTool() result = tool._run("get_progress", package_name=package_name, topic=topic) return result.get("progress")
271-276: Public API docstrings could be more detailed.Per coding guidelines, public APIs should have comprehensive docstrings. These convenience functions are part of the public API (no underscore prefix) but lack
Args,Returns, andRaisesdocumentation.Example enhanced docstring
def get_learning_progress(package_name: str, topic: str) -> dict[str, Any] | None: """Get learning progress for a specific topic. Args: package_name: The package being studied (e.g., "docker"). topic: The specific topic within the package (e.g., "containers"). Returns: Progress dictionary with keys (package_name, topic, completed, score, last_accessed, total_time_seconds) if found, None otherwise. """
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cortex/tutor/tools/deterministic/progress_tracker.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide for Python code
Include type hints in Python code
Add docstrings for all public APIs in Python code
Use dry-run mode by default for all installation operations
Do not use silent sudo - require explicit user confirmation for privilege escalation
Implement Firejail sandboxing for execution of untrusted code
Log all operations to ~/.cortex/history.db for audit purposes
Files:
cortex/tutor/tools/deterministic/progress_tracker.py
🧠 Learnings (1)
📚 Learning: 2026-01-12T20:51:13.817Z
Learnt from: CR
Repo: cortexlinux/cortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-12T20:51:13.817Z
Learning: Applies to **/*.py : Log all operations to ~/.cortex/history.db for audit purposes
Applied to files:
cortex/tutor/tools/deterministic/progress_tracker.py
⏰ 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). (4)
- GitHub Check: Test (Python 3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.10)
- GitHub Check: test (3.11)
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cortex/tutor/tools/deterministic/progress_tracker.py (2)
224-246: Redundant kwargs lookup is dead code.Lines 230 and 242 use
kwargs.get("concept") or concept, but when_runcalls these methods with**kwargs, Python extracts matching named parameters during argument unpacking. Sinceconceptis a named parameter, it's extracted from kwargs before the method body executes—meaningkwargs.get("concept")will always returnNone.Suggested simplification
def _add_mastered_concept( self, concept: str | None = None, **kwargs: Any, ) -> dict[str, Any]: """Add a mastered concept to student profile.""" - concept = kwargs.get("concept") or concept if not concept: return {"success": False, "error": "concept required"} self.store.add_mastered_concept(concept) return {"success": True, "message": f"Added mastered concept: {concept}"} def _add_weak_concept( self, concept: str | None = None, **kwargs: Any, ) -> dict[str, Any]: """Add a weak concept to student profile.""" - concept = kwargs.get("concept") or concept if not concept: return {"success": False, "error": "concept required"} self.store.add_weak_concept(concept) return {"success": True, "message": f"Added weak concept: {concept}"}
271-289: Consider reusing tool instance for repeated calls.Each convenience function instantiates a new
ProgressTrackerTool, which creates a newSQLiteStoreconnection. For sporadic use this is fine, but if these functions are called in a loop or frequently, consider accepting an optional tool instance parameter or using a module-level singleton to avoid repeated connection overhead.
Clarify audit logging requirement for tutor progress operations.
The coding guidelines specify logging all operations to
~/.cortex/history.db, but the codebase showshistory.dbis used exclusively byInstallationHistoryfor package operations, not learning progress tracking. The progress tracker already maintains audit trails via SQLiteStore with timestamps. Verify whether explicithistory.dblogging is required for tutor operations or if the current timestamp-based SQLite approach satisfies audit requirements.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cortex/tutor/tools/deterministic/progress_tracker.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide for Python code
Include type hints in Python code
Add docstrings for all public APIs in Python code
Use dry-run mode by default for all installation operations
Do not use silent sudo - require explicit user confirmation for privilege escalation
Implement Firejail sandboxing for execution of untrusted code
Log all operations to ~/.cortex/history.db for audit purposes
Files:
cortex/tutor/tools/deterministic/progress_tracker.py
🧠 Learnings (1)
📚 Learning: 2026-01-12T20:51:13.817Z
Learnt from: CR
Repo: cortexlinux/cortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-12T20:51:13.817Z
Learning: Applies to **/*.py : Log all operations to ~/.cortex/history.db for audit purposes
Applied to files:
cortex/tutor/tools/deterministic/progress_tracker.py
🔇 Additional comments (5)
cortex/tutor/tools/deterministic/progress_tracker.py (5)
1-30: LGTM!Module setup and class initialization are clean. Good use of a class-level constant for the repeated error message, and the default path resolution via
get_config()is appropriate.
32-72: LGTM!The dispatcher pattern is clean and provides consistent error handling with informative messages for unknown actions.
74-179: LGTM!The progress tracking handlers correctly implement the validation logic and preserve existing values when parameters are not explicitly provided. The fix for allowing explicit
completed=False(lines 157-160) addresses the commit message concern properly.
181-222: LGTM!Statistics and profile handlers are well-implemented with proper validation and safe division handling.
248-265: LGTM!Reset and package listing handlers are straightforward with clear success messages.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/AI_TUTOR.md`:
- Line 6: Coverage is currently 74%, below the 80% requirement; add focused
tests to exercise uncovered logic (unit tests, integration tests, and edge
cases) in modules with low coverage until overall coverage reaches >=80%, rerun
the coverage tool to confirm, then update the markdown badge at the top (the
line with the coverage badge) and the "test coverage" section (around the noted
section) to reflect the new percentage and test count; ensure CI passes and the
badge percent and test count are accurate before merging.
♻️ Duplicate comments (1)
cortex/cli.py (1)
2361-2376: Verify argument definitions are not duplicated.A previous review flagged that tutor argument definitions may be duplicated between this file and
cortex/tutor/cli.py. Ifcortex/tutor/cli.pyhas its own parser setup, consider centralizing the argument definitions to avoid inconsistencies when arguments are updated.#!/bin/bash # Check if cortex/tutor/cli.py defines its own argument parser for tutor rg -n "add_argument|add_parser|ArgumentParser" cortex/tutor/cli.py
🧹 Nitpick comments (1)
docs/AI_TUTOR.md (1)
92-92: Consider adding language specifiers to ASCII art blocks.The fenced code blocks containing ASCII art diagrams (lines 92, 250, 310, 349, 390, 432) don't have language specifiers, which markdown linters flag. While not critical, adding
textorplaintextimproves rendering consistency:♻️ Example fix
-``` +```text ___ _ _ _ _ _ _____ _ |_ _|_ __ | |_ ___| | (_) __ _ ___ _ __ | |_ |_ _| _| |_ ___ _ __Apply the same pattern to the architecture diagrams at lines 250, 310, 349, 390, and 432.
Also applies to: 250-250, 310-310, 349-349, 390-390, 432-432
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cortex/cli.pydocs/AI_TUTOR.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide for Python code
Include type hints in Python code
Add docstrings for all public APIs in Python code
Use dry-run mode by default for all installation operations
Do not use silent sudo - require explicit user confirmation for privilege escalation
Implement Firejail sandboxing for execution of untrusted code
Log all operations to ~/.cortex/history.db for audit purposes
Files:
cortex/cli.py
🧠 Learnings (2)
📚 Learning: 2026-01-12T20:51:13.828Z
Learnt from: CR
Repo: cortexlinux/cortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-12T20:51:13.828Z
Learning: Add documentation for all new features
Applied to files:
docs/AI_TUTOR.md
📚 Learning: 2026-01-12T20:51:13.828Z
Learnt from: CR
Repo: cortexlinux/cortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-12T20:51:13.828Z
Learning: Applies to tests/**/*.py : Maintain test coverage above 80% for pull requests
Applied to files:
docs/AI_TUTOR.md
🧬 Code graph analysis (1)
cortex/cli.py (3)
cortex/tutor/branding.py (1)
print_banner(52-68)cortex/tutor/cli.py (2)
cmd_question(152-207)cmd_teach(109-149)cortex/branding.py (1)
cx_print(49-69)
🪛 markdownlint-cli2 (0.18.1)
docs/AI_TUTOR.md
92-92: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
250-250: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
310-310: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
349-349: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
390-390: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
432-432: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (4)
cortex/cli.py (2)
1183-1241: LGTM! Clean implementation of the tutor command handler.The method follows good practices:
- Proper docstring with Args/Returns
- Lazy imports to avoid loading API-key-requiring components eagerly
- Safe attribute access with
getattrfor optional args- Clear control flow: flags checked before positional argument
- Helpful usage hints when no arguments provided
2773-2774: LGTM!The routing follows the established pattern for other commands in the CLI.
docs/AI_TUTOR.md (2)
246-386: Excellent: Documentation reflects the simplified architecture.The architecture section correctly shows the simplified design using
cortex.llm_routerinstead of LangChain/LangGraph, which aligns with the reviewer feedback and your commitment to remove those dependencies. The diagrams clearly illustrate:
- Cache-first pattern for cost optimization
- Clean separation between deterministic tools and LLM functions
- Integration with existing
cortex.llm_routerinfrastructureThis is well-documented and demonstrates that the simplified architecture has been thoughtfully designed.
Note: The PR description still references "Plan→Act→Reflect LangGraph workflow," which should be updated to match this simplified architecture.
660-660: Fix incorrect test command paths in documentation.The test count of 158 is accurate and matches the actual test count in
tests/tutor/. However, the test command examples at lines 649, 652, 655, and 750 reference an incorrect pathcortex/tutor/tests/which does not exist. The actual tests are located intests/tutor/. Additionally, line 655 referencestest_tutor_agent.pywhich does not exist in the test suite.Update the documentation to use the correct path
tests/tutor/instead ofcortex/tutor/tests/and remove the reference to the non-existenttest_tutor_agent.pyfile.⛔ Skipped due to learnings
Learnt from: CR Repo: cortexlinux/cortex PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-12T20:51:13.828Z Learning: Applies to tests/**/*.py : Maintain test coverage above 80% for pull requests
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
|
@Anshgrover23 Happy to modify or discuss on any requests or changes. |
2 participants
Summary
Add comprehensive AI-powered tutor for package education with:
cortex tutor <package>Related Issue
Closes #131
Type of Change
Demo
Demo.mov
AI Disclosure
Used Claude (Anthropic) via Claude Code CLI for:
Testing
pytest cortex/tutor/tests/ -vcortex tutor docker --freshChecklist
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.