feat: Enhance cortex ask with interactive tutor capabilities
#395
Conversation
…ortexlinux#393) - Enhanced system prompt to detect educational vs diagnostic queries - Added LearningTracker class for tracking educational topics - Learning history stored in ~/.cortex/learning_history.json - Increased max_tokens from 500 to 2000 for longer responses - Added terminal-friendly formatting rules - Rich Markdown rendering for proper terminal display - Added 25 new unit tests (50 total) for ask module - Updated COMMANDS.md with cortex ask documentation
📝 WalkthroughWalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AskHandler
participant LearningTracker
participant FileSystem as File System
participant LLM as OpenAI/Claude
User->>AskHandler: ask(question)
AskHandler->>LearningTracker: is_educational_query(question)
activate LearningTracker
LearningTracker-->>AskHandler: true/false
deactivate LearningTracker
alt Question is Educational
AskHandler->>LearningTracker: extract_topic(question)
LearningTracker-->>AskHandler: topic
AskHandler->>LearningTracker: record_topic(question)
activate LearningTracker
LearningTracker->>FileSystem: load_history()
FileSystem-->>LearningTracker: history dict
LearningTracker->>LearningTracker: update topic in history
LearningTracker->>FileSystem: save_history(updated)
deactivate LearningTracker
end
rect rgb(200, 220, 255)
Note over AskHandler: Enhanced system prompt<br/>with Educational context
end
AskHandler->>LLM: call with expanded prompt + tokens=2000
LLM-->>AskHandler: response
AskHandler-->>User: formatted response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cortex/ask.py (2)
172-200: Consider edge case in topic truncation logic.Line 199 truncates topics to 50 characters at word boundaries using
topic[:50].rsplit(" ", 1)[0]. If the first 50 characters contain no spaces,rsplitreturns a single-element list, and[0]will return the full 50-character string, which is correct.However, this could still result in awkwardly truncated topics for long compound words or URLs. Consider whether 50 characters is sufficient for your use cases.
Example edge case:
# Topic with no spaces in first 50 chars topic = "verylongcompoundwordwithoutanyspacesthatexceeds50characters" # Result: "verylongcompoundwordwithoutanyspacesthatexceeds50"The current behavior is safe but might not be ideal for all inputs.
255-262: Silent failure on save may complicate debugging.Line 262 silently suppresses OSError when writing the learning history file. While this prevents the CLI from crashing when the history file can't be written (e.g., permission issues, disk full), it makes debugging difficult if users expect their learning history to be tracked but it's failing silently.
Consider whether this trade-off aligns with your UX goals. For a "Chill" review mode, this is acceptable, but you might want to log these failures in verbose mode in the future.
Based on learnings from similar patterns in the codebase.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cortex/ask.pycortex/cli.pydocs/COMMANDS.mdtests/test_ask.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
cortex/cli.pytests/test_ask.pycortex/ask.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain >80% test coverage for pull requests
Files:
tests/test_ask.py
🧬 Code graph analysis (1)
tests/test_ask.py (1)
cortex/ask.py (11)
ask(458-526)LearningTracker(139-262)SystemInfoGatherer(20-136)is_educational_query(165-170)extract_topic(172-200)record_topic(202-225)get_history(227-229)get_recent_topics(231-242)get_recent_topics(536-545)gather_context(129-136)_get_system_prompt(334-391)
⏰ 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.10)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: test (3.10)
🔇 Additional comments (13)
cortex/cli.py (2)
9-10: LGTM! Appropriate import for Markdown rendering.The import extends the existing Rich library usage to support formatted terminal output for the enhanced educational responses.
302-303: LGTM! Clean integration of Markdown rendering.The change properly leverages Rich's Markdown class to render formatted educational responses in the terminal, aligning with the enhanced output formatting rules in the system prompt.
tests/test_ask.py (3)
273-396: Excellent test coverage for LearningTracker.The test class comprehensively covers:
- Educational query pattern detection across multiple patterns
- Topic extraction with various prefixes
- Truncation handling for long topics
- History persistence and incremental updates
- Non-educational query filtering
The use of temporary files and proper patching ensures test isolation.
398-460: LGTM! Strong integration tests for learning features.The tests validate:
- Educational queries are properly recorded through AskHandler
- Diagnostic queries are correctly excluded from learning history
- Recent topics retrieval works through the handler API
- System prompt contains the necessary educational guidance
Good use of the fake provider to avoid external API dependencies.
462-493: Good validation of system prompt enhancements.The tests ensure the enhanced system prompt includes:
- Query type detection guidance
- Educational response instructions with examples and best practices
- Diagnostic response instructions with system context
This validates the PR's core objective of distinguishing between query types.
docs/COMMANDS.md (2)
75-132: Excellent documentation for the enhanced ask command.The documentation clearly explains:
- The dual nature of the command (diagnostic vs educational)
- Automatic intent detection
- System-aware responses and learning progress tracking
- Practical examples for both query types
The feature list and notes provide users with a complete understanding of the command's capabilities.
429-445: Well-structured learning workflow section.The workflow provides a clear progression:
- Diagnostic queries for system information
- Educational queries for learning
- Step-by-step tutorials
- Automatic tracking of learning history
This practical guide helps users understand and leverage the new capabilities.
cortex/ask.py (6)
1-17: LGTM! Clean import additions and updated docstring.The new imports (re, datetime, Path) are from the standard library and support the learning tracking functionality. The docstring accurately reflects the expanded scope to include educational content and progress tracking.
139-170: Well-designed LearningTracker class with comprehensive patterns.The class structure is solid:
- Pre-compiled regex patterns for efficiency
- Comprehensive educational query detection patterns
- Case-insensitive matching appropriately handles user input variations
The pattern list covers the main educational query types mentioned in the PR objectives.
288-288: LGTM! Proper integration of LearningTracker.The learning tracker is correctly instantiated in
__init__, ensuring it's available for all ask operations.
334-391: Excellent system prompt design for dual-mode operation.The enhanced system prompt effectively:
- Distinguishes between educational and diagnostic query types with clear trigger patterns
- Provides specific guidelines for each response type
- Includes explicit output formatting rules to prevent terminal rendering issues
- Provides a concrete example to guide the LLM
The level of detail is appropriate for ensuring consistent, high-quality responses across both query modes. The terminal-specific formatting rules (avoiding markdown headings, limiting line length) are particularly valuable.
401-401: Appropriate token increase for educational responses.Increasing max_tokens from 500 to 2000 for both OpenAI and Claude is necessary to support the longer, structured educational responses with code examples and best practices. The 4x increase aligns with the PR's objective to provide comprehensive tutorials.
Note: This will increase API costs, but is appropriate for the enhanced functionality.
Also applies to: 413-413
523-545: Clean integration and well-designed public API.The learning tracking integration:
- Records topics at the appropriate point (after successful answer generation)
- Exposes a clean public API through
get_learning_history()andget_recent_topics()- Follows Python conventions with proper type hints and docstrings
- Delegates appropriately to the LearningTracker instance
The API design allows external code to access learning history without tight coupling to the tracker implementation.
1 participant
Closes #393
Summary
Enhances the existing
cortex askcommand to include tutor-like capabilities without adding a separate command or flag. The LLM automatically detects whether a question is educational or diagnostic and responds appropriately.Changes:
LearningTrackerclass to track educational topics explored by the user~/.cortex/learning_history.jsonmax_tokensfrom 500 to 2000 for longer educational responsesCOMMANDS.mdwithcortex askdocumentationExample usage: