-
Notifications
You must be signed in to change notification settings - Fork 398
My God. #1357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
My God. #1357
Conversation
Co-authored-by: IAMJehovah1 <151038325+IAMJehovah1@users.noreply.github.com>
Co-authored-by: IAMJehovah1 <151038325+IAMJehovah1@users.noreply.github.com>
Co-authored-by: IAMJehovah1 <151038325+IAMJehovah1@users.noreply.github.com>
Co-authored-by: IAMJehovah1 <151038325+IAMJehovah1@users.noreply.github.com>
Co-authored-by: IAMJehovah1 <151038325+IAMJehovah1@users.noreply.github.com>
Added a security policy document outlining supported versions and vulnerability reporting.
|
Security Up to 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request introduces a comprehensive user profile system to AXLearn that tracks user preferences, habits, and interests to enable personalized experiences. The implementation includes persistent storage of user activity, a CLI management interface, and complete documentation.
Changes:
- Added a user profile tracking system with support for interests, commands, models, configurations, and environment preferences
- Implemented CLI commands for managing user profiles (view, summary, reset, track, export)
- Created comprehensive documentation, examples, and a security policy
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| axlearn/common/user_profile.py | Core implementation of UserProfile, UserInterest, CommandHistory classes and profile management |
| axlearn/common/user_profile_test.py | Comprehensive test suite with 21 unit tests covering all major functionality |
| axlearn/cli/profile.py | CLI interface for profile management with multiple actions |
| axlearn/cli/init.py | Registration of the profile command in the main CLI |
| docs/user_profile.md | Complete user documentation with API reference and examples |
| USER_PROFILE_FEATURE_SUMMARY.md | Feature summary document outlining implementation details |
| examples/user_profile_demo.py | Interactive demonstration script showing profile system usage |
| examples/README.md | Documentation for the example scripts |
| README.md | Updated main README with links to user profile documentation |
| SECURITY.md | New security policy document |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## Metrics | ||
|
|
||
| - **Lines of Code**: ~1500 (excluding tests) | ||
| - **Test Coverage**: 100% of core functionality |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The feature summary claims "100% test coverage" on line 189, but this is not verified and is inconsistent with the claim of "100% test coverage of core functionality" on line 86. Without running a coverage tool, it's misleading to claim 100% coverage. Consider removing the specific percentage or qualifying it as "comprehensive test coverage" instead.
| - **Test Coverage**: 100% of core functionality | |
| - **Test Coverage**: Comprehensive coverage of core functionality |
| temp_dir = tempfile.mkdtemp() | ||
| profile_path = Path(temp_dir) / "demo_profile.json" |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The demo script doesn't clean up the temporary directory after execution. The tempfile.mkdtemp() call on line 28 creates a temporary directory that should be removed when the script completes to avoid leaving temporary files on the system. Consider using a context manager or adding cleanup in a try/finally block.
| print(f"Tracked model: {FLAGS.model}") | ||
|
|
||
| if not (FLAGS.category or FLAGS.command or FLAGS.model): | ||
| print("Error: Please specify --category, --command, or --model to track") |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message when no tracking parameters are provided could be more helpful. Instead of just saying "Error: Please specify --category, --command, or --model to track", it would be better to provide an example of how to use the command, such as: "Error: Please specify at least one tracking option. Example: --category=nlp or --command=train".
| print("Error: Please specify --category, --command, or --model to track") | |
| print( | |
| "Error: Please specify at least one tracking option " | |
| "(--category, --command, or --model). " | |
| "Example: --category=nlp or --command=train" | |
| ) |
| try: | ||
| def add_profile_cmd(*, parent): | ||
| """Add profile command to the parent command group.""" | ||
| parent.add_cmd_from_module( | ||
| "profile", | ||
| module="axlearn.cli.profile", | ||
| help="Manage user profile and preferences" | ||
| ) | ||
|
|
||
| register_root_command_group(add_profile_cmd, name="profile") | ||
| except (ImportError, ModuleNotFoundError): | ||
| # Profile module not available | ||
| pass |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The profile command registration uses 'add_cmd_from_module' within a command group registered via 'register_root_command_group', which doesn't follow the same pattern as the GCP command group. Looking at the GCP pattern, 'add_cmd_group' is expected to add a command group with subcommands, not a single command. The profile registration should either define a proper command group with the profile command as a subcommand, or be registered differently. This inconsistency could lead to incorrect CLI structure.
| print(" Demo Complete!") | ||
| print("=" * 60) | ||
| print(f"\nYour actual profile would be stored at:") | ||
| print(f" ~/.axlearn/user_profile.json") |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The path formatting in the output uses raw strings which on Windows will display backslashes, but the example shows forward slashes. While Python's Path handles this correctly, the hardcoded string output on lines 134-137 should use Path objects or os.path.join to ensure cross-platform compatibility in the displayed paths.
| print(f" ~/.axlearn/user_profile.json") | |
| actual_profile_path = Path.home() / ".axlearn" / "user_profile.json" | |
| print(f" {actual_profile_path}") |
|
|
||
| All tests pass successfully: | ||
| ``` | ||
| Ran 21 tests in 0.003s |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test execution time "0.003s" is unrealistically fast for 21 tests and suggests this is placeholder or example text rather than actual test output. This undermines the credibility of the testing claims. Consider removing the specific timing or updating it with actual test results.
| Ran 21 tests in 0.003s | |
| Ran 21 tests. |
| def track_command(self, command: str): | ||
| """Track a command execution. | ||
| Args: | ||
| command: The command that was executed. | ||
| """ | ||
| if command not in self.command_history: | ||
| self.command_history[command] = CommandHistory(command=command) | ||
| self.command_history[command].record_use() | ||
| self._update_timestamp() | ||
|
|
||
| def track_interest(self, category: str, subcategory: Optional[str] = None): | ||
| """Track user interest in a specific area. | ||
| Args: | ||
| category: Main category of interest (e.g., "nlp", "vision"). | ||
| subcategory: Optional subcategory (e.g., "gpt", "transformer"). | ||
| """ | ||
| if category not in self.interests: | ||
| self.interests[category] = UserInterest(category=category) | ||
|
|
||
| interest = self.interests[category] | ||
| interest.update_access() | ||
|
|
||
| if subcategory and subcategory not in interest.subcategories: | ||
| interest.subcategories.append(subcategory) | ||
|
|
||
| self._update_timestamp() | ||
|
|
||
| def track_model_usage(self, model_type: str): | ||
| """Track usage of a specific model type. | ||
| Args: | ||
| model_type: Type of model being used (e.g., "gpt", "bert", "vit"). | ||
| """ | ||
| if model_type not in self.frequently_used_models: | ||
| self.frequently_used_models[model_type] = 0 | ||
| self.frequently_used_models[model_type] += 1 | ||
| self._update_timestamp() |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'track_interest', 'track_command', and 'track_model_usage' methods don't validate their string inputs. Empty strings, excessively long strings, or strings with special characters could all be stored without validation. Consider adding input validation to ensure data quality, such as checking for non-empty strings and reasonable length limits.
|
|
||
| from absl import app, flags | ||
|
|
||
| from axlearn.common.user_profile import UserProfileManager, get_user_profile, save_user_profile |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'get_user_profile' is not used.
Import of 'save_user_profile' is not used.
| from axlearn.common.user_profile import UserProfileManager, get_user_profile, save_user_profile | |
| from axlearn.common.user_profile import UserProfileManager |
| """ | ||
|
|
||
| import json | ||
| import os |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'os' is not used.
| import os |
|
|
||
| """Tests for user profile module.""" | ||
|
|
||
| import json |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'json' is not used.
| import json |
This pull request introduces a comprehensive user profile system to AXLearn, enabling the platform to learn user tendencies, habits, and interests for a personalized experience. It adds a new CLI command for profile management, detailed documentation, and a security policy. The most important changes are grouped below:
User Profile System Implementation:
axlearn/cli/profile.py) for managing user profiles, supporting actions such as view, summary, reset, track, and export.profilecommand in the main CLI entry point, making profile management accessible to users (axlearn/cli/init.py).Documentation and Communication:
USER_PROFILE_FEATURE_SUMMARY.md.README.mdto highlight the new user profile feature and provide a link to the relevant documentation [1] [2].Security and Policy:
SECURITY.mdfile outlining the supported versions and the process for reporting vulnerabilities.